1533195 - "hot enough" profiler functions may benefit from being inlined?

Status: NEW

(Core :: Gecko Profiler, enhancement, P5)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://searchfox.org/mozilla-central/search?q=function+is+hot+enough+that+we+use+RacyFeatures&path=
shows four functions in platform.cpp that use RacyFeatures::IsActiveWith..., so that we quickly check an atomic flag (to know if the profiler is currently working), instead of locking a mutex, which could block for a while.

Because these functions are in the platform.cpp compilation unit, they cannot be inlined when called from other units, which means a bit of extra time&memory is spent calling the function.

It may well be negligible, but if these calls are indeed "hot enough", we should investigate a bit further, and see if we could easily move them to GeckoProfiler.h where they could then be inlined into their "hot" callers.

As an extra benefit, this would make the atomic optimization more visible to devs looking only at GeckoProfiler.h.

Comment 1

[(Away)]

Because these functions are in the platform.cpp compilation unit, they
cannot be inlined when called from other units, which means a bit of extra
time&memory is spent calling the function.

Any build that we're doing profiling on should have LTO enabled, which allows inlining to happen across translation units.

Do you have reason to believe that this is not happening and that it measurably affects perf?

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to David Major [:dmajor] from comment #1)

Because these functions are in the platform.cpp compilation unit, they
cannot be inlined when called from other units, which means a bit of extra
time&memory is spent calling the function.

Any build that we're doing profiling on should have LTO enabled, which allows inlining to happen across translation units.

Do you have reason to believe that this is not happening and that it measurably affects perf?

Thanks for the info David. So this bug is probably "invalid" then.

Is there a way to verify that? (Apart from looking at the asm?)

Also, with the work on moving parts of the profiler to mozglue, would that LTO inlining still work from libxul code?

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to David Major [:dmajor] from comment #1)

Do you have reason to believe that this is not happening and that it measurably affects perf?

And no, I don't have reasons to believe there's a measurable impact.
So after checking if the LTO inlining happens, measuring the impact would be the next step.

Lowering bug priority anyway, as it's unlikely to be important.