Closed Bug 1698493 Opened 1 year ago Closed 1 year ago

ProfilerState.h should define no-op inline functions to avoid MOZ_GECKO_PROFILER ifdefs in code adding markers

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
88 Branch
Tracking Status
firefox88 --- fixed

People

(Reporter: florian, Assigned: florian)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Some functions are used by code adding markers, and require adding ifdef around this code, which is annoying (and error prone).

As discussed in bug 1635350, I think adding some inline functions returning false in ProfilerState.h would simplify the code adding markers.

In the network code, we use profiler_can_accept_markers to avoid doing expensive work when recording markers, it would be a good idea to return false when the profiler isn't compiled in. I believe that this alone would avoid using the ifdef in that code.
(update: I would also need a noop for the adding marker functions)

Severity: -- → N/A
Priority: -- → P2

(In reply to Julien Wajsberg [:julienw] from comment #1)

In the network code, we use profiler_can_accept_markers to avoid doing expensive work when recording markers, it would be a good idea to return false when the profiler isn't compiled in. I believe that this alone would avoid using the ifdef in that code.

My hope is that this simple patch alone should be enough to let us remove most MOZ_GECKO_PROFILER ifdefs that are outside of tools/profiler and mozglue/baseprofiler.

(update: I would also need a noop for the adding marker functions)

I mentioned in my review of https://phabricator.services.mozilla.com/D104587 that profiler_add_network_marker should be moved out of GeckoProfiler.h, to be somewhere in netwerk/protocol/http/. Hopefully it can call a marker API that always exists and is a no-op when the profiler isn't built (that probably just means using the PROFILER_MARKER macro instead of calling profiler_add_marker directly.
Gerald opened bug 1692929 for the follow-ups to that patch.

Assignee: nobody → florian
Status: NEW → ASSIGNED
Pushed by fqueze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e27b36cffb75
ProfilerState.h should define no-op inline functions to avoid MOZ_GECKO_PROFILER ifdefs in code adding markers, r=gerald.
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
Regressions: 1699036
Blocks: 1699742
You need to log in before you can comment on or make changes to this bug.