User "Catrope" changed the status of MediaWiki.r104106. Old Status: new New Status: ok
User "Catrope" also posted a comment on MediaWiki.r104106. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/104106#c26493 Commit summary: Another major performance improvement for updated annotated retentions Comment: I'm not sure if you realize this, but the nodes preceding fromNode are traversed three times by this code, and the nodes to be annotated are traversed twice: # getNodeFromOffset( this.cursor ) traverses the nodes preceding fromNode (1st time) # getNodeFromOffset( to ) traverses the nodes preceding fromNode (2nd time), and the nodes to be annotated (1st time) # traverseLeafNodes() traverses the nodes preceding fromNode (3rd time) and the nodes to be annotated (2nd time) Ironically, this means that your performance improvement (updating fewer nodes) might actually be slower in extreme cases (both from and to are close to the end of the document, and the document is very flat, i.e. it has a large number of top-level nodes and relatively few leaf nodes). Regardless of whether it actually is slower (I believe this revision will probably improve performance in almost all cases), there's room for improvement: don't traverse stuff three times. I think we should make traverseLeafNodes() smarter so it will: * allow you to pass a "to" parameter as well as a "from" node * allow both nodes and offsets for from and to * keep track of offsets so we can avoid calling getNodeFromOffset() for the to node (for the from node it's OK) * (bonus) pass offsets as well as the index to the callback function _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
