Closed Bug 1375776 Opened 2 years ago Closed 2 years ago

Reduce usage of MOZ_GECKO_PROFILER some more

Categories

(Core :: Gecko Profiler, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(7 files)

Because it's still worth reducing the number of places that check MOZ_GECKO_PROFILER.
The JS Engine now has a PseudoStack pointer, rather than a pointer and two
sizes.
Attachment #8882950 - Flags: review?(shu)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
No point having all these explicit empty destructors.

Also, we can avoid IOMarkerPayload's constructor by using a UniqueFreePtr.
Attachment #8882952 - Flags: review?(mstange)
This requires:

- Moving the constructors of ProfilerMarkerPayload and its subclasses into the
  .h file so they are visible even when ProfilerMarkerPayload.cpp isn't
  compiled.

- Similarly, using a macro to make StreamPayload() a crashing no-op when the
  profiler isn't enabled. (It is never called in that case.)
Attachment #8882953 - Flags: review?(mstange)
This makes it more like all the other payload classes.
Attachment #8882954 - Flags: review?(mstange)
If these timings are on for all MOZ_GECKO_PROFILER platforms (i.e. all tier 1
platforms) then we might as well simplify things by having them on for all
platforms.
Attachment #8882955 - Flags: review?(sphink)
Attachment #8882956 - Flags: review?(mstange)
Attachment #8882955 - Flags: review?(sphink) → review+
Comment on attachment 8882950 [details] [diff] [review]
(part 1) - Fix a comment about the PseudoStack

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

Thank you, meticulous as always.
Attachment #8882950 - Flags: review?(shu) → review+
Attachment #8882951 - Flags: review?(mstange) → review+
Comment on attachment 8882952 [details] [diff] [review]
(part 3) - Improve destructor of ProfilerMarkerPayload and its subclasses

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

Having out-of-line empty destructors can sometimes help with reducing the number of includes in a header file. E.g. if your class has a UniquePtr<SomeClass> mMember, the place where you destroy mMember determines whether you need to #include "SomeClass.h" in the header or in the cpp file (so that the UniquePtr destructor can call ~SomeClass).
But it doesn't seem like that concern applies here.
Attachment #8882952 - Flags: review?(mstange) → review+
Attachment #8882953 - Flags: review?(mstange) → review+
Attachment #8882954 - Flags: review?(mstange) → review+
Attachment #8882956 - Flags: review?(mstange) → review+
Depends on: 1378312
Duplicate of this bug: 1365439
You need to log in before you can comment on or make changes to this bug.