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

ast: cache ident lookups for consts in ast Expr str #22101

Conversation

spytheman
Copy link
Member

  • ast: cache Ident scope lookups for consts in Expr.str
  • ast,cgen: remove instrumentation for the cached ident lookup, cleanup

@spytheman spytheman changed the title cache ident lookups for consts in ast Expr str ast: cache ident lookups for consts in ast Expr str Aug 23, 2024
@spytheman
Copy link
Member Author

The performance is improved for compiling larger projects like V itself, and is ~ the same for shorter programs like hello_world.v:
image

@spytheman
Copy link
Member Author

Updating the .cached_name without synchronization, may cause some of the sanitizers to trigger 🤔 .

I think that since the cached result will be the same, even in the case of a race, it may be safe to add v__ast__Expr_str in the suppression lists.

@esquerbatua
Copy link
Contributor

Great job!

@spytheman
Copy link
Member Author

Thanks.

@spytheman spytheman merged commit 2bf59b1 into vlang:master Aug 23, 2024
73 checks passed
@medvednikov
Copy link
Member

Getting a stable 1.5% just from this tiny change is insane.

Makes you think how many more things can be optimized.

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