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

clients(lr): run benchmark repeatedly given special query parameter #14075

Merged
merged 9 commits into from
Jun 8, 2022

Conversation

paulirish
Copy link
Member

@paulirish paulirish commented Jun 2, 2022

This weird mode is for some lightrider variance work underway.

Example output:
image

cc @adrianaixba

types/artifacts.d.ts Outdated Show resolved Hide resolved
types/lhr/lhr.d.ts Outdated Show resolved Hide resolved
const bidxRunCount = options.settings.__internalMegaBenchmarkIndex;
// Keep the first one in there
const bidxes = [baseArtifacts.BenchmarkIndex];
for (let i = 0; i < bidxRunCount; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be simpler if the internal setting was just minus one'd here (And we asserted it either doesn't exist or is >= 1).

Copy link
Member Author

Choose a reason for hiding this comment

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

as we're the only users i think this isn't a huge deal. and it'd complicate the impl

if (options.settings.__internalMegaBenchmarkIndex) {
const bidxRunCount = options.settings.__internalMegaBenchmarkIndex;
// Keep the first one in there
const bidxes = [baseArtifacts.BenchmarkIndex];
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: bidxes -> indexes?

@@ -61,6 +61,11 @@ export async function runLighthouseInLR(connection, url, flags, lrOpts) {
config.settings.onlyCategories = categoryIDs;
}
}
// Set to 0 to disable
Copy link
Collaborator

@connorjclark connorjclark Jun 2, 2022

Choose a reason for hiding this comment

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

Set what to 0? Who is this comment for?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I think I see. Could you instead make megaBidx be optional, or union with | false ?

Copy link
Member Author

Choose a reason for hiding this comment

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

moved it to channel='fr' && queryParam('bidx') instead.

@connorjclark
Copy link
Collaborator

I assume I can review this fine without understanding the motivation :)

@connorjclark connorjclark changed the title clients(lr): add option to run benchmarkIndex repeatedly clients(lr): run benchmarkIndex repeatedly given special query parameter Jun 2, 2022
@connorjclark connorjclark changed the title clients(lr): run benchmarkIndex repeatedly given special query parameter clients(lr): run benchmark repeatedly given special query parameter Jun 2, 2022
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.

3 participants