On 04.05.2011 12:28, Daniel Veillard wrote: > On Tue, May 03, 2011 at 06:14:44PM +0300, Stefan Kost wrote: >> hi, >> >> I am still not happy with the time it takes for gtk-doc to generate the >> html. I did some profiling and tired various changes. Its not too >> impressive. Just posting them here for feedback and comment on the >> directions of what would be acceptable or not. I technically have commit >> rights, so I can push things once they are ready. If patch 0002 looks >> like a good idea, I can extend it for other accessors. > Heretic idea, but what about not using the standard docbook > stylesheets for this but xsl tuned up to what you actually use and > need ? > >> Test case was generating the html for gstreamer-core api docs >> >> real user sys >> ------------------------------ before >> 1m22.872s 1m17.317s 0m1.444s >> 1m19.261s 1m17.437s 0m1.384s >> 1m19.770s 1m17.417s 0m1.264s >> ------------------------------ 0001-xpath-annotate-branch-prediction.patch >> 1m18.123s 1m16.677s 0m1.256s >> 1m18.684s 1m17.181s 0m1.292s >> 1m18.887s 1m17.217s 0m1.364s > Okay this have a bit of an effect at the expense of making the code > harder to read. > >> ------------------------------ >> 0002-xpath-split-traversal-into-init-and-next-functions.patch >> 1m18.537s 1m16.925s 0m1.388s >> 1m18.616s 1m16.913s 0m1.412s >> 1m18.249s 1m16.653s 0m1.328s > this looks more harmful than anything else, make is more complex, > increase the number of local variables in a function already big > all that to avoid a test to NULL which takes no time. NACK > Attaching two profiles for now. >> ------------------------------ >> 0003-xpath-avoid-a-memcpy-on-the-expense-of-temporarily-w.patch >> 1m17.481s 1m16.137s 0m1.208s >> 1m17.977s 1m16.481s 0m1.328s >> 1m17.791s 1m16.413s 0m1.232s > why 100 then, try to adjust a bit. I attached a new WIP patch here. This was doing duplicate work really. But indeed it is not going to make a huge performance difference. But I think it would make sense to merge the code here. Having two functions almost doing the same and one exclusively calling the other is maybe confusing. Maybe this was called by other code in the past. Stefan > > But really I would not try to chase improvement there. improving the > compilation of xsltstylesheet is way more likely to return actual gains. > For example, try to avoid dynamic lookup of variables at runtime, > most of those can be done at compilation time and this could avoid > lookups. > > But before optimizing, where is the time actually spent ? > My attemps with gprof in the past were kind of use less, I used > callgrind/kcachegrind like 5-6 years ago to improve speed and > using this and targetting obvious candidate, did indeed lead to > improvements. > > Daniel >
Attachment:
profile_after.svg
Description: image/svg
Attachment:
profile_before.svg
Description: image/svg
>From 82f2bde9abc9085e3f1a1ccccea6868e9c6e87f1 Mon Sep 17 00:00:00 2001
From: Stefan Kost <ensonic users sf net>
Date: Tue, 3 May 2011 17:25:39 +0300
Subject: [PATCH 3/4] xpath: simplify number formatting.
Just allocate a target buffer and fill, instead of printing to a temp buffer and
then dup'ing. As xmlXPathFormatNumber() is a static method only called by
xmlXPathCastNumberToString() we don't need to redo all the inf/nan cases. In the
end we could completely inline the code.
---
xpath.c | 179 ++++++++++++++++++++++++++-------------------------------------
1 files changed, 73 insertions(+), 106 deletions(-)
diff --git a/xpath.c b/xpath.c
index 476ac9d..a857590 100644
--- a/xpath.c
+++ b/xpath.c
@@ -2666,110 +2666,80 @@ xmlXPathPopExternal (xmlXPathParserContextPtr ctxt) {
static void
xmlXPathFormatNumber(double number, char buffer[], int buffersize)
{
- switch (xmlXPathIsInf(number)) {
- case 1:
- if (buffersize > (int)sizeof("Infinity"))
- snprintf(buffer, buffersize, "Infinity");
- break;
- case -1:
- if (buffersize > (int)sizeof("-Infinity"))
- snprintf(buffer, buffersize, "-Infinity");
- break;
- default:
- if (xmlXPathIsNaN(number)) {
- if (buffersize > (int)sizeof("NaN"))
- snprintf(buffer, buffersize, "NaN");
- } else if (number == 0 && xmlXPathGetSign(number) != 0) {
- snprintf(buffer, buffersize, "0");
- } else if (number == ((int) number)) {
- char work[30];
- char *ptr, *cur;
- int value = (int) number;
-
- ptr = &buffer[0];
- if (value == 0) {
- *ptr++ = '0';
- } else {
- snprintf(work, 29, "%d", value);
- cur = &work[0];
- while ((*cur) && (ptr - buffer < buffersize)) {
- *ptr++ = *cur++;
- }
- }
- if (ptr - buffer < buffersize) {
- *ptr = 0;
- } else if (buffersize > 0) {
- ptr--;
- *ptr = 0;
- }
- } else {
- /*
- For the dimension of work,
- DBL_DIG is number of significant digits
- EXPONENT is only needed for "scientific notation"
- 3 is sign, decimal point, and terminating zero
- LOWER_DOUBLE_EXP is max number of leading zeroes in fraction
- Note that this dimension is slightly (a few characters)
- larger than actually necessary.
- */
- char work[DBL_DIG + EXPONENT_DIGITS + 3 + LOWER_DOUBLE_EXP];
- int integer_place, fraction_place;
- char *ptr;
- char *after_fraction;
- double absolute_value;
- int size;
+ if (number == ((int) number)) {
+ int value = (int) number;
+ int size;
- absolute_value = fabs(number);
+ if (value == 0) {
+ snprintf(buffer, buffersize, "0");
+ } else {
+ size = snprintf(buffer, buffersize, "%d", value);
+ /* ensure '\0' termination */
+ if (size == buffersize) {
+ buffer[buffersize - 1] = 0;
+ }
+ }
+ } else {
+ /*
+ * buffersize should be DBL_DIG + EXPONENT_DIGITS + 3 + LOWER_DOUBLE_EXP
+ * DBL_DIG is number of significant digits
+ * EXPONENT is only needed for "scientific notation"
+ * 3 is sign, decimal point, and terminating zero
+ * LOWER_DOUBLE_EXP is max number of leading zeroes in fraction
+ * Note that this dimension is slightly (a few characters)
+ * larger than actually necessary.
+ */
+ int integer_place, fraction_place;
+ char *ptr;
+ char *after_fraction;
+ double absolute_value;
+ int size;
- /*
- * First choose format - scientific or regular floating point.
- * In either case, result is in work, and after_fraction points
- * just past the fractional part.
- */
- if ( ((absolute_value > UPPER_DOUBLE) ||
- (absolute_value < LOWER_DOUBLE)) &&
- (absolute_value != 0.0) ) {
- /* Use scientific notation */
- integer_place = DBL_DIG + EXPONENT_DIGITS + 1;
- fraction_place = DBL_DIG - 1;
- size = snprintf(work, sizeof(work),"%*.*e",
- integer_place, fraction_place, number);
- while ((size > 0) && (work[size] != 'e')) size--;
+ absolute_value = fabs(number);
- }
- else {
- /* Use regular notation */
- if (absolute_value > 0.0) {
- integer_place = (int)log10(absolute_value);
- if (integer_place > 0)
- fraction_place = DBL_DIG - integer_place - 1;
- else
- fraction_place = DBL_DIG - integer_place;
- } else {
- fraction_place = 1;
- }
- size = snprintf(work, sizeof(work), "%0.*f",
- fraction_place, number);
- }
+ /*
+ * First choose format - scientific or regular floating point.
+ * In either case, result is in work, and after_fraction points
+ * just past the fractional part.
+ */
+ if ( ((absolute_value > UPPER_DOUBLE) ||
+ (absolute_value < LOWER_DOUBLE)) &&
+ (absolute_value != 0.0) ) {
+ /* Use scientific notation */
+ integer_place = DBL_DIG + EXPONENT_DIGITS + 1;
+ fraction_place = DBL_DIG - 1;
+ size = snprintf(buffer, buffersize,"%*.*e",
+ integer_place, fraction_place, number);
+ while ((size > 0) && (buffer[size] != 'e')) size--;
- /* Remove fractional trailing zeroes */
- after_fraction = work + size;
- ptr = after_fraction;
- while (*(--ptr) == '0')
- ;
- if (*ptr != '.')
- ptr++;
- while ((*ptr++ = *after_fraction++) != 0);
-
- /* Finally copy result back to caller */
- size = strlen(work) + 1;
- if (size > buffersize) {
- work[buffersize - 1] = 0;
- size = buffersize;
- }
- memmove(buffer, work, size);
- }
- break;
+ }
+ else {
+ /* Use regular notation */
+ if (absolute_value > 0.0) {
+ integer_place = (int)log10(absolute_value);
+ if (integer_place > 0)
+ fraction_place = DBL_DIG - integer_place - 1;
+ else
+ fraction_place = DBL_DIG - integer_place;
+ } else {
+ fraction_place = 1;
+ }
+ size = snprintf(buffer, buffersize, "%0.*f",
+ fraction_place, number);
+ }
+ /* ensure '\0' termination */
+ if (size == buffersize) {
+ buffer[buffersize - 1] = 0;
+ }
+
+ /* Remove fractional trailing zeroes */
+ after_fraction = buffer + size;
+ ptr = after_fraction;
+ while (*(--ptr) == '0')
+ ;
+ if (*ptr != '.')
+ ptr++;
+ while ((*ptr++ = *after_fraction++) != 0);
}
}
@@ -5585,11 +5555,8 @@ xmlXPathCastNumberToString (double val) {
} else if (val == 0 && xmlXPathGetSign(val) != 0) {
ret = xmlStrdup((const xmlChar *) "0");
} else {
- /* could be improved */
- char buf[100];
- xmlXPathFormatNumber(val, buf, 99);
- buf[99] = 0;
- ret = xmlStrdup((const xmlChar *) buf);
+ ret = xmlMalloc(50);
+ xmlXPathFormatNumber(val, ret, 49);
}
}
return(ret);
--
1.7.1