RE: Inefficient code in xml.el

[klaus.berndl]

Well, i get the point, but how an elisp-programmer should know this?!

The manual says:

 - Function: match-data
     This function returns a newly constructed list containing all the
     information on what text the last search matched.  Element zero is
     the position of the beginning of the match for the whole
     expression; element one is the position of the end of the match
     for the expression.  The next two elements are the positions of
     the beginning and end of the match for the first subexpression,
     and so on.  In general, element number 2N corresponds to
     `(match-beginning N)'; and element number 2N + 1 corresponds to
     `(match-end N)'.

     All the elements are markers or `nil' if matching was done on a
     buffer, and all are integers or `nil' if matching was done on a
     string with `string-match'.

     As always, there must be no possibility of intervening searches
     between the call to a search function and the call to `match-data'
     that is intended to access the match data for that search.

          (match-data)
               =>  (#<marker at 9 in foo>
                    #<marker at 17 in foo>
                    #<marker at 13 in foo>
                    #<marker at 17 in foo>)

What about to mention such inefficiency-problems in the documentation?!
The manual only says "... Corresponds to ....". IMHO the documentation
must mention the performnace-topic if it is so important you wrote in
these postings!

Ciao,
Klaus


David Kastrup wrote:
> address@hidden (Kim F. Storm) writes:
> 
>> I noticed that xml.el has several occurences of code like this:
>> 
>>         ((looking-at (concat "<!ENTITY[ \t\n\r]*\\(" xml-name-re
>>                              "\\)[ \t\n\r]*\\(" xml-entity-value-re
>>                              "\\)[ \t\n\r]*>"))
>>          (let ((name  (buffer-substring (nth 2 (match-data))
>>                                         (nth 3 (match-data))))
>>                (value (buffer-substring (+ (nth 4 (match-data)) 1)
>>                                         (- (nth 5 (match-data)) 1))))
>>            (goto-char (nth 1 (match-data)))
>> 
>> 
>> Using (match-data) like that is VERY, VERY inefficient.
> 
> Uh, that is only half the story.  It is not just devastatingly
> inefficient by itself.  Every single such call creates a whole slew of
> markers, and those slow down _any_ text manipulation in the buffer
> _afterwards_ until they get garbage-collected at some indeterminate
> point of time in the future.
> 
> The code passage above creates 30 new markers _every_ time it is run.
> All of these are maintained for every insertion/deletion in the buffer
> until garbage collection finally removes them.
> 
>> Use either match-beginning/match-end, or match-string instead.
> 
> Yes, yes, YES!