Skip to content

core(runner): store gather timing on artifacts#13587

Merged
adamraine merged 7 commits into
fr-defer-audit-runnerfrom
artifacts-timing
Feb 1, 2022
Merged

core(runner): store gather timing on artifacts#13587
adamraine merged 7 commits into
fr-defer-audit-runnerfrom
artifacts-timing

Conversation

@adamraine

@adamraine adamraine commented Jan 24, 2022

Copy link
Copy Markdown
Contributor

Proposal to fix the timing issues in #13569.

  • Taking a position on core(runner): independent gather and audit functions #13569 (comment) , lh:runner:gather will always reflect the time spent gathering the page, and never the time to load artifacts from disk.
  • There will always be a lh:runner:gather in artifacts.Timing
  • Sample artifacts will have unique startTime for each timing entry so they aren't deduped in Runner._getTiming:

lighthouse/lighthouse-core/runner.js

Lines 200 to 202 in 62848fb

// As entries can share a name, dedupe based on the startTime timestamp
].map(entry => /** @type {[number, PerformanceEntry]} */ ([entry.startTime, entry]));
const timingEntries = Array.from(new Map(timingEntriesKeyValues).values())

adamraine
adamraine commented Jan 24, 2022
View reviewed changes
Comment thread lighthouse-core/test/results/artifacts/artifacts.json
@adamraine adamraine marked this pull request as ready for review January 26, 2022 19:50
@adamraine adamraine requested a review from a team as a code owner January 26, 2022 19:50
@adamraine adamraine requested review from connorjclark and removed request for a team January 26, 2022 19:50
paulirish
paulirish approved these changes Jan 31, 2022
View reviewed changes

@paulirish paulirish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yup its great

@adamraine adamraine merged commit 900f948 into fr-defer-audit-runner Feb 1, 2022
@adamraine adamraine deleted the artifacts-timing branch February 1, 2022 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulirish paulirish paulirish approved these changes

@connorjclark connorjclark connorjclark approved these changes

Assignees

@connorjclark connorjclark

Labels

waiting4reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@adamraine @paulirish @connorjclark @devtools-bot