Closed Bug 1650157 Opened 4 years ago Closed 4 years ago

High LUL memory usage with the profiler and Fission

Categories

(Core :: Gecko Profiler, defect, P2)

defect

Tracking

()

RESOLVED WORKSFORME
Fission Milestone M7
Tracking Status
firefox80 --- wontfix

People

(Reporter: yoasif, Assigned: jseward)

References

Details

Attachments

(3 files)

Attached file memory-report.json.gz (deleted) —

Noticed Firefox taking up a lot of memory on my Linux machine. I am running a bunch of extensions in Nightly, but this is unusual.

Took a memory report in the hopes that we can investigate this further.

Thanks!

Attached file about:support (deleted) —

Andrew, could you please take a look over the memory report?
Thanks!

Flags: needinfo?(continuation)

Thanks for the report. It looks to me that the largest issue is that you have around 300MB or so of memory being used in every content process for the profiler. Because you have Fission enabled, there are a lot of processes.

Here's an excerpt from a typical process:

345.05 MB (100.0%) -- explicit
├──322.71 MB (93.53%) -- profiler
│ ├──211.21 MB (61.21%) ── lul
│ └──111.50 MB (32.32%) ── profiler-state

Component: Untriaged → Gecko Profiler
Flags: needinfo?(continuation)
Product: Firefox → Core
Summary: Large amount of memory use while browsing → High memory usage with the profiler and Fission

This makes bug 1635442 more important now.

Severity: -- → S2
Depends on: 1635442
Priority: -- → P2

Julian, can we place lul data in shared memory? kmag says lul data is duplicated in each content process. Fission site isolation can use many more content processes than e10s, so profiler memory usage is a problem with Fission.

This bug doesn't need to block Fission Nightly. Tracking for Fission riding the trains to Beta (M7).

Fission Milestone: --- → M7
Flags: needinfo?(jseward)

That's one option. Bug 1635811 proposes another option: caching the lul data in a file. "[...] If we map the same cache file into each process, this would also save memory."

I should point out that (unless it has been changed since the initial implementation), the way that the unwind information is stored in LUL is grossly space-inefficient. We could do very much better. I wrote a blog post [1] about it, back in the day. So maybe implementing that would help too.

[1] https://blog.mozilla.org/jseward/2013/09/03/how-compactly-can-cfiexidx-stack-unwinding-info-be-represented/

Flags: needinfo?(jseward)

But to answer your question directly: the lul data is readonly (once it has been read from the .so file, and we're in "unwinding" mode), so in that aspect there's no problem in putting it in shared memory. However, the same .so file will normally be mapped at different virtual addresses in different processes. And I can't remember offhand whether the lul data is phrased in terms of offsets from the start of .so's, or whether it has the absolute virtual addresses baked in. In the latter case, it would then not be sharable across processes. I'd have to look.

However, even in the latter case, it would be pretty simply to refactor the info to use the offset-based scheme.

All of which is a long way of saying: yes we should be able to share it across processes; there may (or may not) be a need to do a bit of refactoring though.

Left to myself, though, I would first follow the line of enquiry suggested in comment 7. IMO there are huge wins there and they are easy to do. Large fruit hanging very low, etc.

(In reply to Julian Seward [:jseward] from comment #8)

All of which is a long way of saying: yes we should be able to share it across processes; there may (or may not) be a need to do a bit of refactoring though.

Left to myself, though, I would first follow the line of enquiry suggested in comment 7. IMO there are huge wins there and they are easy to do. Large fruit hanging very low, etc.

Thanks. Summary of the three suggestions so far:

  1. optimize unwind info to be more space efficient (comment 7)
  2. map unwind info in parent process' shared memory (comment 0)
  3. cache unwind info in a file (comment 6, pointing to bug 1635811), though comments suggest content process sandboxing might make sharing the cache file difficult.
Severity: S2 → S3
Summary: High memory usage with the profiler and Fission → High LUL memory usage with the profiler and Fission

And

  1. Don't initialize LUL until somebody starts profiling (comment 4).

Wait, I guess we're already doing that. Bug 1635442 is just asking for a way to do profiling without LUL (and without stacks). Never mind then.

Assigning to Julian. He said he is available to help reduce LUL memory usage for one week in Q3 and one week in Q4.

He will try to optimize unwind info to be more space efficient (the approach suggested in comment 7), not map unwind info in parent process' shared memory (comment 0) or a file (comment 6). If profiler memory usage is still a problem after he optimizes unwind info, we can then look into also using shared memory.

Assignee: nobody → jseward
Attached patch lul-space.diff-5 (deleted) — Splinter Review

Here's my current WIP patch. It's pretty complete, I think, but currently I have only tested it on x86_64-linux. It reduces the memory consumption of the LUL component, as shown in about:memory, by almost a factor of 6 (so, here, from 211 MB down to about 35.2 MB) and can fairly reliably be assumed to do the same independent of the particular build flags etc used. Feedback welcomed.

Next week I'll test it on other platforms and try to get something reviewable available.

Depends on: 1665029

Patch moved to bug 1665029.

Asif, do you still Nightly taking up lots more memory with Fission than without Fission? Were you using the Firefox Profiler when you saved your memory report?

A memory usage optimization for Firefox Profiler's memory usage (bug 1665029) landed last week.

Flags: needinfo?(yoasif)

Chris, I may have been running the profiler just to get a handle on what was happening (nothing really visible). Unfortunately, Firefox crashed very soon after I attempted to capture the profile (I think it may have been the OOM-killer in Linux).

I haven't seen some of the extremely bad behavior in a few days, but I am not sure if it is a coincidence. I'm still running with Fission enabled though, and will be happy to share another memory report if it seems bad (along with a profile, if I can manage to capture it).

Flags: needinfo?(yoasif)

Closing as WORKSFORME. I assume profiler memory usage is good enough now that Julian's improvements in bug 1665029 have landed. If not, please file a new bug.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: