GetLoops: Use Data.Sequence instead #74

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

bgamari commented Nov 23, 2012

Unfortunately I don't have any numbers to characterize the difference but asymptotically it seems that this should be a bit of a win. Of course, how much of a win will depend upon how large loops typically get before closing.

bgamari added some commits Nov 18, 2012

Scale3: Don't use numeric literal in place of rational
Not that it will likely make a difference but there's no reason to
truncate a rational in this case
Implement Mirror3 transform
Has yet to be wired in to parser
GetLoops: Use Data.Sequence.Seq instead of []
This should result in a small performance boost as getLoops' often needs
to check the end of the working loop to ensure it hasn't completely.
Owner

colah commented Nov 26, 2012

I don't necessarily have objections to merging this, but you're going to need to explain a bit further. When is using Data.Sequence.Seq faster? Loops should be bounded by something like 8*6 in the worst case....

It just makes the code a little less clean and approachable, so I want to make sure we're getting something out of it. :)

Contributor

bgamari commented Nov 26, 2012

I didn't have a systematic benchmark when this was written so this wasn't easy to determine. Now that we have a simple benchmark (which I'll submit a pull request for shortly), I can try measuring the true impact of this change. It may be that it isn't as large as I expected. I'll let you know soon.

@bgamari bgamari closed this Aug 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment