Add method of adding timing information for transaction contents

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
5 months ago

People

(Reporter: bas.schouten, Assigned: bas.schouten)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(3 attachments, 4 obsolete attachments)

Assignee

Description

8 months ago
As part of a transaction, and therefor as part of a composition, we generally submit a certain amount of visual data to the compositor that was generated at certain points in the past. By being able to associate this contents and the time it was generated with a composition we'd be able to fairly accurately determine the total latency from generation of the event (for example user input), to the result being shown on the screen (i.e. max one frame after composition takes place).

I've discussed this with Markus and I have some ideas on an infrastructure to do this.
Assignee

Comment 2

8 months ago
Attachment #9019490 - Attachment is obsolete: true
Attachment #9019490 - Flags: review?(mstange)
Attachment #9019500 - Flags: review?(mstange)
Assignee

Comment 3

8 months ago
Attachment #9019500 - Attachment is obsolete: true
Attachment #9019500 - Flags: review?(mstange)
Attachment #9019504 - Flags: review?(mstange)
Priority: -- → P3
Assignee

Updated

7 months ago
Blocks: 1505256
Comment on attachment 9019504 [details] [diff] [review]
Part 1: Add the ability to register CompositionPayloads and forward through transactions v3

Review of attachment 9019504 [details] [diff] [review]:
-----------------------------------------------------------------

The approach looks good to me.

::: gfx/layers/Layers.h
@@ +730,5 @@
>  
>    virtual CompositorBridgeChild* GetCompositorBridgeChild() { return nullptr; }
>  
> +  void RegisterPayload(const CompositionPayload& aPayload) { mPayload.AppendElement(aPayload); MOZ_ASSERT(mPayload.Length() < 10000); }
> +  void RegisterPayload(const InfallibleTArray<CompositionPayload>& aPayload) { mPayload.AppendElements(aPayload); MOZ_ASSERT(mPayload.Length() < 10000); }

some long lines here

::: gfx/layers/LayersTypes.h
@@ +472,5 @@
> +  bool operator ==(const CompositionPayload& aOther) const {
> +    return mType == aOther.mType && mTimeStamp == aOther.mTimeStamp;
> +  }
> +  /* The type of payload that is in this composition */
> +  CompositionPayloadType mType; 

end-of-line whitespace

::: gfx/layers/ipc/LayersMessageUtils.h
@@ +666,5 @@
>  {};
>  
> +template <>
> +struct ParamTraits<mozilla::layers::CompositionPayload>
> +  : public PlainOldDataSerializer<mozilla::layers::CompositionPayload>

Let's actually handwrite the serialization for this. That'll give us protection against invalid enum values from compromised content processes, too, and it would make me feel better about correct TimeStamp serialization.
Attachment #9019504 - Flags: review?(mstange) → review+
Assignee

Updated

7 months ago
Blocks: 1506537
Assignee

Comment 7

7 months ago
WR will need some sort of equivelant of this.
Attachment #9024405 - Flags: review?(mstange)
Assignee

Comment 8

7 months ago
This works, although perhaps something more specialized is desirable here where the payload is stored with the Composite trace marker and displayed in the profiler sidebar, I'll let you decide what you want to do here Markus, if you provide me with pointers I can implement something better.
Attachment #9024483 - Flags: review?(mstange)
Blocks: 1506976
Attachment #9024483 - Flags: review?(mstange) → review+
Attachment #9024405 - Flags: review?(mstange) → review?(rhunt)

Updated

7 months ago
Attachment #9024405 - Flags: review?(rhunt) → review+

Comment 9

5 months ago
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c1fade7ceeb
Part 1: Add CompositionPayload type and allow submitting it as part of a transaction. r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/76070b4feec3
Part 2: Propagate composition payload from clients to host. r=rhunt
https://hg.mozilla.org/integration/mozilla-inbound/rev/a123204b6b0d
Part 3: Add profiler markers for payload presentarion. r=mstange

Comment 10

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.