Skip to content

Commit 4314f4c

Browse files
committed
Revert "[BOLT][Instrumentation] Put Allocator itslef in shared memory by default"
This reverts commit ad4e077. Breaks BOLT upstream testing: https://lab.llvm.org/buildbot/#/builders/244/builds/13736
1 parent 205547b commit 4314f4c

File tree

1 file changed

+14
-42
lines changed

1 file changed

+14
-42
lines changed

bolt/runtime/instr.cpp

Lines changed: 14 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,7 @@ class BumpPtrAllocator {
210210

211211
/// Used for allocating indirect call instrumentation counters. Initialized by
212212
/// __bolt_instr_setup, our initialization routine.
213-
BumpPtrAllocator *GlobalAlloc;
214-
215-
// Storage for GlobalAlloc which can be shared if not using
216-
// instrumentation-file-append-pid.
217-
void *GlobalMetadataStorage;
218-
213+
BumpPtrAllocator GlobalAlloc;
219214
} // anonymous namespace
220215

221216
// User-defined placement new operators. We only use those (as opposed to
@@ -235,10 +230,6 @@ void *operator new[](size_t Sz, BumpPtrAllocator &A, char C) {
235230
memset(Ptr, C, Sz);
236231
return Ptr;
237232
}
238-
239-
// Declaration for global allocator to construct it in shared memory if needed.
240-
// Needed because we can't #include <new>
241-
void *operator new(size_t, void *) noexcept;
242233
// Only called during exception unwinding (useless). We must manually dealloc.
243234
// C++ language weirdness
244235
void operator delete(void *Ptr, BumpPtrAllocator &A) { A.deallocate(Ptr); }
@@ -304,17 +295,7 @@ class SimpleHashTable {
304295
/// Increment by 1 the value of \p Key. If it is not in this table, it will be
305296
/// added to the table and its value set to 1.
306297
void incrementVal(uint64_t Key, BumpPtrAllocator &Alloc) {
307-
if (!__bolt_instr_conservative) {
308-
TryLock L(M);
309-
if (!L.isLocked())
310-
return;
311-
auto &E = getOrAllocEntry(Key, Alloc);
312-
++E.Val;
313-
return;
314-
}
315-
Lock L(M);
316-
auto &E = getOrAllocEntry(Key, Alloc);
317-
++E.Val;
298+
++get(Key, Alloc).Val;
318299
}
319300

320301
/// Basic member accessing interface. Here we pass the allocator explicitly to
@@ -358,10 +339,10 @@ class SimpleHashTable {
358339
if (Entry.Key == VacantMarker)
359340
continue;
360341
if (Entry.Key & FollowUpTableMarker) {
361-
MapEntry *Next =
362-
reinterpret_cast<MapEntry *>(Entry.Key & ~FollowUpTableMarker);
363-
assert(Next != Entries, "Circular reference!");
364-
forEachElement(Callback, IncSize, Next, args...);
342+
forEachElement(Callback, IncSize,
343+
reinterpret_cast<MapEntry *>(Entry.Key &
344+
~FollowUpTableMarker),
345+
args...);
365346
continue;
366347
}
367348
Callback(Entry, args...);
@@ -426,11 +407,8 @@ class SimpleHashTable {
426407
}
427408

428409
MapEntry &getOrAllocEntry(uint64_t Key, BumpPtrAllocator &Alloc) {
429-
if (TableRoot) {
430-
MapEntry &E = getEntry(TableRoot, Key, Key, Alloc, 0);
431-
assert(!(E.Key & FollowUpTableMarker), "Invalid entry!");
432-
return E;
433-
}
410+
if (TableRoot)
411+
return getEntry(TableRoot, Key, Key, Alloc, 0);
434412
return firstAllocation(Key, Alloc);
435413
}
436414
};
@@ -1628,19 +1606,13 @@ extern "C" void __attribute((force_align_arg_pointer)) __bolt_instr_setup() {
16281606
MAP_ANONYMOUS | MapPrivateOrShared | MAP_FIXED, -1, 0);
16291607
assert(Ret != MAP_FAILED, "__bolt_instr_setup: Failed to mmap counters!");
16301608

1631-
GlobalMetadataStorage = __mmap(0, 4096, PROT_READ | PROT_WRITE,
1632-
MapPrivateOrShared | MAP_ANONYMOUS, -1, 0);
1633-
assert(GlobalMetadataStorage != MAP_FAILED,
1634-
"__bolt_instr_setup: failed to mmap page for metadata!");
1635-
1636-
GlobalAlloc = new (GlobalMetadataStorage) BumpPtrAllocator;
1637-
// Conservatively reserve 100MiB
1638-
GlobalAlloc->setMaxSize(0x6400000);
1639-
GlobalAlloc->setShared(Shared);
1640-
GlobalWriteProfileMutex = new (*GlobalAlloc, 0) Mutex();
1609+
// Conservatively reserve 100MiB shared pages
1610+
GlobalAlloc.setMaxSize(0x6400000);
1611+
GlobalAlloc.setShared(Shared);
1612+
GlobalWriteProfileMutex = new (GlobalAlloc, 0) Mutex();
16411613
if (__bolt_instr_num_ind_calls > 0)
16421614
GlobalIndCallCounters =
1643-
new (*GlobalAlloc, 0) IndirectCallHashTable[__bolt_instr_num_ind_calls];
1615+
new (GlobalAlloc, 0) IndirectCallHashTable[__bolt_instr_num_ind_calls];
16441616

16451617
if (__bolt_instr_sleep_time != 0) {
16461618
// Separate instrumented process to the own process group
@@ -1655,7 +1627,7 @@ extern "C" void __attribute((force_align_arg_pointer)) __bolt_instr_setup() {
16551627

16561628
extern "C" __attribute((force_align_arg_pointer)) void
16571629
instrumentIndirectCall(uint64_t Target, uint64_t IndCallID) {
1658-
GlobalIndCallCounters[IndCallID].incrementVal(Target, *GlobalAlloc);
1630+
GlobalIndCallCounters[IndCallID].incrementVal(Target, GlobalAlloc);
16591631
}
16601632

16611633
/// We receive as in-stack arguments the identifier of the indirect call site

0 commit comments

Comments
 (0)