Bug 217853 - Add system trace points for flushing remote image buffers
Summary: Add system trace points for flushing remote image buffers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-16 15:59 PDT by Wenson Hsieh
Modified: 2020-10-16 21:56 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.38 KB, patch)
2020-10-16 16:50 PDT, Wenson Hsieh
simon.fraser: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (5.38 KB, patch)
2020-10-16 20:40 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2020-10-16 15:59:13 PDT
(Also, add it in WebKit2 layer so that we don't end up calling kdebug_trace millions of times throughout the course of the Design subtest).
Comment 1 Wenson Hsieh 2020-10-16 16:50:21 PDT
Created attachment 411631 [details]
Patch
Comment 2 Wenson Hsieh 2020-10-16 19:00:20 PDT
Comment on attachment 411631 [details]
Patch

EWS failure seems to be due to flaky tests.
Comment 3 Simon Fraser (smfr) 2020-10-16 19:58:27 PDT
Comment on attachment 411631 [details]
Patch

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

> Source/WTF/wtf/SystemTracing.h:118
> +    FlushRemoteImageBufferStart,
> +    FlushRemoteImageBufferEnd,

I'm not too hot on the "flush image buffer" terminology.
Comment 4 Wenson Hsieh 2020-10-16 20:04:33 PDT
Comment on attachment 411631 [details]
Patch

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

>> Source/WTF/wtf/SystemTracing.h:118
>> +    FlushRemoteImageBufferEnd,
> 
> I'm not too hot on the "flush image buffer" terminology.

How about "FlushDisplayListStart" and "FlushDisplayListEnd"?
Comment 5 Simon Fraser (smfr) 2020-10-16 20:26:41 PDT
Comment on attachment 411631 [details]
Patch

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

>>> Source/WTF/wtf/SystemTracing.h:118
>>> +    FlushRemoteImageBufferEnd,
>> 
>> I'm not too hot on the "flush image buffer" terminology.
> 
> How about "FlushDisplayListStart" and "FlushDisplayListEnd"?

The trace points encompass IPC encoding of the DL, and async dispatch forIPC and displayList.clear(), right? Do we want to track all those, or just the encoding? Or do we really want to track flushDrawingContext/flushDrawingContextAndWaitCommit ?
Comment 6 Wenson Hsieh 2020-10-16 20:29:57 PDT
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 411631 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411631&action=review
> 
> >>> Source/WTF/wtf/SystemTracing.h:118
> >>> +    FlushRemoteImageBufferEnd,
> >> 
> >> I'm not too hot on the "flush image buffer" terminology.
> > 
> > How about "FlushDisplayListStart" and "FlushDisplayListEnd"?
> 
> The trace points encompass IPC encoding of the DL, and async dispatch forIPC
> and displayList.clear(), right? Do we want to track all those, or just the
> encoding? Or do we really want to track
> flushDrawingContext/flushDrawingContextAndWaitCommit ?

My intention was to track flushDrawingContext/flushDrawingContextAndWaitCommit in the remote image buffer case.
Comment 7 Simon Fraser (smfr) 2020-10-16 20:32:25 PDT
Maybe what you have is fine then. Maybe set data1 to 1 in the "and wait" case so we can tell them apart.
Comment 8 Wenson Hsieh 2020-10-16 20:37:07 PDT
(In reply to Simon Fraser (smfr) from comment #7)
> Maybe what you have is fine then. Maybe set data1 to 1 in the "and wait"
> case so we can tell them apart.

Sounds good — changed the second trace scope to `TraceScope tracingScope(FlushRemoteImageBufferStart, FlushRemoteImageBufferEnd, 1);`.
Comment 9 Wenson Hsieh 2020-10-16 20:40:24 PDT
Created attachment 411650 [details]
Patch for landing
Comment 10 EWS 2020-10-16 21:55:38 PDT
Committed r268636: <https://trac.webkit.org/changeset/268636>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411650 [details].
Comment 11 Radar WebKit Bug Importer 2020-10-16 21:56:15 PDT
<rdar://problem/70402506>