search

sabre-io / xml

sabre/xml is an XML library that you may not hate.
http://sabre.io/xml/
BSD 3-Clause "New" or "Revised" License
516 stars 77 forks source link

Cache namespaces parsed from clark notation #82

Closed icewind1991 closed 8 years ago

icewind1991 commented 8 years ago

When serializing large xml documents the cost parsing of the namespaces from clark notation becomes a significant portion (>10%) of the serialization cost. Since most xml documents will only use a fairly small number of xml elements we're better of keeping the parsed version of the namespace/element around for reuse.

This saves doing a regex (and some memory allocations) for each element we serialize

comparison

cc @evert

staabm commented 8 years ago

This patch saves 30ms walltime, what is the total parsing walltime? 300ms? Please post a profile of a single run.

Would it be possible to do the Caching in the Service class, so all calls to it can benefit from the cache?

evert commented 8 years ago

I'm a bit new to blackfire. Is there a way to find out which script you used to generate the profile? It if it's all good and the benefit checks out, I do agree with @staabm that I think it's a bit better to move the cache to the function in the Service class, not in the least because it also means that the cache survives multiple runs.

You can simply declare a static property inside a function body like this:

function foo() {

   static $cache;

}
evert commented 8 years ago

Never mind on sharing the original script, I seen now that it was a whole webdav request. Looking into this a bit further

icewind1991 commented 8 years ago

Switched to using a static variable in the method.

Updated comparison: https://blackfire.io/profiles/compare/74200494-931e-436c-b367-1ec3fca31419/graph

evert commented 8 years ago

Failing build is unrelated. Looks good!