LGTM

In D50782#1202492, @dberris wrote:

In D50782#1202318, @eizan wrote:

I'm trying to understand the memory allocation/deallocation dynamics of this module. It looks like all of the Array<...> objects only have their memory allocation increase because the XRay segmented array doesn't support having its allocation shrink.

The Array<...> objects only grow, yes, but we're able to trim them and re-use the memory that's in the internal freelist for the array segments.

However, the memory buffers that each cell in ProfileBuffers point to do get deallocated inside serialize(). Is this correct? Why are these buffers special that they should be deallocated rather than kept around for reuse later like all the other memory?

The actual buffers are obtained through mmap, and they are not fixed-size -- the size of the buffers are dependent on how large the serialised version of the function call tries will be. We can't re-use these buffers across multiple profile collection sessions. The ProfileBuffers array hosts structs that are fixed-size (it's a pointer and a size).

Note that in the reset() function, we destroy the allocators and re-initilize them (through placement new). The static storage for the allocators and the arrays get effectively re-used, without having to reach for memory from the heap (all of the storage for the Array<...> instances will be obtained through the Allocator<...> instances). If you look at Allocator<...>, the destructor will return the memory to the system as well.

I'm trying to understand the memory allocation/deallocation dynamics of this module. It looks like all of the Array<...> objects only have their memory allocation increase because the XRay segmented array doesn't support having its allocation shrink. However, the memory buffers that each cell in ProfileBuffers point to do get deallocated inside serialize(). Is this correct? Why are these buffers special that they should be deallocated rather than kept around for reuse later like all the other memory?

Can you comment for posterity on why the change you're making to the RecursionGuard -> Guard variable is more correct?

This has been submitted: https://reviews.llvm.org/rL317310

I'm going to abandon this change as there doesn't appear to be a pressing need for it right now.

• Add header with license and description to fdrmode-bench.cc
• rename .test files to use less exotic character set

• cleanup unused commented-out code

add microbenchmark module after run and remove timeit from XRay/lit.local.cfg

• move running of benchmark & parsing output into lib/microbenchmark.py
• Move XRay dir into MicroBenchmarks and fix up paths
• move building of XRay benchmarks into microbenchmark lib
• pass ARCH to build_microbenchmark invocation of cmake

Out of curiosity, why did you make bool Revision 'volatile'? Will it be modified elsewhere?

• Replace add_executable() with llvm_test_run() in XRay CMakeLists.txt

• Remove TEST_SUITE_ENABLE_MICROBENCHMARKS
• benchmark targets properly depend on test suite build deps

• Move libs/benchmark-1.0.0 into MicroBenchmarks
• Guard building of all microbenchmarks based on ...ENABLE_XRAY_MICROBENCHMARKS
• Rename ...ENABLE_XRAY_MICROBENCHMARKS -> ...ENABLE_MICROBENCHMARKS

• whitespace

• whitespace

Rebasing off of latest head...

• Remove stale commented out code from MicroBenchmarks/XRay/CMakeLists.txt
• Add missing file(COPY...) command for XRay lit.local.cfg
• Move XRay-related system introspection into XRay CMakeLists.txt
• require ENABLE_XRAY_MICROBENCHMARKS=1 to run XRay Microbenchmarks
• don't explicitly add MicroBenchmarks to test suite list

• comment fix in test-suite/CMakeLists.txt to correctly list excluded dirs

Hello, sorry for taking so long to make the suggested changes.