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

Stabilize oxc_semantic benchmarks #4270

Closed
overlookmotel opened this issue May 15, 2024 · 2 comments
Closed

Stabilize oxc_semantic benchmarks #4270

overlookmotel opened this issue May 15, 2024 · 2 comments

Comments

@overlookmotel
Copy link
Collaborator

Problem

Benchmarks for oxc_semantic are very unstable.

This is bad for 2 reasons:

  1. It's annoying to have PRs constantly bombarded erroneous with "semantic got 3% faster" / "semantic got 5% slower" messages from CodSpeed. This noise obscures what effect the PR actually has on performance.
  2. If we want to optimize semantic, we need to have stable measures to evaluate changes against.

Cause

I believe the cause to be reallocations of the various Vecs in Semantic struct.

Often Vecs can grow without changing location in memory - the allocator just extends the allocation in place. But sometimes it can't, and then you have a massive memory copy to move contents of the Vec to a new location.

I believe this non-deterministic element is what makes the benchmarks so noisy.

Possible solution

Cache the Vecs semantic needs in the Allocator. Re-use them on each run of semantic and then put them back into the allocator for the next run.

During benchmark warmup, these Vecs will grow to the size they need to be, so there'll be no reallocations during the measured run.

This will also improve semantic's performance in the real world where you're running it on multiple files in series.

Difficulty

Allocator uses interior mutability so you only need a &Allocator. How to combine that with this scheme?

@overlookmotel
Copy link
Collaborator Author

overlookmotel commented Jun 20, 2024

Hopefully #3776 has solved this. #3784 re-enabled semantic benchmarks and we can see if they stay steady now.

Leaving this open for now until we see if it's worked.

NB "Possible solution" section above would still be of benefit - it'd improve semantic's real-world perf. Although oxc-project/backlog#31 would also have same effect.

@Boshen
Copy link
Member

Boshen commented Jun 26, 2024

Seems to be stabilized after the allocator change.

@Boshen Boshen closed this as completed Jun 26, 2024
@overlookmotel overlookmotel transferred this issue from oxc-project/backlog Jul 15, 2024
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

No branches or pull requests

2 participants