Closed Bug 1582741 Opened 7 months ago Closed 5 months ago

Balance the frees to the allocations in native allocation tracking

Categories

(Core :: Gecko Profiler, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: gregtatum, Assigned: gregtatum)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Right now in the native allocation tracking, we hook the allocations and deallocations. For each of these we perform a Bernoulli trial to sample them. This creates a situation where we can sample some allocation, but not sample its deallocation and vice versa. This makes it difficult to understand what is leaking in code.

https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/tools/profiler/core/memory_hooks.cpp

Instead, we could change the mechanism to retain a set of all the memory addresses we have created a marker for, and then only track the frees that are in that set. This would create a balanced view into the memory.

Blocks: 1592625
Assignee: nobody → gtatum

The bloat log was not compatible with the native allocations, and is always on
for debug builds of mochitests. We had no native allocation coverage on debug
builds because of it.

This commit rewrites the test as an xpcshell test which is both faster and simpler.
I don't think we need the added complexity of running the test in the full browser
environment. An xpcshell test fully excercises the code in a simpler fashion.

Depends on D51933

This patch creates a HashSet that tracks the allocations that are tracked by the profiler.
This way, we only collect markers for de-allocations that have a matching allocation. A
following commit makes it so that all of the markers are collected on the main thread, but
for now this is still done on a per-thread basis.

Depends on D51934

This file adds coverage for the balanced native allocations feature from the
previous commit. It asserts that a de-allocation will have a matching allocation.

Depends on D51935

This commit adds the memory address of the allocation and the thread id
of the allocation to the payload. These both are required for properly
processing the balanced allocations on the front-end. All of the native
allocation payloads are now stored on the main thread, and so are
disassociated with the thread where they were generated.

Depends on D51937

I had a test and eslint fail from the rebasing. I fixed that now.

Pushed by gtatum@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b48b1fa02fcc
Rewrite native allocations test as an xpcshell test; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/cd7174320d45
Balance the frees to the allocations in native allocation tracking; r=njn,gerald
https://hg.mozilla.org/integration/autoland/rev/308028db97ef
Create a test for balanced native allocation; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/fb009d42d012
Add values to the native allocation payload; r=gerald

Looks like we ran out of memory on windows, and asan overwrote the memory hooks. I'll try and decrease memory usage, and disable the test on asan builds.

Flags: needinfo?(gtatum)
Pushed by gtatum@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b1c0de5b727c
Rewrite native allocations test as an xpcshell test; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/b37cf462c341
Balance the frees to the allocations in native allocation tracking; r=njn,gerald
https://hg.mozilla.org/integration/autoland/rev/5991133b6ac7
Create a test for balanced native allocation; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/e2b8a34e2aac
Add values to the native allocation payload; r=gerald
Regressions: 1596761
You need to log in before you can comment on or make changes to this bug.