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.

Sign up for GitHub

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

Jump to bottom

Performance 2024 Queries #3712

Merged
merged 26 commits into from
Oct 30, 2024
Merged

Performance 2024 Queries #3712

merged 26 commits into from
Oct 30, 2024

Conversation

kevinfarrugia
Copy link
Contributor

@kevinfarrugia kevinfarrugia commented Aug 2, 2024 •
edited by tunetheweb
Loading

Makes progress on #3602

@kevinfarrugia kevinfarrugia marked this pull request as draft August 2, 2024 15:20
@tunetheweb tunetheweb added the analysis Querying the dataset label Aug 21, 2024
@tunetheweb tunetheweb added this to the 2024 Analysis milestone Aug 21, 2024
@mgifford
Copy link
Contributor

These tests seem to calculate their own web core vitals data, rather than just using the score that is provided by Google Lighthouse.  Is there a reason to calculate them separately? 

I think it could be easily added to so that we're pulling the performance data at the same time as accessibility  sql/2024/accessibility/lighthouse_a11y_score.sql
https://github.com/HTTPArchive/almanac.httparchive.org/pull/3749/files#diff-6d6ec13b6d0025840195d847d1e13775c6fecb660c65f79ab449209cab0d8589

@kevinfarrugia
Copy link
Contributor Author

These tests seem to calculate their own web core vitals data, rather than just using the score that is provided by Google Lighthouse.  Is there a reason to calculate them separately? 

I think it could be easily added to so that we're pulling the performance data at the same time as accessibility  sql/2024/accessibility/lighthouse_a11y_score.sql https://github.com/HTTPArchive/almanac.httparchive.org/pull/3749/files#diff-6d6ec13b6d0025840195d847d1e13775c6fecb660c65f79ab449209cab0d8589

Sorry I didn't notice this earlier. We should not be calculating our own web vitals data but either using the page-level data from httparchive.all.pages (field: payload._CrUX) or origin level data from the CrUX dataset, namely chrome-ux-report.materialized.device_summary.

@kevinfarrugia kevinfarrugia marked this pull request as ready for review October 7, 2024 10:44
@tunetheweb
Copy link
Member

@kevinfarrugia is this good to merge now?

tunetheweb
tunetheweb reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Overall LGTM but a count of comments. Let me know when good to merge.

sql/2024/performance/bfcache_cachecontrol_nostore.sql Show resolved Hide resolved
sql/2024/performance/bfcache_unload.sql Outdated Show resolved Hide resolved
sql/2024/performance/cls_animations.sql Outdated Show resolved Hide resolved
@tunetheweb tunetheweb merged commit 3d31ca3 into main Oct 30, 2024
3 checks passed
@tunetheweb tunetheweb deleted the performance-2024 branch October 30, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tunetheweb tunetheweb tunetheweb approved these changes

@mgifford mgifford Awaiting requested review from mgifford

Assignees
No one assigned
Labels
analysis Querying the dataset
Projects
None yet
Milestone
2024 Analysis
Development

Successfully merging this pull request may close these issues.
4 participants
@kevinfarrugia @mgifford @tunetheweb @guaca