Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upArbitrarily high physical memory usage while scrolling #4915
Comments
|
Whoa: I used GDB to see where the mmap calls are occurring and they all seem to be coming from jemalloc. So I now suspect that the |
|
I've confirmed that the current jemalloc measurements are bogus. #4917 has a fix. With that fix applied, it's clear that the increase is coming jemalloc, i.e. from allocations within Rust code. Here's a particularly horrifying case I got with wikipedia: |
|
@pcwalton I don't know if you're following this investigation, but it looks like we have a horrific number of allocations coming from Rust code during scrolling. |
|
I'm hoping to get some detailed profiling data next week that will identify which parts of the Rust code are responsible. |
|
I built a more fine-grained memory profiler out of balsa wood, rubber bands, and GDB. I've used it to find a few things. At first I thought the increasing RSS was due to traversal.rs's bloom filters being cloned frequently (#4938) but that appears to just cause heap churn without increasing memory usage. I'm now most suspicious of this stack trace, which occurred 128 times, allocating 4 MiB each time, while scrolling once through the Guardians of the Galaxy page. This may be misleading because this stack trace may only be responsible for a small fraction of each 4 MiB chunk. However, there were only 439 such chunks allocated during the page scrolling, so it seems likely that this really is accounting for a decent fraction (~128/439) of allocations. It's not clear that these allocations are long-lived, but it's my best lead at the moment. |
|
In the same run, the following stack trace also showed up 29 times (out of the 439 chunks). |
|
That last one is just the code that gives threads names :( |
|
Is it possible the default minimum allocation size for jemalloc is 4MB? If so, it would make sense that it's happening in the code that gives the threads names, as that may be the first thing we do after spawning the pthread that executes allocating rust code. As to the other one, it looks like the source of those allocations is probably servo/components/compositing/compositor.rs Lines 902 to 905 in 66f4faf handle_window_message in the stack above). I wonder if we're generating a bazillion scroll events...
|
|
I just want to add that you can also create arbitrary amounts of memory usage by just moving the mouse inside the window. It doesn't seem to be a specific scrolling issue. |
|
If jemalloc is using thread-local heaps, as the |
|
glandium confirmed for me that the |
I added a debugging println and only got ~1,000 when scrolling through the GotG page, which doesn't seem that high. |
|
I just discovered that the jemalloc used by Rust has good Valgrind hooks Which is the same one I was suspicous about yesterday. I did some more digging with println!() statements and determined that the First: Second: There's obviously a lot of badness going on here, but one thing that particularly concerns me is that the RSS never goes down. |
|
I noticed that this issue affects only some sites. Mouse movement on en.wikipedia.org causes high memory usage; Hacker News doesn't show this behavior. Probably the main difference between these test cases is that mouse movement on Wikipedia causes all the text to move by a few pixels (issue #2894). |
|
One other thing I didn't mention: |
|
We should probably throttle those events. |
|
I did a Massif run where I quit Servo via ctrl-c rather than clicking in the Window's 'X' button. This avoids the generation of any And that means the memory increase caused by mouse moves have another cause. I don't have more data about that yet. Finally, I tried changing |
|
I just learned that Servo builds are unoptimized by default, so I tried an optimized (release) build. The memory increase while moving the mouse is substantially faster in an optimized build, but I have trouble reproducing the spike when quitting -- quitting is mostly instantaneous (or nearly) now. |
|
I did some more profiling with Massif on an optimized build. I kept moving the Here are the five allocation stacks that account for the most live memory just I looked closely at all of them but don't understand the code well enough to |
Looking at the code for this one, it appears that this is all safe code. A new Arc is created for the style, then cloned and passed to the cache. Finally it is inserted into the mutable function argument. This ends up being stored in layout_data for the node, which is manipulated unsafely. So perhaps we're never freeing layout_data.shared_data? |
|
That should happen as part of layout data reaping in the Node destructor. |
|
I found that tests/ref/inline_block_img_a.html is a reduced test case for this. It leaks every flow every time and none of them ever get freed. It has 6 total flows. |
Cycles were being created in the flow tree since absolutely positioned descendants had pointers to their containing blocks. This adds WeakFlowRef (based on Weak<T>) and makes these backpointers weak references. This also harmonizes our custom Arc<T>, FlowRef, to be consistent with the upstream implementation. Fixes servo#4915.
Cycles were being created in the flow tree since absolutely positioned descendants had pointers to their containing blocks. This adds WeakFlowRef (based on Weak<T>) and makes these backpointers weak references. This also harmonizes our custom Arc<T>, FlowRef, to be consistent with the upstream implementation. Fixes servo#4915.
Cycles were being created in the flow tree since absolutely positioned descendants had pointers to their containing blocks. This adds WeakFlowRef (based on Weak<T>) and makes these backpointers weak references. This also harmonizes our custom Arc<T>, FlowRef, to be consistent with the upstream implementation. Fixes servo#4915.
Cycles were being created in the flow tree since absolutely positioned descendants had pointers to their containing blocks. This adds WeakFlowRef (based on Weak<T>) and makes these backpointers weak references. This also harmonizes our custom Arc<T>, FlowRef, to be consistent with the upstream implementation. Fixes #4915.
|
I retested after updating and I can confirm the leak is fixed. On the GotG page the "jemalloc-heap-allocated" measurement is bouncing around in the 61--65 MiB range instead of shooting for the moon. Excellent. |
I should add: before this fix, "jemalloc-heap-allocated" reached 340--380 MiB as soon as the page finished loading, before even wiggling the mouse at all. So the baseline memory usage is a lot lower as well. Double-excellent. |
On both Linux and Mac I can get physical memory usage to become arbitrarily high by repeatedly scrolling up and down the "Guardians of the Galaxy" wikipedia page from servo-static-suite. Here's sample -m output on Linux (with my patches from #4894 applied):
The relatively low
system-heap-allocatedandjemalloc-heap-allocatednumbers indicate that the memory is not on either heap, so it must be coming from mmap(). I did some measurement withvmmapon Mac and it gave me similar indications that it's not heap memory.I normally use Massif to investigate such cases but I'm getting various task failures when I run Servo under Massif and view this page, alas.