Closed Bug 1501442 Opened 2 years ago Closed 2 years ago

Add method of adding timing information for transaction contents

Categories

(Core :: Graphics, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

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

References

Details

Attachments

(3 files, 4 obsolete files)

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.
Attachment #9019490 - Attachment is obsolete: true
Attachment #9019490 - Flags: review?(mstange)
Attachment #9019500 - Flags: review?(mstange)
Attachment #9019500 - Attachment is obsolete: true
Attachment #9019500 - Flags: review?(mstange)
Attachment #9019504 - Flags: review?(mstange)
Priority: -- → P3
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+
Blocks: 1506537
WR will need some sort of equivelant of this.
Attachment #9024405 - Flags: review?(mstange)
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)
Attachment #9024405 - Flags: review?(rhunt) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.