Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvements #22

Open
byorgey opened this issue Dec 24, 2016 · 6 comments
Open

Performance improvements #22

byorgey opened this issue Dec 24, 2016 · 6 comments

Comments

@byorgey
Copy link
Member

byorgey commented Dec 24, 2016

It seems that the performance of SVGFonts could still be improved quite a bit. I'm opening this ticket as a placeholder. See #16 for some earlier discussion of performance.

@Fuuzetsu
Copy link

Fuuzetsu commented Dec 25, 2016

@byorgey I have some work done already. There is an extensive commit with performance numbers there already. The reason why I didn't submit it is because while writing the commit message, I realised that part of the changes I made were a lie and didn't improve performance (much). Notably I

  • changed xml to hexpat: it's faster and 2x less memory used without a spike: it seems it's a good producer
  • did some minor changes to parser which cut time by a third
  • moved outline calculation to a FontData, computer lazily and removed PreparedFont; I was hoping to improve sharing and while it works fine (lazily), I have come to believe that outlinefunction and other things which were removed in the process worked similarly well because we weren't forcing the actual outlines of theOutlineMap`: so my changes were sort of a no-op and hence why I withheld the PR because it doesn't improve that much for the actual work. Initially I thought it helped because I confounded the hexpat migration improvement data with the numbers I got after this change.

While I haven't managed to improve performance by any worthwhile factor after initial processing, the data in the test I'm using does point out to an interesting thing: my test is something as follows:

vcat $ foldl' ... (for [1..500] \i -> textSVG ...) ...

The heap graph starts off with Diagrams.Segments memory spiking and gradually falling off. I suspect that a problem with my use of SVGFonts (generate a lot of text and concat it together) may in part be coming from diagrams: perhaps diagram concatenation is too lazy? Or perhaps I should be forcing the diagrams somehow (maybe seq width?)? Honestly I'm a bit out of my scope as far as diagrams is concerned. I don't have the code on hand this very second but I can push the branch with actual numbers and heap graphs later if you're interesting in pursuing this/providing guidance.

@Fuuzetsu
Copy link

Fuuzetsu commented Dec 25, 2016

Fuuzetsu@4ad81f1 ; sadly as I mention, seems OutlineMap elimination didn't work as I had hoped but I don't have time to revert those changes right now. Really, perf data in that commit is the most worthwhile thing. It makes me think the program I'm using to benchmark is too lazy and perhaps I should try and find the way to make diagrams in better way but perhaps I'm wrong all together.

EDIT: Although the diagram build-up only happens at start and later stays at constant way so perhaps it's the svg text builder itself that's leaking and that could be improved.

@byorgey
Copy link
Member Author

byorgey commented Jan 6, 2017

Hmm, with Diagrams.Segments memory spiking and then gradually falling off, that makes me think that the problem might like more in diagrams itself. SVGFonts generates a lot of paths which contain a lot of Segments. So any speed or memory improvements to diagrams' handling of paths & segments is going to improve things. Though I do not think we can avoid the memory spike in particular: in order to start drawing the diagram we have to compute its Envelope which in turn depends on all the Segments it contains. So we cannot actually start drawing anything before generating all the Segments.

@Fuuzetsu
Copy link

Fuuzetsu commented Jan 9, 2017

Do you think it'd be feasible for someone who never directly used or looked into diagrams to find such a leak or is that something you'd have to look into yourself?

@cchalmers
Copy link
Member

I've actually been working on a rewrite of diagrams and spent quite a lot time trying to optimise path operations. Hopefully those changes will help with some of the performance issues, although I'm not sure how and when it will get merged (if at all).

It's not finished yet but if you want to take a look see geometry on the segment branch. You can't turn it into a diagram yet on that branch but you could vcat a Path and get the width to force it.

You could also try using the bounding box of the string as the Envelope for the diagram. It doesn't even have to be the "true" bounding box. It might be faster to use the height of the font at that size and horizontal advances for the width (in a lot of cases, this is what we actually want anyway).

@cchalmers
Copy link
Member

If you do try geometry also use HEAD of diagrams-solve. I've inlined the formulas there which can make quite a big difference. (this might not have much effect on diagrams-lib since things aren't inlined/specialised there)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants