| Summary: | A suggestion on Performance optimization in cairo-pdf-operators.c. | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Lance <lliu> |
| Component: | pdf backend | Assignee: | Adrian Johnson <ajohnson> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | enhancement | ||
| Priority: | medium | CC: | freedesktop, lliu |
| Version: | 1.8.8 | ||
| Hardware: | x86 (IA32) | ||
| OS: | Windows (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | Provide a builtin implementation of isspace and isdigit | ||
(In reply to comment #0) > I have done some profiling on cairo PDF backend using DevPartner. I found that > _count_word_up_to() consumes considerable amount of time. The related part of > profiling result is list as follows. > # Count % Time > 1 static int > 2 _count_word_up_to (const unsigned char > *s, int length) > 3 74,980 0.0 16,680.9 { > 4 74,980 0.0 15,453.6 int word = 0; > 5 > 6 259,305 0.1 47,794.9 while (length--) { > 7 259,305 5.2 3,597,598.4 if (! (isspace (*s) || *s == > '<')) { > 8 184,325 0.1 36,245.0 s++; > 9 184,325 0.1 37,524.4 word++; > 10 } else { > 11 74,980 0.0 14,235.6 return word; > 12 } > 13 } > 14 > 15 return word; > 16 } > Analysis: > Line 6 and Line 7 have been called for the same count (259305), but the time > taken by Line 7 is much longer than that of Line 6. The ratio can be calculated > as follows. > Time (Line 7) / Time (Line 6) = 3597598.4/47794.9 = 75.27 > From the source code, I think such a big ratio is caused by inefficent > isspace(). I think the functionality of isspace() should be simple based on the > method name. So I suggest replacing isspace() with an efficent implementation, > for example, using a switch-case or a look-up table. In addition to isspace() method, isdigit() has a similar problem. In my profiling result. isspace() and isdigit() took 11.0% and 7.5% respectively. So it would be nice to have light implementations for these two methods. Thanks, Lance All of the ctype.h functions in glibc are slow in UTF-8 locales, so alternatives would be useful. Fontconfig maintains a good list of the whitespace characters in the UCS; it would not be inappropriate to base an utf-8-aware isspace(3) on that code. Anything which only cares about POSIX- or C-locale spaces — aka ASCII whitespace — could use a function which tests only for /[ \f\n\r\t\v]+/. Cairo’s use of isdigit(3) probably only cares about ASCII digits; a simple function which only tests against /[0-9]+/ would do for that. In both cases, character encoding conversion must be avoided if one wishes to optimize performance. But we generate ASCII only! The code should simply hardcode a few characters there. Created attachment 29919 [details] [review] Provide a builtin implementation of isspace and isdigit Try the following patch and let me know if it improves the performance. (In reply to comment #4) > Created an attachment (id=29919) [details] > Provide a builtin implementation of isspace and isdigit > Try the following patch and let me know if it improves the performance. Thank you for the patch. It improves the performance. Here's the related part of profiling result. # Count % Time 1 static int 2 _count_word_up_to (const unsigned char *s, int length) 3 74,980 0.0 15,180.9 { 4 74,980 0.0 15,711.4 int word = 0; 5 6 259,305 0.1 51,434.9 while (length--) { 7 259,305 0.1 55,511.2 if (! (_cairo_isspace (*s) || *s == '<')) { 8 184,325 0.1 38,146.3 s++; 9 184,325 0.1 39,336.9 word++; 10 } else { 11 74,980 0.0 14,809.0 return word; 12 } 13 } 14 15 return word; 16 } It is nice to see that Line 6 and Line 7 take similar amount of time now. (In reply to comment #4) > Created an attachment (id=29919) [details] > Provide a builtin implementation of isspace and isdigit > Try the following patch and let me know if it improves the performance. I noticed that this patch was not included in cairo-1.9.4. Is it possible to add this patch into future cairo versions? Thanks. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
I have done some profiling on cairo PDF backend using DevPartner. I found that _count_word_up_to() consumes considerable amount of time. The related part of profiling result is list as follows. # Count % Time 1 static int 2 _count_word_up_to (const unsigned char *s, int length) 3 74,980 0.0 16,680.9 { 4 74,980 0.0 15,453.6 int word = 0; 5 6 259,305 0.1 47,794.9 while (length--) { 7 259,305 5.2 3,597,598.4 if (! (isspace (*s) || *s == '<')) { 8 184,325 0.1 36,245.0 s++; 9 184,325 0.1 37,524.4 word++; 10 } else { 11 74,980 0.0 14,235.6 return word; 12 } 13 } 14 15 return word; 16 } Analysis: Line 6 and Line 7 have been called for the same count (259305), but the time taken by Line 7 is much longer than that of Line 6. The ratio can be calculated as follows. Time (Line 7) / Time (Line 6) = 3597598.4/47794.9 = 75.27 From the source code, I think such a big ratio is caused by inefficent isspace(). I think the functionality of isspace() should be simple based on the method name. So I suggest replacing isspace() with an efficent implementation, for example, using a switch-case or a look-up table.