Closed vadz closed 5 years ago
I'm going to accept the PR. However, I would like to better understand what exactly causes the slow down in the current implementation. It's simply hard to imagine that generating the font key in an only slightly different way and caching the text color can significantly speed up the operation. Could you quantify the gain in speed?
Thanks for merging! Out of curiosity, why did you prefer to squash merge this instead of doing a normal merge?
As for your question, we made these changes after profiling the code and they result in ~20% improvement in PDF generation time (@thesiv please correct me if I'm wrong). Again, our case is probably almost the worst case scenario as there is a lot of text (almost nothing but text, in fact) and only a couple of fonts (so almost all of the calls hit the cache) but, still, based on these profiling results I'd expect this to have not an insignificant effect in the other cases too.
Out of curiosity, why did you prefer to squash merge this instead of doing a normal merge?
Good question. The squash merge was preselected by github. No idea, why. And I just clicked on the button without further thoughts.
Regarding the speed improvement: 20% ... that's indeed a huge improvement, even if it is a special use case. Thanks for sharing this improvement.
I'm forwarding this from https://github.com/vadz/wxpdfdoc/pull/2 as I think this is generally useful. In our use case, it results in significant speed up when generating text-heavy PDFs and while the exact amount will depend on the exact contents, I think it should help in just about all cases.