Refactor RemoveNativeDebugInfoForJit.
This is partial revert of CL/1099280 (Remove global maps).
It somewhat resurrects the lazy method removal.
The original goal was to only remove methods from the GC,
and do all of them in bulk for simplicity and efficiency.
However, this is proving infeasible since we have several
corner cases which remove methods outside the GC code path.
The behaviour for the GC code path is preserved by this CL.
Instead of passing method array, the methods are individually
marked for removal and then repacking is immediately forced.
The only difference is that coroner cases are done lazily.
Test: ./art/test.py -b -r --host --jit --64
Change-Id: I42729545d6b51df788d92f9cf149a6e065b90c68
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index 0344fc5..9978a6f 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -1295,8 +1295,6 @@
/* has_should_deoptimize_flag= */ false,
cha_single_implementation_list)) {
code_cache->Free(self, region, reserved_code.data(), reserved_data.data());
- MutexLock mu(self, *Locks::jit_lock_);
- RemoveNativeDebugInfoForJit(ArrayRef<const void*>(reinterpret_cast<const void**>(&code), 1));
return false;
}
@@ -1405,8 +1403,6 @@
codegen->GetGraph()->HasShouldDeoptimizeFlag(),
codegen->GetGraph()->GetCHASingleImplementationList())) {
code_cache->Free(self, region, reserved_code.data(), reserved_data.data());
- MutexLock mu(self, *Locks::jit_lock_);
- RemoveNativeDebugInfoForJit(ArrayRef<const void*>(reinterpret_cast<const void**>(&code), 1));
return false;
}
diff --git a/runtime/jit/debugger_interface.cc b/runtime/jit/debugger_interface.cc
index cecf533..0efa4d2 100644
--- a/runtime/jit/debugger_interface.cc
+++ b/runtime/jit/debugger_interface.cc
@@ -101,7 +101,14 @@
// Some writes are synchronized so libunwindstack can read the memory safely from another process.
constexpr std::memory_order kNonRacingRelaxed = std::memory_order_relaxed;
+// Size of JIT code range covered by each packed JITCodeEntry.
+constexpr uint32_t kJitRepackGroupSize = 64 * KB;
+
+// Automatically call the repack method every 'n' new entries.
+constexpr uint32_t kJitRepackFrequency = 64;
+
// Public binary interface between ART and native tools (gdb, libunwind, etc).
+// The fields below need to be exported and have special names as per the gdb api.
extern "C" {
enum JITAction {
JIT_NOACTION = 0,
@@ -191,6 +198,18 @@
JITDescriptor __dex_debug_descriptor GUARDED_BY(g_dex_debug_lock) {};
}
+// The fields below are internal, but we keep them here anyway for consistency.
+// Their state is related to the static state above and it must be kept in sync.
+
+// Used only in debug builds to check that we are not adding duplicate entries.
+static std::unordered_set<const void*> g_dcheck_all_jit_functions GUARDED_BY(g_jit_debug_lock);
+
+// Methods that have been marked for deletion on the next repack pass.
+static std::vector<const void*> g_removed_jit_functions GUARDED_BY(g_jit_debug_lock);
+
+// Number of small (single symbol) ELF files. Used to trigger repacking.
+static uint32_t g_jit_num_unpacked_entries = 0;
+
struct DexNativeInfo {
static constexpr bool kCopySymfileData = false; // Just reference DEX files.
static JITDescriptor& Descriptor() { return __dex_debug_descriptor; }
@@ -459,13 +478,6 @@
descriptor.free_entries_ = nullptr; // Don't reuse zygote's entries.
}
-// Size of JIT code range covered by each packed JITCodeEntry.
-static constexpr uint32_t kJitRepackGroupSize = 64 * KB;
-
-// Automatically call the repack method every 'n' new entries.
-static constexpr uint32_t kJitRepackFrequency = 64;
-static uint32_t g_jit_num_unpacked_entries = 0;
-
// Split the JIT code cache into groups of fixed size and create single JITCodeEntry for each group.
// The start address of method's code determines which group it belongs to. The end is irrelevant.
// New mini debug infos will be merged if possible, and entries for GCed functions will be removed.
@@ -549,11 +561,23 @@
g_jit_num_unpacked_entries = 0;
}
+void RepackNativeDebugInfoForJitLocked() REQUIRES(g_jit_debug_lock);
+
void AddNativeDebugInfoForJit(const void* code_ptr,
const std::vector<uint8_t>& symfile,
bool allow_packing) {
MutexLock mu(Thread::Current(), g_jit_debug_lock);
DCHECK_NE(symfile.size(), 0u);
+ if (kIsDebugBuild && code_ptr != nullptr) {
+ DCHECK(g_dcheck_all_jit_functions.insert(code_ptr).second) << code_ptr << " already added";
+ }
+
+ // Remove all methods which have been marked for removal. The JIT GC should
+ // force repack, so this should happen only rarely for various corner cases.
+ // Must be done before addition in case the added code_ptr is in the removed set.
+ if (!g_removed_jit_functions.empty()) {
+ RepackNativeDebugInfoForJitLocked();
+ }
CreateJITCodeEntryInternal<JitNativeInfo>(ArrayRef<const uint8_t>(symfile),
/*addr=*/ code_ptr,
@@ -575,9 +599,20 @@
}
}
-void RemoveNativeDebugInfoForJit(ArrayRef<const void*> removed) {
+void RemoveNativeDebugInfoForJit(const void* code_ptr) {
MutexLock mu(Thread::Current(), g_jit_debug_lock);
- RepackEntries(/*compress_entries=*/ true, removed);
+ g_dcheck_all_jit_functions.erase(code_ptr);
+
+ // Method removal is very expensive since we need to decompress and read ELF files.
+ // Collet methods to be removed and do the removal in bulk later.
+ g_removed_jit_functions.push_back(code_ptr);
+}
+
+void RepackNativeDebugInfoForJitLocked() {
+ // Remove entries which are inside packed and compressed ELF files.
+ std::vector<const void*>& removed = g_removed_jit_functions;
+ std::sort(removed.begin(), removed.end());
+ RepackEntries(/*compress_entries=*/ true, ArrayRef<const void*>(removed));
// Remove entries which are not allowed to be packed (containing single method each).
for (const JITCodeEntry* it = __jit_debug_descriptor.head_; it != nullptr;) {
@@ -587,6 +622,14 @@
}
it = next;
}
+
+ removed.clear();
+ removed.shrink_to_fit();
+}
+
+void RepackNativeDebugInfoForJit() {
+ MutexLock mu(Thread::Current(), g_jit_debug_lock);
+ RepackNativeDebugInfoForJitLocked();
}
size_t GetJitMiniDebugInfoMemUsage() {
diff --git a/runtime/jit/debugger_interface.h b/runtime/jit/debugger_interface.h
index 477d58c..8433e69 100644
--- a/runtime/jit/debugger_interface.h
+++ b/runtime/jit/debugger_interface.h
@@ -57,7 +57,11 @@
REQUIRES_SHARED(Locks::jit_lock_); // Might need JIT code cache to allocate memory.
// Notify native tools (e.g. libunwind) that JIT code has been garbage collected.
-void RemoveNativeDebugInfoForJit(ArrayRef<const void*> removed_code_ptrs)
+// The actual removal might be lazy. Removal of address that was not added is no-op.
+void RemoveNativeDebugInfoForJit(const void* code_ptr);
+
+// Merge and compress entries to save space.
+void RepackNativeDebugInfoForJit()
REQUIRES_SHARED(Locks::jit_lock_); // Might need JIT code cache to allocate memory.
// Returns approximate memory used by debug info for JIT code.
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 7ad4700..3b8c2a7 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -377,6 +377,11 @@
return reinterpret_cast<uintptr_t>(code) - RoundUp(sizeof(OatQuickMethodHeader), alignment);
}
+static const void* FromAllocationToCode(const uint8_t* alloc) {
+ size_t alignment = GetInstructionSetAlignment(kRuntimeISA);
+ return reinterpret_cast<const void*>(alloc + RoundUp(sizeof(OatQuickMethodHeader), alignment));
+}
+
static uint32_t GetNumberOfRoots(const uint8_t* stack_map) {
// The length of the table is stored just before the stack map (and therefore at the end of
// the table itself), in order to be able to fetch it from a `stack_map` pointer.
@@ -459,22 +464,18 @@
}
}
-void JitCodeCache::FreeCodeAndData(const void* code_ptr, bool free_debug_info) {
+void JitCodeCache::FreeCodeAndData(const void* code_ptr) {
if (IsInZygoteExecSpace(code_ptr)) {
// No need to free, this is shared memory.
return;
}
uintptr_t allocation = FromCodeToAllocation(code_ptr);
- if (free_debug_info) {
- // Remove compressed mini-debug info for the method.
- // TODO: This is expensive, so we should always do it in the caller in bulk.
- RemoveNativeDebugInfoForJit(ArrayRef<const void*>(&code_ptr, 1));
- }
+ const uint8_t* data = nullptr;
if (OatQuickMethodHeader::FromCodePointer(code_ptr)->IsOptimized()) {
- private_region_.FreeData(GetRootTable(code_ptr));
+ data = GetRootTable(code_ptr);
} // else this is a JNI stub without any data.
- private_region_.FreeCode(reinterpret_cast<uint8_t*>(allocation));
+ FreeLocked(&private_region_, reinterpret_cast<uint8_t*>(allocation), data);
}
void JitCodeCache::FreeAllMethodHeaders(
@@ -490,19 +491,13 @@
->RemoveDependentsWithMethodHeaders(method_headers);
}
- // Remove compressed mini-debug info for the methods.
- std::vector<const void*> removed_symbols;
- removed_symbols.reserve(method_headers.size());
- for (const OatQuickMethodHeader* method_header : method_headers) {
- removed_symbols.push_back(method_header->GetCode());
- }
- std::sort(removed_symbols.begin(), removed_symbols.end());
- RemoveNativeDebugInfoForJit(ArrayRef<const void*>(removed_symbols));
-
ScopedCodeCacheWrite scc(private_region_);
for (const OatQuickMethodHeader* method_header : method_headers) {
- FreeCodeAndData(method_header->GetCode(), /*free_debug_info=*/ false);
+ FreeCodeAndData(method_header->GetCode());
}
+
+ // We have potentially removed a lot of debug info. Do maintenance pass to save space.
+ RepackNativeDebugInfoForJit();
}
void JitCodeCache::RemoveMethodsIn(Thread* self, const LinearAlloc& alloc) {
@@ -993,7 +988,12 @@
const uint8_t* data) {
MutexLock mu(self, *Locks::jit_lock_);
ScopedCodeCacheWrite ccw(*region);
+ FreeLocked(region, code, data);
+}
+
+void JitCodeCache::FreeLocked(JitMemoryRegion* region, const uint8_t* code, const uint8_t* data) {
if (code != nullptr) {
+ RemoveNativeDebugInfoForJit(reinterpret_cast<const void*>(FromAllocationToCode(code)));
region->FreeCode(code);
}
if (data != nullptr) {
diff --git a/runtime/jit/jit_code_cache.h b/runtime/jit/jit_code_cache.h
index d8216d2..f46dc75 100644
--- a/runtime/jit/jit_code_cache.h
+++ b/runtime/jit/jit_code_cache.h
@@ -276,6 +276,9 @@
void Free(Thread* self, JitMemoryRegion* region, const uint8_t* code, const uint8_t* data)
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Locks::jit_lock_);
+ void FreeLocked(JitMemoryRegion* region, const uint8_t* code, const uint8_t* data)
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(Locks::jit_lock_);
// Perform a collection on the code cache.
void GarbageCollectCache(Thread* self)
@@ -422,6 +425,7 @@
// Remove CHA dependents and underlying allocations for entries in `method_headers`.
void FreeAllMethodHeaders(const std::unordered_set<OatQuickMethodHeader*>& method_headers)
+ REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Locks::jit_lock_)
REQUIRES(!Locks::cha_lock_);
@@ -432,8 +436,9 @@
REQUIRES(Locks::mutator_lock_);
// Free code and data allocations for `code_ptr`.
- void FreeCodeAndData(const void* code_ptr, bool free_debug_info = true)
- REQUIRES(Locks::jit_lock_);
+ void FreeCodeAndData(const void* code_ptr)
+ REQUIRES(Locks::jit_lock_)
+ REQUIRES_SHARED(Locks::mutator_lock_);
// Number of bytes allocated in the code cache.
size_t CodeCacheSize() REQUIRES(!Locks::jit_lock_);