This is an archive of the discontinued LLVM Phabricator instance.

[profile] LLVM support for memory-mapping profile counters
Needs ReviewPublic

Authored by vsk on Apr 19 2016, 3:43 PM.

Details

Reviewers
davidxl
Summary

Summary

Using memory-mapped profile counters makes it possible to take snapshots of a running process's profiling information without changing the program. This is useful if the process exits abnormally, or if profiling data needs to be collected periodically.

Add the llvm support required to create instrumented programs which memory-map their counters directly onto a raw profile.

More details

This patch bumps the raw profile format version. This format change is needed to record a new field in the profile header: CounterSectionAlignment. This parameter specifies the page size the instrumented program worked with. It's needed to determine section offsets in RawInstrProfReader. If the instrumented program has disabled the memory-mapped counters feature, CounterSectionAlignment=1.

This patch also adds an emitCounterPadding method to the InstrProfiling pass. This padding is needed for compatibility with the Darwin linker, which packs sections together tightly. Without it, the instrumented program could memory-map the contents of unrelated sections into the counters section.

Depends on: http://reviews.llvm.org/D19298

Diff Detail

Event Timeline

vsk updated this revision to Diff 54279.Apr 19 2016, 3:43 PM
vsk retitled this revision from to [profile] LLVM support for memory-mapping profile counters.
vsk updated this object.
vsk added a reviewer: davidxl.
vsk added a subscriber: llvm-commits.
davidxl added inline comments.Apr 19 2016, 4:16 PM
lib/ProfileData/InstrProfReader.cpp
295

Using the for-loop and pattern matching Magic seems weird. Can the paddings be zero filled?

lib/Transforms/Instrumentation/InstrProfiling.cpp
390

Should this be conditionally emitted for darwin only?

Also do you need to emit this padding bytes per module? Have you considered using linker script to do that?

vsk added inline comments.Apr 19 2016, 6:40 PM
lib/ProfileData/InstrProfReader.cpp
295

Sorry about that. This is a hacky workaround for a memory bug I introduced in InstrProfilingWriter.c. Should be fixed shortly.

lib/Transforms/Instrumentation/InstrProfiling.cpp
390

I'd like to add support for this feature on other platforms, so I didn't introduce a platform check here.

I did consider using a linker script. I decided not to because it would require more frontend changes than I'd like. On Darwin, I think this would require creating an empty padding section, aligning it to a page boundary, and passing in an -order_file that places the padding section after the counter section.

vsk updated this revision to Diff 54310.Apr 19 2016, 6:41 PM
vsk updated this object.
  • Remove a hacky for-loop.
vsk added inline comments.Apr 19 2016, 6:53 PM
lib/Transforms/Instrumentation/InstrProfiling.cpp
390

Hm, I think I misunderstood your question.

To clarify, I'm not sure whether or not other linkers have the same behavior, so I thought it would be safer to always emit the padding.

davidxl added inline comments.Apr 20 2016, 12:17 PM
lib/Transforms/Instrumentation/InstrProfiling.cpp
390

Can you measure the size overhead (binary, .o files, and raw profile) with clang self build?

A couple of more questions:

  1. getPageSize is on host -- not the target -- this is not intended, I think
  2. do you assume padding bytes are tail paddings ?
  3. With comdats, this may not work -- for instance, some of the comdat function's counters may be reclaimed by the linker thus leading to counter section size not rounded up to the page size.

This is why I think you need a driver level change.

vsk added inline comments.Apr 20 2016, 3:48 PM
lib/Transforms/Instrumentation/InstrProfiling.cpp
390

We can get the size overhead under 1 page per instrumented Module, and under 3 pages per raw profile. I'll work on getting actual measurements.

Based on your feedback on the compiler-rt patch, I think the right thing to do is to drop emitCounterPadding() and simply pass a section alignment option to the linker.

In more detail:

  1. I used the host's page size to guess the target's page size. I'm not aware of a way to determine the target's page size directly, even at the driver level.
  2. Only some of these padding bytes will be tail paddings (the ones in the last page of the llvm_prf_counts section). This is enough to ensure that we do not mmap() in the section _after_ llvm_prf_counts.
  3. Thanks for the catch. Instead of addressing this issue by emitting an entire host page's worth of padding at the module-level, I'll try to modify the driver. That should let me drop emitCounterPadding.
davidxl added inline comments.Apr 20 2016, 3:55 PM
lib/Transforms/Instrumentation/InstrProfiling.cpp
390

ok -- 2 also answers one of the my question in compiler-rt patch -- mapping passing the end of counter section can lead to problems (names are in readonly).

However, my real question is that in emitCounterPadding current impl, there is no guarantee that paddings can appear after counter (for the last module) -- but it will be a moot point if you can get rid of it.