Re: [xslt] speeding up xslt in gtk-doc

[Daniel Veillard <veillard redhat com>]

On Fri, May 06, 2011 at 05:49:20PM +0300, Stefan Kost wrote:
> On 04.05.2011 12:18, Daniel Veillard wrote:
> > On Wed, May 04, 2011 at 11:43:03AM +0300, Stefan Kost wrote:
> >> On 03.05.2011 18:14, 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.
> >> Daniel, btw. are you fine with me pushing a couple of obvious patches
> >> that only fix compiler warnings (like unused variables and missing
> >> casts). I would do that for libxml2 and libxslt.
> There are a lot of warnings like this:
> ISO C90 does not support "long long"

  Ignore all compilers where we need this have it

> ISO C forbids assignment between function pointer and 'void *'

  Leave them, they come from an API definition, we can't really change
it as it's public, and trying to hide the issue isn't good either,
we need to learn to live with those.

> not sure how you want to handle them.
> 
> Also tests have a few
> ignoring return value of 'write', declared with attribute warn_unused_result
> that could be taken care of if desired.

  that would be a good idea, yes

> For
> 0006-xmlmemory-add-a-cast-size_t-has-no-portable-printf-m.patch
> I would actually suggest to use "%lu" and (long unsigned) when using
> that with a size_t.
[...]
> diff --git a/xpath.c b/xpath.c
> index a857590..d4db467 100644
> --- a/xpath.c
> +++ b/xpath.c
> @@ -7669,7 +7669,7 @@ xmlXPathNextDescendantOrSelfElemParent(xmlNodePtr cur,
>  #ifdef LIBXML_DOCB_ENABLED
>  	    case XML_DOCB_DOCUMENT_NODE:
>  #endif
> -	    case XML_HTML_DOCUMENT_NODE:	    
> +	    case XML_HTML_DOCUMENT_NODE:
>  		return(contextNode);
>  	    default:
>  		return(NULL);
> @@ -12420,7 +12420,7 @@ apply_predicates: /* --------------------------------------------------- */
>          if ((predOp != NULL) && (seq->nodeNr > 0)) {
>  	    /*
>  	    * E.g. when we have a "/foo[some expression][n]".
> -	    */	    
> +	    */
>  	    /*
>  	    * QUESTION TODO: The old predicate evaluation took into
>  	    *  account location-sets.
> @@ -12429,7 +12429,7 @@ apply_predicates: /* --------------------------------------------------- */
>  	    *  All what I learned now from the evaluation semantics
>  	    *  does not indicate that a location-set will be processed
>  	    *  here, so this looks OK.
> -	    */	    
> +	    */
>  	    /*
>  	    * Iterate over all predicates, starting with the outermost
>  	    * predicate.

that's just end of line blanks, the problem is that they are all over
the place. Fixing only 3 of them doesn't really solve the issue, and
fixing all of them introduce a lot of churn making potential
backporting harder on new patches.
So far I resisted cleaning them up for this reason.

> >From 20da44a0d8c1cfbf963cde3825fbc66b513e0ddd Mon Sep 17 00:00:00 2001
> From: Stefan Kost <ensonic users sf net>
> Date: Fri, 6 May 2011 17:03:51 +0300
> Subject: [PATCH 5/6] xpath: remove unused variable
> 
> As noted by gcc, this variable is not beeing used.
> ---
>  xpath.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/xpath.c b/xpath.c
> index 0bb6f43..5258b3a 100644
> --- a/xpath.c
> +++ b/xpath.c
> @@ -10030,7 +10030,6 @@ static void
>  xmlXPathCompNumber(xmlXPathParserContextPtr ctxt)
>  {
>      double ret = 0.0;
> -    double mult = 1;
>      int ok = 0;
>      int exponent = 0;
>      int is_exponent_negative = 0;
> -- 
> 1.7.1

  Ok, ACK

> >From f3bf078327f038f3de29ca36897b3768ab340062 Mon Sep 17 00:00:00 2001
> From: Stefan Kost <ensonic users sf net>
> Date: Fri, 6 May 2011 17:40:10 +0300
> Subject: [PATCH 6/6] xmlmemory: add a cast size_t has no portable printf modifier
> 
> ---
>  xmlmemory.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xmlmemory.c b/xmlmemory.c
> index 433abb8..984aa82 100644
> --- a/xmlmemory.c
> +++ b/xmlmemory.c
> @@ -205,7 +205,8 @@ xmlMallocLoc(size_t size, const char * file, int line)
>  
>      if (xmlMemTraceBlockAt == ret) {
>  	xmlGenericError(xmlGenericErrorContext,
> -			"%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt, size);
> +			"%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt,
> +			(long int)size);
>  	xmlMallocBreakpoint();
>      }
>  
> @@ -273,7 +274,8 @@ xmlMallocAtomicLoc(size_t size, const char * file, int line)
>  
>      if (xmlMemTraceBlockAt == ret) {
>  	xmlGenericError(xmlGenericErrorContext,
> -			"%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt, size);
> +			"%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt,
> +			(long int)size);
>  	xmlMallocBreakpoint();
>      }
>  
> @@ -349,7 +351,8 @@ xmlReallocLoc(void *ptr,size_t size, const char * file, int line)
>      if (xmlMemTraceBlockAt == ptr) {
>  	xmlGenericError(xmlGenericErrorContext,
>  			"%p : Realloced(%ld -> %ld) Ok\n",
> -			xmlMemTraceBlockAt, p->mh_size, size);
> +			xmlMemTraceBlockAt, (long int)p->mh_size,
> +			(long int)size);
>  	xmlMallocBreakpoint();
>      }
>      p->mh_tag = MEMTAG;

  agreed %lu and (unsigned long) cast would be better.

Daniel


-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/