Closed Bug 1576819 Opened 1 year ago Closed 11 months ago

Add PROFILER_ADD_MARKER_WITH_PAYLOAD, use it and PROFILER_ADD_MARKER everywhere

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

Details

Attachments

(1 file)

Greg suggested in his review of bug 1576550, that the extra timing routine before each profiler_add_marker() call could instead be added to the existing PROFILER_ADD_MARKER macro.

Unfortunately, PROFILER_ADD_MARKER only accepts two arguments, and it's not used everywhere anyway... yet!

So I think we could:

  • Use PROFILER_ADD_MARKER for all two-argument calls to profiler_add_marker() -- this will be more consistent, and will also save our users from having to fully qualify the category pair.
  • Add PROFILER_ADD_MARKER_WITH_PAYLOAD, which will take an additional payload type and the list of constructor arguments -- this will also be more consistent, but also make it easier to change to stack-allocated payloads in bug 1576555.

With these, it will be trivial to add the internal profiling instrumentation in bug 1576550.

I will keep the first review separate from the patch stack starting at bug 1576550, just in case this change here is not accepted or needs significant changes (so I don't waste time merging these patches if I then have to revert/remerge them).

All calls to profiler_add_marker() (outside of the profilers code) are
now replaced by either:

  • PROFILER_ADD_MARKER(name, categoryPair)
  • PROFILER_ADD_MARKER_WITH_PAYLOAD(name, categoryPair, TypeOfMarkerPayload, (payload, arguments))

This makes all calls consistent, and they won't need to prefix the category pair
with JS::ProfilingCategoryPair::.

Also it will make it easier to add (and later remove) internal-profiling
instrumentation (bug 1576550), and soon to replace heap-allocated payloads with
stack-allocated ones (bug 1576555).

Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49471b9a3a99
Use PROFILER_ADD_MARKER{,_WITH_PAYLOAD} everywhere - r=gregtatum
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.