This is an archive of the discontinued LLVM Phabricator instance.

[tsan] Implement WriteMemoryProfile for Darwin
ClosedPublic

Authored by kubamracek on Oct 25 2016, 4:58 PM.

Details

Summary

TSan’s memory usage profiling currently doesn’t work on Darwin. This patch implements measuring the amount of resident and dirty memory for each memory region. I also removed the GetShadowMemoryConsumption function, which seems to be unused.

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek updated this revision to Diff 75818.Oct 25 2016, 4:58 PM
kubamracek retitled this revision from to [tsan] Implement WriteMemoryProfile for Darwin.
kubamracek updated this object.
kubamracek added reviewers: dvyukov, zaks.anna, kcc, eugenis.
kubamracek set the repository for this revision to rL LLVM.
kubamracek added a project: Restricted Project.
kubamracek added a subscriber: llvm-commits.
zaks.anna added inline comments.Oct 28 2016, 5:51 PM
lib/tsan/rtl/tsan_platform_mac.cc
108 ↗(On Diff #75818)

This is not TSan-specific. Should it go into sanitizer-common?

dvyukov added inline comments.Oct 30 2016, 6:13 PM
lib/tsan/rtl/tsan_platform_mac.cc
108 ↗(On Diff #75818)

I would leave it here, unless you have a plan to implement memory profiler for asan on mac.
sanitizer_common contains too much stuff already.
E.g. linux GetMemoryProfile was moved from tsan to sanitizer_common, but it is still used only in tsan. While moving empty stubs were added for other OSes, but the interface is actually such that it cannot be possibly ported to other OSes.
Generalizing only one use case does not usually lead to good results.

143 ↗(On Diff #75818)

I suspect this won't compile when SANITIZER_GO is defined. Go does not have HeapMemBeg.
You can surround it all with #if !SANITIZER_GO as WriteMemoryProfile is not called in Go.
You can test it by running lib/tsan/go/buildgo.sh

158 ↗(On Diff #75818)

s/shadow_res/meta_res/
s/shadow_dirty/meta_dirty/

kubamracek added inline comments.Oct 31 2016, 12:47 PM
lib/tsan/rtl/tsan_platform_mac.cc
108 ↗(On Diff #75818)

It definitely makes sense to implement a memory profiler for ASan as well. The main issue is that some common memory profiling tools on macOS don't work against ASanified/TSanified processes because of the huge reserved regions, so this code is the only way to get the amount of dirty/resident memory (for a specific region).

I'll leave the code here (in tsan/) for now, but when I get to implement this for ASan, I'll probably move this to sanitizer_common/sanitizer_mac.cc. Sounds good?

143 ↗(On Diff #75818)

Sure, I'll fix that. The Go test is run in "check-all" and "check-tsan" on Darwin, so we'll notice if we break it. (I run those before committing.)

158 ↗(On Diff #75818)

Right, thanks for noticing!

kubamracek updated this revision to Diff 76460.Oct 31 2016, 1:00 PM
kubamracek removed rL LLVM as the repository for this revision.
dvyukov added inline comments.Oct 31 2016, 1:19 PM
lib/tsan/rtl/tsan_platform_mac.cc
108 ↗(On Diff #75818)

Sounds good to me.

dvyukov accepted this revision.Oct 31 2016, 1:20 PM
dvyukov edited edge metadata.
This revision is now accepted and ready to land.Oct 31 2016, 1:20 PM
This revision was automatically updated to reflect the committed changes.