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

Compiler: cache cleanup transformer #11197

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

asterite
Copy link
Member

@asterite asterite commented Sep 10, 2021

Otherwise on every finished hook we might potentially be cleaning up big chunks
of the program over and over agin.

Consider this program:

{% for i in 0..3000 %}
  puts "Hello"
{% end %}

On my machine it takes about a second to compile.

Now this program:

{% for i in 0..3000 %}
  macro finished
    puts "Hello"
  end
{% end %}

It takes 8.5 seconds to compile! 😱

The reason is that we call cleanup on every finished hook, and that ends up creating a new CleanupTransformer. Every transformer will cleanup calls and their respective target methods (target_defs), and keep a cache of which have been transformed (many calls might end up hitting the same method, of course.) However, because we create a new CleanupTransformer on every finished hook, these methods are "cleaned up" over and over again (this could spawn a big chunk of the program, because it goes into those methods, calls in those methods, and target methods of those calls, etc.)

I noticed this because the interpreter uses cleanup a lot more, and it was taking like 1 minute to create bytecode for the compiler's program, while it took about 20 seconds to generate LLVM code, which was very unreasonable.

I know Lucky and other programs use macro finished a lot so this might improve compilation times a bit, but I don't expect a huge difference (in this sample program I have 3000 macro finished hooks, maybe a regular program have a dozen of them, I don't know)

Otherwise on every `finished` hook we might potentially be cleaning up big chunks
of the program over and over agin.
@asterite
Copy link
Member Author

I forgot to mention: with this PR, that snippet with those macro finished takes the same time now: about a second to compile, instead of 8.5 seconds.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

🚀

@beta-ziliani beta-ziliani added this to the 1.2.0 milestone Sep 10, 2021
Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

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

More performance 😋 Thanks @asterite 🙏

@asterite asterite merged commit 24486c7 into master Sep 10, 2021
@asterite asterite deleted the opt/compiler-cache-cleanup-transformer branch September 10, 2021 17:49
@elorest
Copy link
Contributor

elorest commented Sep 10, 2021

I think this may help a lot for me. We have a project which takes about 2 minutes to compile on my mac but about 40 minutes to compile to arm. We do use finish on a class which is inherited from a few hundred times.

@elorest
Copy link
Contributor

elorest commented Sep 10, 2021

So without --release flag… removing macro finish cut my compile time from 1:48.22 to 1:16.82.

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

Successfully merging this pull request may close these issues.

5 participants