Bug 219141

Summary: [macOS] Add telemetry for things we believe are unused
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, ggaren, jonlee, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Description Brent Fulgham 2020-11-18 19:00:07 PST 
This patch adds telemetry to a set of sandbox features we believe are unused. We should measure, and confirm before removing.
Comment 1 Radar WebKit Bug Importer 2020-11-18 20:02:19 PST 
<rdar://problem/71569084>
Comment 2 Brent Fulgham 2020-11-18 21:24:38 PST 
Created attachment 414538 [details]
Patch
Comment 3 Per Arne Vollan 2020-11-19 11:14:30 PST 
Comment on attachment 414538 [details]
Patch

R=me. It might be good to make sure this does not introduce MotionMark regressions before landing, since adding reports for IOKit classes sometimes introduces performance regressions.
Comment 4 Brent Fulgham 2020-11-20 11:08:05 PST 
Created attachment 414699 [details]
Patch
Comment 5 Per Arne Vollan 2020-11-20 11:49:00 PST 
Comment on attachment 414699 [details]
Patch

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

R=me.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:121
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000

Does this rule already have telemetry?

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:140
> +        (with telemetry-backtrace)

Does this rule already have telemetry?

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:174
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000

Does this rule also already have telemetry?
Comment 6 Brent Fulgham 2020-12-01 10:10:39 PST 
Comment on attachment 414699 [details]
Patch

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

>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:121
>> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000
> 
> Does this rule already have telemetry?

You are right! I'll correct all of them like this.
Comment 7 Brent Fulgham 2020-12-01 10:11:51 PST 
Created attachment 415145 [details]
Patch for landing
Comment 8 Brent Fulgham 2020-12-01 10:12:35 PST 
Created attachment 415146 [details]
Patch for landing
Comment 9 Alex Christensen 2020-12-01 10:29:26 PST 
Comment on attachment 415146 [details]
Patch for landing

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

> Source/WebKit/ChangeLog:3
> +        [macOS] Add telemetry for things we believe are unused

It would be nice if this title had "sandbox" in it.  "things" isn't very helpful.
Comment 10 Brent Fulgham 2020-12-01 10:41:25 PST 
Created attachment 415148 [details]
Patch for landing
Comment 11 Brent Fulgham 2020-12-01 10:42:26 PST 
Comment on attachment 415146 [details]
Patch for landing

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

>> Source/WebKit/ChangeLog:3
>> +        [macOS] Add telemetry for things we believe are unused
> 
> It would be nice if this title had "sandbox" in it.  "things" isn't very helpful.

Good point. I *think* I caught commit-queue before it landed.
Comment 12 EWS 2020-12-01 11:09:04 PST 
Committed r270318: <https://trac.webkit.org/changeset/270318>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415148 [details].