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

core: truncate measure timings to hundredths #7748

Merged
merged 2 commits into from
Mar 29, 2019
Merged

Conversation

connorjclark
Copy link
Collaborator

Reduces LHR size by ~4KB. That was ~1% of the LHR I tested it with.

Related Issues/PRs

#7160

@connorjclark
Copy link
Collaborator Author

Wait, wrong timings :)

@connorjclark connorjclark changed the title core: truncate user timings to hundredths core: truncate measure timings to hundredths Mar 27, 2019
Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

Here's a before and after:
image

works for me % comment nit

@@ -180,6 +180,12 @@ class Runner {
// 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());
for (const timing of timingEntries) {
Copy link
Member

Choose a reason for hiding this comment

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

add a comment above this loop to explain why this is being done.

@@ -180,6 +180,12 @@ class Runner {
// 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());
for (const timing of timingEntries) {
// @ts-ignore - ignore readonly
timing.startTime = parseFloat(timing.startTime.toFixed(2));
Copy link
Member

Choose a reason for hiding this comment

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

It looks like you may have to make a copy of these in order to do this. Real PerformanceEntry's properties really do appear to be read only

Copy link
Member

Choose a reason for hiding this comment

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

yeaup.
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@paulirish
Copy link
Member

shout out to @wardpeet who made extension-test a thing! that failure was so rad. :D

@paulirish paulirish merged commit 39d877f into master Mar 29, 2019
@paulirish paulirish deleted the truncate-timings branch March 29, 2019 03:33
@patrickhulce
Copy link
Collaborator

hip hip hurray!! @wardpeet is like, the best example of open source awesomeness around 😃

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

Successfully merging this pull request may close these issues.

4 participants