[PATCH] D45756: [XRay][profiler] Part 1: XRay Allocator and Array Implementations

[Keith via Phabricator via llvm-commits]

kpw accepted this revision.
kpw added a comment.
This revision is now accepted and ready to land.

This looks better now, although there were a few issues that slipped through the tests and I wonder if there are others lurking.

If we're going to maintain this cache-line aware storage structure, do you have a plan to measure the performance impact on the GWP implementation versus using a more simple container like sanitizer/vector.h?
This seems important to me because certain kinds of mistakes might still leave the interface operating correctly, but screw up the performance details.



================
Comment at: compiler-rt/lib/xray/xray_segmented_array.h:299-300
+
+template <class T, size_t N>
+typename Array<T, N>::Chunk Array<T, N>::SentinelChunk;
+
----------------
dberris wrote:
> kpw wrote:
> > Can you explain this statement please? My template foo is a bit rusty. Is this a templated member definition and which version of the standard does it require? LLVM is still on 11 IIUC.
> So, what happens here is we're telling the compiler that if the type `Array<T, N>` is ever instantiated in a translation unit, that we're going to get storage for the static data member. This is because `SentinelChunk` is a static data member of the `Array<T,N>` type, and it needs static (program duration) storage to be defined across translation units. This is how you spell this in C++98, AFAICT.
Oh. I see. This is outside of the Array class, so it's an out of line static member definition with default initialization. The templates obscur that a bit, but I get it now.


https://reviews.llvm.org/D45756