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

nimgrep: change 2 iterators to closure #15962

Merged
merged 1 commit into from
Nov 14, 2020
Merged

Conversation

a-mr
Copy link
Contributor

@a-mr a-mr commented Nov 13, 2020

This simple change allows to mostly eliminate nimgrep's compilation time increase (29 sec -> 6.5 sec) and code bloat (760 KB -> 540 KB) introduced in #15612.

#15924 is avoided by not destructuring tuple in the iterators' headers.

cc @timotheecour

@timotheecour
Copy link
Member

timotheecour commented Nov 13, 2020

  • any chance you could show a minimized example (ideally not involving any imports) showing code that, under a similar transformation, undergoes such a drastic compilation time decrease? that will help to track if codegen does something silly
    I'm guessing this has to do with Tuple destructuring is broken with closure iterators #15924 as you mentioned

  • my experience as I worked on add filewalks fusion#32 showed that closure iterators were still a bit slower than inline iterators (even in that use case), but this still seems like an acceptable tradeoff while waiting for add filewalks fusion#32 to be merged in fusion so it can be reused in nimgrep

@Araq Araq merged commit 784720a into nim-lang:devel Nov 14, 2020
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.

3 participants