Commit 9127fdc7 authored by Gerald Squelart's avatar Gerald Squelart
Browse files

Bug 1721569 - Make TracePayload easier to modify, and use maximum space for name - r=padenot 

An upcoming patch will change the size of ProfilerThreadId, which causes troubles with TracePayload.
Trying to move members around didn't help, I'm guessing some padding remains, which cannot be accounted for by just enumerating member sizes.

The proposed solution is to wrap all members in a macro (so they don't need to be repeated), and create a private struct with these members and a character, in order to find the exact offset at which the name will actually start.
This is uglier, but more flexible, and allows future changes (internal or external) more easily, without having to modify the name-size formula.

Differential Revision: https://phabricator.services.mozilla.com/D121050
parent 5d1937d5

dom/media/AsyncLogger.h

+32 −20
Original line number Diff line number Diff line
@@ -51,29 +51,41 @@ class AsyncLogger { 


  // The order of the fields is important here to minimize padding.

  struct TracePayload {

    // If this marker is of phase B or E (begin or end), this is the time at

    // which it was captured.

    TimeStamp mTimestamp;

    // If this marker is of phase X (COMPLETE), this holds the duration of the

    // event in microseconds. Else, the value is not used.

    uint32_t mDurationUs;

    // The thread on which this tracepoint was gathered.

    ProfilerThreadId mTID;

#define MEMBERS_EXCEPT_NAME                                                  \

  /* If this marker is of phase B or E (begin or end), this is the time at   \

   * which it was captured. */                                               \

  TimeStamp mTimestamp;                                                      \

  /* The thread on which this tracepoint was gathered. */                    \

  ProfilerThreadId mTID;                                                     \

  /* If this marker is of phase X (COMPLETE), this holds the duration of the \

   * event in microseconds. Else, the value is not used. */                  \

  uint32_t mDurationUs;                                                      \

  /* A trace payload can be either:                                          \

   * - Begin - this marks the beginning of a temporal region                 \

   * - End - this marks the end of a temporal region                         \

   * - Complete - this is a timestamp and a length, forming complete a       \

   * temporal region */                                                      \

  TracingPhase mPhase



    MEMBERS_EXCEPT_NAME;



   private:

    // Mock structure, to know where the first character of the name will be.

    struct MembersWithChar {

      MEMBERS_EXCEPT_NAME;

      char c;

    };

    static constexpr size_t scRemainingSpaceForName =

        PAYLOAD_TOTAL_SIZE - offsetof(MembersWithChar, c) -

        ((MPSC_MSG_RESERVERD + alignof(MembersWithChar) - 1) &

         ~(alignof(MembersWithChar) - 1));

#undef MEMBERS_EXCEPT_NAME



   public:

    // An arbitrary string, usually containing a function signature or a

    // recognizable tag of some sort, to be displayed when analyzing the

    // profile.

    char mName[PAYLOAD_TOTAL_SIZE - sizeof(TracingPhase) - sizeof(int) -

               sizeof(uint32_t) - sizeof(TimeStamp) -

               // Really, we'd want alignof(TracePayload), but it's not fully

               // declared yet. The alignment is going to be that of TimeStamp.

               ((MPSC_MSG_RESERVERD + alignof(TimeStamp) - 1) &

                ~(alignof(TimeStamp) - 1))];

    // A trace payload can be either:

    // - Begin - this marks the beginning of a temporal region

    // - End - this marks the end of a temporal region

    // - Complete - this is a timestamp and a length, forming complete a

    // temporal region

    TracingPhase mPhase;

    char mName[scRemainingSpaceForName];

  };



  // The goal here is to make it easy on the allocator. We pack a pointer in the