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

misc(build): prevent over optimization of computeBenchmarkIndex #13366

Merged
merged 5 commits into from
Nov 17, 2021

Conversation

connorjclark
Copy link
Collaborator

Rollup was deleting the for (let j = 0; j < 10000; j++) s += 'a'; line in the benchmark index function, resulting in a value of 500,000+ in bundled results.

@connorjclark connorjclark requested a review from a team as a code owner November 12, 2021 22:48
@connorjclark connorjclark requested review from adamraine and removed request for a team November 12, 2021 22:48
@google-cla google-cla bot added the cla: yes label Nov 12, 2021
@brendankenny
Copy link
Member

Nice find!

Really it's not great for a sufficiently advanced JS engine either. v8 just needs a small tweak to its heuristics and it would notice the same opportunity.

The traditional way around this is to assert anything about the end result. e.g. if (s.length !== 10_000) throw new Error('...') shouldn't have any real penalty and the compiler would need to do considerably more work at that point to eliminate it.

Would that work instead of special casing the file?

@connorjclark connorjclark changed the title misc(build): do not treeshake page-functions.js misc(build): prevent compiler over optimization of computeBenchmarkIndex Nov 12, 2021
@connorjclark connorjclark changed the title misc(build): prevent compiler over optimization of computeBenchmarkIndex misc(build): prevent treeshake of computeBenchmarkIndex Nov 17, 2021
@connorjclark connorjclark changed the title misc(build): prevent treeshake of computeBenchmarkIndex misc(build): prevent over optimization of computeBenchmarkIndex Nov 17, 2021
@connorjclark connorjclark merged commit b6eaa9f into master Nov 17, 2021
@connorjclark connorjclark deleted the bidx_treeshake branch November 17, 2021 01:02
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.

4 participants