This is an archive of the discontinued LLVM Phabricator instance.

[XRay] Move buffer extents back to the heap
ClosedPublic

Authored by dberris on Nov 18 2018, 9:33 PM.

Details

Summary

This change addresses an issue which shows up with the synchronised race
between threads writing into a buffer, and another thread reading the
buffer.

In a lot of cases, we cannot guarantee that threads will always see the
signal to finalise their buffers in time despite the grace periods and
state machine maintained through atomic variables. This change addresses
it by ensuring that the same instance being updated to indicate how much
of the buffer is "used" by the writing thread is the same instance being
read by the thread processing the buffer to be written out to disk or
handled through the iterators.

To do this, we ensure that all the "extents" instances live in their own
the backing store, in a different contiguous page from the
buffer-specific backing store. We also take precautions to ensure that
the atomic variables are cache-line-sized to prevent false-sharing from
unnecessarily causing cache contention on unrelated writes/reads.

It's feasible that we may in the future be able to move the storage of
the extents objects into the single backing store, slightly changing the
way to compute the size(s) of the buffers, but in the meantime we'll
settle for the isolation afforded by having a different backing store
for the extents instances.

Diff Detail

Repository
rL LLVM

Event Timeline

dberris created this revision.Nov 18 2018, 9:33 PM
mboerger added inline comments.Nov 19 2018, 8:05 AM
compiler-rt/lib/xray/xray_buffer_queue.cc
59 ↗(On Diff #174558)

Shouldn't this be cache line size minus size of Extends?

dberris marked an inline comment as done.Nov 19 2018, 2:00 PM
dberris added inline comments.
compiler-rt/lib/xray/xray_buffer_queue.cc
59 ↗(On Diff #174558)

Nope, the union is meant to ensure that the size of the member is the longer of these two elements.

This revision is now accepted and ready to land.Nov 19 2018, 2:06 PM
dberris updated this revision to Diff 174702.Nov 19 2018, 3:38 PM
dberris marked an inline comment as done.

clang-format with LLVM style

This revision was automatically updated to reflect the committed changes.