1699742 - Remove MOZ_GECKO_PROFILER ifdefs that are no longer needed after bug 1698493

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P3)

Description

[Florian Quèze [:florian]]

Most MOZ_GECKO_PROFILER ifdefs are no longer needed now that functions like profiler_can_accept_markers are declared even when the profiler isn't built.

The patch I'm going to submit here removes lots of ifdefs. Some are still left, mostly for 2 reasons:

• marker code that uses MarkerSchema. I think this should not require ifdefs eventually, but changes to our profiler marker headers are needed for this, so I think it's better to handle this as a separate bug.
• code that starts/stops the profiler, or handles profiler IPC. I think the ifdefs are fine for these cases and could remain.

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 1699742 - Remove MOZ_GECKO_PROFILER ifdefs that are no longer needed, r=gerald.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e91e6c6edfa
Remove MOZ_GECKO_PROFILER ifdefs that are no longer needed, r=gerald.

Comment 3

[Simon Giesecke [:sg] [he/him]]

First of all, getting rid of the #ifdef is nice for readability, where this doesn't affect the generated code!

However, I wonder if the patch is removing #ifdef too eagerly, where this now results in member variables being added that are essentially unused if MOZ_GECKO_PROFILER is not defined. E.g. https://searchfox.org/mozilla-central/rev/4e87b5392eafe1f1d49017e76f7317b06ec0b1d8/layout/base/nsRefreshDriver.h#430-432.

Comment 4

[Simon Giesecke [:sg] [he/him]]

Ah, I missed that the plan is to remove MOZ_GECKO_PROFILER and stop supporting builds without profiler. If that's the plan, this probably doesn't matter. I was assuming that non-profiler builds might still be desirable for some reason.

Comment 5

[Florian Quèze [:florian]]

(In reply to Simon Giesecke [:sg] [he/him] from comment #4)

Ah, I missed that the plan is to remove MOZ_GECKO_PROFILER and stop supporting builds without profiler. If that's the plan, this probably doesn't matter. I was assuming that non-profiler builds might still be desirable for some reason.

Non-MOZ_GECKO_PROFILER builds should only exist on Tier3 platforms or architectures. You are correct that we will waste a little bit of memory there, but that's memory that's used on all the other supported platforms, so it shouldn't represent unacceptable overhead.

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5e91e6c6edfa