Closed Bug 1635442 Opened 5 years ago Closed 4 years ago

Only initialize LUL when the StackWalk feature is requested on Linux

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: gregtatum, Assigned: florian)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

LUL gets initialized, and is very slow to initialize, even when it's not needed. As far as I can tell, we only need it to do native stack walking. However, we can turn this feature off. This would be useful for Linux profiler tests, as they often time out compared to other platforms.

Here is where LUL is initialized: https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/tools/profiler/core/platform-linux-android.cpp#259

I see no feature checks though that we are actually doing native stack unwinding. If we disabled it, then Linux would become much faster when disabling stack walking.

Here is where that code is getting initialized:

https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/tools/profiler/core/platform.cpp#671

And here is the feature that needs to be checked that it is enabled: https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/tools/profiler/public/GeckoProfiler.h#166

Here is a profile with stackwalking disabled on Linux, where the initialization time can be seen: https://perfht.ml/2xCFHnh

I'm not sure if the code has other assumptions about LUL being present that I may be missing in my cursory check, but I think this is worth investigating and fixing.

We increased the timeout to a test in bug 1516311, we should probably reset it if we fix this bug.
But if other tests in the same directory need LUL, we might need to add the timeout back to those tests :-)

As shown in bug 1650157, LUL also uses a lot of memory in each process, which is especially bad when using Fission.

So it would be great if we could avoid initializing LUL when the profiler is not used; or as Greg suggested, when the profiler's stackwalk is not enabled. It may make starting the profiler the first time much slower, but I think it's better than imposing this impact on all Fission/Linux users.

I'm not familiar enough with the Linux side, and LUL in particular, so my assumption (that it's possible to delay LUL init until actually needed) may be wrong. Markus, could you please comment on this; do you have other suggestions, or know someone else who may know even more?
I'd be happy to tackle this if you say it's okay, or feel free to take it if you want+have the time to do it. 😉 (Or Greg, did you want to take it?)

Severity: -- → S2
Flags: needinfo?(mstange.moz)

I don't see problems with this suggestion. Initializing LUL when we start the profiler with stackwalking for the first time, rather than when we initialize the profiler, sounds good to me.

Flags: needinfo?(mstange.moz)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Pushed by fqueze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/beaf3344999f Only initialize LUL when the StackWalk feature is requested on Linux, r=gerald.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: