This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][Tools] Cleanup memory pool used in Archer
ClosedPublic

Authored by protze.joachim on Jun 3 2021, 3:17 AM.

Details

Summary

The main motivation for reusing objects is that it helps to avoid creating and leaking synchronization clocks in TSan. The reused object will reuse the synchronization clock in TSan.

Before, new and delete operator were overloaded to get and return memory for the object from/to the object pool.
This patch replaces the operator overloading with explicit static New/Delete functions.

Objects for parallel regions and implicit tasks will always be recruited and returned to the thread-local object pool. Only for explicit task, there is a chance that an other thread completes the task and will free the object. This patch optimizes the thread-local New/Delete calls by avoiding locks and only lock if the pool is empty. Remote threads return the object into a separate queue.

The chunk size for allocations is now decided based on page size. The objects will also be aligned to cache lines avoiding false sharing.

This is the first patch in a series to provide better tasking support.

Diff Detail

Event Timeline

protze.joachim created this revision.Jun 3 2021, 3:17 AM
protze.joachim requested review of this revision.Jun 3 2021, 3:17 AM
protze.joachim edited the summary of this revision. (Show Details)Jun 3 2021, 3:20 AM
Hahnfeld added inline comments.Jun 6 2021, 11:26 PM
openmp/tools/archer/ompt-tsan.cpp
331–346

I find PoolData vs DataPool highly confusing. Maybe DataPoolEntry?

336–344

Shouldn't be virtual, the idea of CRTP is that you can do without. Reset can be called after static_cast<T *>

protze.joachim marked 2 inline comments as done.

Renamed class as suggested. Also applied a more recent version of clang-format.

Hahnfeld added inline comments.Jun 8 2021, 11:15 PM
openmp/tools/archer/ompt-tsan.cpp
336–344

Still a virtual class...

Making DataPoolEntry non-virtual

openmp/tools/archer/ompt-tsan.cpp
336–344

Now I got your point. Fixed in the updated diff.

This revision is now accepted and ready to land.Jun 9 2021, 3:08 AM
This revision was landed with ongoing or failed builds.Jun 9 2021, 4:38 AM
This revision was automatically updated to reflect the committed changes.