Bug 183218 - Add a second tier of prevalence to facilitate telemetry on very prevalent domains
Summary: Add a second tier of prevalence to facilitate telemetry on very prevalent dom...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-28 11:09 PST by John Wilander
Modified: 2018-03-23 14:02 PDT (History)
7 users (show)

See Also:


Attachments
Patch (47.51 KB, patch)
2018-02-28 11:35 PST, John Wilander
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (11.53 MB, application/zip)
2018-02-28 13:37 PST, EWS Watchlist
no flags Details
Patch for landing (47.56 KB, patch)
2018-02-28 13:57 PST, John Wilander
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 2018-02-28 11:09:16 PST
We should have a second tier for very prevalent resources. This allows for more detailed telemetry but also for potential UI with "top trackers."
Comment 1 Radar WebKit Bug Importer 2018-02-28 11:09:54 PST
<rdar://problem/37992389>
Comment 2 Radar WebKit Bug Importer 2018-02-28 11:09:54 PST
<rdar://problem/37992388>
Comment 3 John Wilander 2018-02-28 11:15:50 PST
I will use <rdar://problem/37992388> as the radar equivalent.
Comment 4 John Wilander 2018-02-28 11:35:44 PST
Created attachment 334755 [details]
Patch
Comment 5 Brent Fulgham 2018-02-28 12:01:29 PST
Comment on attachment 334755 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334755&action=review

r=me

> Source/WebKit/Platform/classifier/ResourceLoadStatisticsClassifier.cpp:37
> +    return sqrt(a * a + b * b + c * c);

This could probably be "std::hypot(a, b, c)", though I think this is known to be faster to execute.

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:532
> +{

ASSERT(isMainThread());

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:547
> +        resourceStatistic.isVeryPrevalentResource = true;

resourceStatistic.isVeryPrevalentResource = newPrevlance == ResourceLoadPrevalence::VeryHigh;

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:576
> +{

ASSERT(isMainThread());
Comment 6 Brent Fulgham 2018-02-28 12:02:04 PST
Comment on attachment 334755 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334755&action=review

>> Source/WebKit/Platform/classifier/ResourceLoadStatisticsClassifier.cpp:37
>> +    return sqrt(a * a + b * b + c * c);
> 
> This could probably be "std::hypot(a, b, c)", though I think this is known to be faster to execute.

I mean that YOUR code is known to be more efficient (if not as able to handle over/underflow).
Comment 7 EWS Watchlist 2018-02-28 13:36:58 PST
Comment on attachment 334755 [details]
Patch

Attachment 334755 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6708809

New failing tests:
http/tests/preload/download_resources.html
Comment 8 EWS Watchlist 2018-02-28 13:37:09 PST
Created attachment 334763 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 9 John Wilander 2018-02-28 13:57:38 PST
Created attachment 334766 [details]
Patch for landing
Comment 10 John Wilander 2018-02-28 13:58:47 PST
Thanks, Brent! Fixed all the things you commented on.
Comment 11 WebKit Commit Bot 2018-02-28 14:25:32 PST
Comment on attachment 334766 [details]
Patch for landing

Clearing flags on attachment: 334766

Committed r229106: <https://trac.webkit.org/changeset/229106>
Comment 12 WebKit Commit Bot 2018-02-28 14:25:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Chris Dumez 2018-02-28 16:37:15 PST
Comment on attachment 334766 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=334766&action=review

> Source/WebKit/Platform/classifier/ResourceLoadStatisticsClassifier.cpp:55
> +        ASSERT(currentPrevalence == Low);

I am hitting this assertion in debug wk2.
Comment 14 John Wilander 2018-02-28 16:55:47 PST
(In reply to Chris Dumez from comment #13)
> Comment on attachment 334766 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334766&action=review
> 
> > Source/WebKit/Platform/classifier/ResourceLoadStatisticsClassifier.cpp:55
> > +        ASSERT(currentPrevalence == Low);
> 
> I am hitting this assertion in debug wk2.

Thanks for the report, Chris. I removed the assertion in https://bugs.webkit.org/show_bug.cgi?id=183233.