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(benchmark): update BenchmarkIndex for m86 changes #11304

Merged
merged 2 commits into from
Aug 24, 2020

Conversation

patrickhulce
Copy link
Collaborator

Summary
tl;dr - Updates the BenchmarkIndex using advice from v8 team and research in #9085.

See #9085 for the very long, full story. We unfortunately missed the boat for m86 DevTools but at least would be nice to get it in for the CLI and LightRider.

Related Issues/PRs
ref #9085

@patrickhulce patrickhulce requested a review from a team as a code owner August 24, 2020 17:02
@patrickhulce patrickhulce requested review from connorjclark and removed request for a team August 24, 2020 17:02
* The division by 10 is to keep similar magnitudes to an earlier version of BenchmarkIndex that
* used a string length of 100000 instead of 10000.
*/
function benchmarkIndexGC() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is identical to the original other than the string is 10x smaller

@@ -160,13 +160,13 @@ class Driver {
}

/**
* Computes the ULTRADUMB™ benchmark index to get a rough estimate of device class.
Copy link
Collaborator

Choose a reason for hiding this comment

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

awwww it's learning :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

heh, yeah I figured now that there was a smidge of thought put into it and actual guidance from js-engine folks it needed a slight upgrade :)

lighthouse-core/lib/page-functions.js Outdated Show resolved Hide resolved
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.

None yet

4 participants