This is an archive of the discontinued LLVM Phabricator instance.

[profile] Support for memory-mapping counters to a raw profile
Needs ReviewPublic

Authored by vsk on Apr 19 2016, 4:16 PM.

Details

Reviewers
llvm-commits
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 compiler-rt support required to create instrumented programs which memory-map their counters directly onto a raw profile.

More details

The memory-mapped counters feature works by writing out an empty raw profile at program start time, and then setting up an mmap. If this fails, we clean up and fall back to the normal atexit() profile writer.

One complication arises with profile truncation. The atexit() writer is able to truncate profiles every time __llvm_profile_initialize_file() is called. The mmap() writer can't do this because it needs to preserve the profiling data created by shared objects. The solution I picked is to use a lock file to synchronize truncation efforts.

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

Diff Detail

Event Timeline

vsk updated this revision to Diff 54291.Apr 19 2016, 4:16 PM
vsk retitled this revision from to [profile] Support for memory-mapping counters to a raw profile.
vsk updated this object.
vsk added a reviewer: llvm-commits.
vsk added a subscriber: davidxl.
vsk updated this revision to Diff 54308.Apr 19 2016, 6:35 PM
vsk updated this object.
  • Fix a memory bug I introduced in lprofWriteDataImpl. This will let me remove a hacky for-loop in D19293.
  • Remove an extraneous pair of parentheses.
vsk updated this revision to Diff 54312.Apr 19 2016, 6:48 PM
  • Remove some extraneous changes (e.g, f(void) -> f()).
davidxl added inline comments.Apr 20 2016, 2:43 PM
lib/profile/InstrProfilingFile.c
431

This is another reason there is need to let driver teach the linker to align the combined counter variable section properly. Silently failing like this is not user friendly.

436

I think we should try to avoid bloating the writer interfaces (which makes it really hard to read)

  1. the header offset can be obtained using 'stat' call before the write
  2. There is no need to reuse the stream -- this can get rid of two other parameters.
  1. can probably also use a static variable to set Write alignment -- so that pagesize parameter can be removed.
442

Can you explain what are these file lockings all about?

Is it to support multi-process concurrent update? or multi-threads with cocurrent dlopen/dlclose? Note that file synchronization is not supported with existing atexit file writer. For this reason, I think I suggest removing file locking code in this patch and we can discuss as a follow up. (Note the to be submitted online profile merging support also needs file locking, so there should be some sync up there).

lib/profile/InstrProfilingWriter.c
175

Since tail paddings are explicitly written out here, why is there a need to emit padding bytes in Instrumentation ?

177

This is to make sure the next header is properly aligned?

vsk added inline comments.Apr 20 2016, 3:48 PM
lib/profile/InstrProfilingFile.c
431

In D19295, I make some attempt to align the counters section to the target's page size. It's true that we silently fail here if the alignment is wrong, but falling back to the atexit() writer seems like a reasonable behavior.

436

Ok, I'll make these changes.

442

It's to support concurrent dlopen()'s. I'll remove the extra truncation lock related changes, and we can discuss it later if it's useful.

lib/profile/InstrProfilingWriter.c
175

If the size of the counters section is not a multiple of the target page size, the mmap() call will map in data from the section _following_ the counters section. On Darwin, this creates a problem because the mmap'd data becomes invalid after exit() is called, but before the atexit() handlers are run. This strange behavior results in data corruption and crashes in the removeTruncateLock atexit() handler.

Since I will get rid of the removeTruncateLock atexit() handler, the padding at the end of the counters section should not be necessary.

177

Yes, although with a few tweaks to RawInstrProfReader it might not be necessary. I will try this out and add a comment.