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

Strictness optimizations #132

Merged
merged 4 commits into from
Oct 23, 2013
Merged

Conversation

JohnLato
Copy link

These changes result in modest performance improvements and significant memory reductions for all current benchmarks (Dragon, Rotations, Poster).

John Lato added 4 commits October 21, 2013 21:17
Float divisions out of the inner calculation.
For diagrams with a large number of transforms, this can be 5-10% faster and
use slightly less memory.
(e1 <> moveOriginBy (negateV . getTotalOffset $ o1) e2)
= let !negOff = negateV . getTotalOffset $ o1
e2Off = moveOriginBy negOff e2
!() = maybe () (\f -> f `seq` ()) $ appEnvelope e2Off
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like black magic to me. Can you explain what this is doing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds strictness to the 'Envelope' function which is the second param. to 'TotalOffset'. Specifically, it adds strictness under the Option by grabbing the envelope function and forcing it to be evaluated (the '\f -> f seq ()' lambda).

The reason for this is that building a fingertree (to make a Path) creates a giant chain of thunks to calculate the total envelope. We don't lose anything from strictly evaluating, because the envelope will always be used eventually. There's still quite a bit of laziness under that 'f' (which would probably be highly beneficial to remove), but I'm not sure how to do so yet. I have an idea, and if it works out, this can go back to something much cleaner.

@byorgey
Copy link
Member

byorgey commented Oct 23, 2013

OK, makes sense, thanks. Yeah, Envelopes need some love. I'm all ears about ways to make them stricter/faster/etc. I've also been meaning for a while to build some sort of caching into them (which I think might help in some common situations) but haven't gotten around to it yet.

byorgey added a commit that referenced this pull request Oct 23, 2013
@byorgey byorgey merged commit d09585d into diagrams:master Oct 23, 2013
@byorgey
Copy link
Member

byorgey commented Oct 23, 2013

Also, thanks for this fantastic work!

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

Successfully merging this pull request may close these issues.

None yet

2 participants