libxml-raku / LibXML-raku

Raku bindings to the libxml2 native library
Artistic License 2.0
11 stars 4 forks source link

Optimize resolve-package for a significant speedup #79

Closed jnthn closed 2 years ago

jnthn commented 2 years ago

Profiling why XLSX::Spreadsheet produced output so slowly revealed that box-class, and in turn resolve-package, was a hot path. The cause, in turn, is that require is not especially cheap - and was being called tens of thousands of times. This was in turn because the construct:

::{$pkg}:exists ?? ::($pkg) !! do require ::($pkg)

Is incorrect; ::{$key} is a stash lookup exactly on the passed key, and does not parse it for :: separators as in ::(...). Thus with the condition always being False, we'd always hit the require.

This could have been fixed by splitting up the $pkg or looking at the output of ::($pkg) to see if it actually found something. I did a slightly more invasive change that introduces a lookup on the entire class name, taking care of thread safety.

The results are dramatic: the entire process of persisting the XLSX sheet in the benchmark considered dropped from 44.66s to just 1.57s (this involves a more than just producing XML, so the improvement to XML generation alone would be a bit more than this).

Even with this change, it seems that box-class calls alone still account for around 12% of the time spent producing the XLSX output. Given we go from integer type code to string and then do the lookup here, there is probably scope for further improvement in this area.

dwarring commented 2 years ago

Thanks!

dwarring commented 2 years ago

Released as LibXML 0.7.9

zaucker commented 2 years ago

@dwarring Thanks for the quick release again, this makes my Excel exports really usable. Down from 90 to about 5 seconds, of which probably half is not LibXML related (just a rough guess ).