Skip to content

Commit

Permalink
8316661: CompilerThread leaks CodeBlob memory when dynamically stoppi…
Browse files Browse the repository at this point in the history
…ng compiler thread in non-product

Backport-of: edcc559f09364da3692862e1f3d0636aa8eec1d4
  • Loading branch information
GoeLin committed Dec 29, 2023
1 parent 80c86c2 commit 191fc6b
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 39 deletions.
28 changes: 7 additions & 21 deletions src/hotspot/share/code/codeBlob.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,6 @@ class CodeBlob {
#ifndef PRODUCT
AsmRemarks _asm_remarks;
DbgStrings _dbg_strings;

~CodeBlob() {
_asm_remarks.clear();
_dbg_strings.clear();
}
#endif // not PRODUCT

CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, int frame_complete_offset,
Expand All @@ -132,10 +127,17 @@ class CodeBlob {
CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, CodeBuffer* cb, int frame_complete_offset,
int frame_size, OopMapSet* oop_maps,
bool caller_must_gc_arguments, bool compiled = false);

void operator delete(void* p) { }

public:
// Only used by unit test.
CodeBlob() : _type(compiler_none) {}

virtual ~CodeBlob() {
assert(_oop_maps == nullptr, "Not flushed");
}

// Returns the space needed for CodeBlob
static unsigned int allocation_size(CodeBuffer* cb, int header_size);
static unsigned int align_code_offset(int offset);
Expand Down Expand Up @@ -404,10 +406,6 @@ class BufferBlob: public RuntimeBlob {
BufferBlob(const char* name, int size);
BufferBlob(const char* name, int size, CodeBuffer* cb);

// This ordinary operator delete is needed even though not used, so the
// below two-argument operator delete will be treated as a placement
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
void operator delete(void* p);
void* operator new(size_t s, unsigned size) throw();

public:
Expand Down Expand Up @@ -492,10 +490,6 @@ class RuntimeStub: public RuntimeBlob {
bool caller_must_gc_arguments
);

// This ordinary operator delete is needed even though not used, so the
// below two-argument operator delete will be treated as a placement
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
void operator delete(void* p);
void* operator new(size_t s, unsigned size) throw();

public:
Expand Down Expand Up @@ -532,10 +526,6 @@ class SingletonBlob: public RuntimeBlob {
friend class VMStructs;

protected:
// This ordinary operator delete is needed even though not used, so the
// below two-argument operator delete will be treated as a placement
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
void operator delete(void* p);
void* operator new(size_t s, unsigned size) throw();

public:
Expand Down Expand Up @@ -750,10 +740,6 @@ class UpcallStub: public RuntimeBlob {
intptr_t exception_handler_offset,
jobject receiver, ByteSize frame_data_offset);

// This ordinary operator delete is needed even though not used, so the
// below two-argument operator delete will be treated as a placement
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
void operator delete(void* p);
void* operator new(size_t s, unsigned size) throw();

struct FrameData {
Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/share/code/codeCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,10 +472,10 @@ CodeHeap* CodeCache::get_code_heap_containing(void* start) {
return nullptr;
}

CodeHeap* CodeCache::get_code_heap(const CodeBlob* cb) {
CodeHeap* CodeCache::get_code_heap(const void* cb) {
assert(cb != nullptr, "CodeBlob is null");
FOR_ALL_HEAPS(heap) {
if ((*heap)->contains_blob(cb)) {
if ((*heap)->contains(cb)) {
return *heap;
}
}
Expand Down Expand Up @@ -604,6 +604,7 @@ void CodeCache::free(CodeBlob* cb) {
heap->set_adapter_count(heap->adapter_count() - 1);
}

cb->~CodeBlob();
// Get heap for given CodeBlob and deallocate
get_code_heap(cb)->deallocate(cb);

Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/code/codeCache.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class CodeCache : AllStatic {
// Creates a new heap with the given name and size, containing CodeBlobs of the given type
static void add_heap(ReservedSpace rs, const char* name, CodeBlobType code_blob_type);
static CodeHeap* get_code_heap_containing(void* p); // Returns the CodeHeap containing the given pointer, or nullptr
static CodeHeap* get_code_heap(const CodeBlob* cb); // Returns the CodeHeap for the given CodeBlob
static CodeHeap* get_code_heap(const void* cb); // Returns the CodeHeap for the given CodeBlob
static CodeHeap* get_code_heap(CodeBlobType code_blob_type); // Returns the CodeHeap for the given CodeBlobType
// Returns the name of the VM option to set the size of the corresponding CodeHeap
static const char* get_code_heap_flag_name(CodeBlobType code_blob_type);
Expand Down Expand Up @@ -397,10 +397,10 @@ template <class T, class Filter, bool is_relaxed> class CodeBlobIterator : publi
// If set to nullptr, initialized by first call to next()
_code_blob = nm;
if (nm != nullptr) {
while(!(*_heap)->contains_blob(_code_blob)) {
while(!(*_heap)->contains(_code_blob)) {
++_heap;
}
assert((*_heap)->contains_blob(_code_blob), "match not found");
assert((*_heap)->contains(_code_blob), "match not found");
}
}

Expand Down
21 changes: 11 additions & 10 deletions src/hotspot/share/compiler/compileBroker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1772,17 +1772,22 @@ bool CompileBroker::init_compiler_runtime() {
return true;
}

void CompileBroker::free_buffer_blob_if_allocated(CompilerThread* thread) {
BufferBlob* blob = thread->get_buffer_blob();
if (blob != nullptr) {
blob->flush();
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
CodeCache::free(blob);
}
}

/**
* If C1 and/or C2 initialization failed, we shut down all compilation.
* We do this to keep things simple. This can be changed if it ever turns
* out to be a problem.
*/
void CompileBroker::shutdown_compiler_runtime(AbstractCompiler* comp, CompilerThread* thread) {
// Free buffer blob, if allocated
if (thread->get_buffer_blob() != nullptr) {
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
CodeCache::free(thread->get_buffer_blob());
}
free_buffer_blob_if_allocated(thread);

if (comp->should_perform_shutdown()) {
// There are two reasons for shutting down the compiler
Expand Down Expand Up @@ -1921,11 +1926,7 @@ void CompileBroker::compiler_thread_loop() {
// Notify compiler that the compiler thread is about to stop
thread->compiler()->stopping_compiler_thread(thread);

// Free buffer blob, if allocated
if (thread->get_buffer_blob() != nullptr) {
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
CodeCache::free(thread->get_buffer_blob());
}
free_buffer_blob_if_allocated(thread);
return; // Stop this thread.
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/compiler/compileBroker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ class CompileBroker: AllStatic {
static bool wait_for_jvmci_completion(JVMCICompiler* comp, CompileTask* task, JavaThread* thread);
#endif

static void free_buffer_blob_if_allocated(CompilerThread* thread);

static void invoke_compiler_on_method(CompileTask* task);
static void handle_compile_error(CompilerThread* thread, CompileTask* task, ciEnv* ci_env,
int compilable, const char* failure_reason);
Expand Down
3 changes: 0 additions & 3 deletions src/hotspot/share/memory/heap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,6 @@ class CodeHeap : public CHeapObj<mtCode> {

// Containment means "contained in committed space".
bool contains(const void* p) const { return low() <= p && p < high(); }
bool contains_blob(const CodeBlob* blob) const {
return contains((void*)blob);
}

void* find_start(void* p) const; // returns the block containing p or null
CodeBlob* find_blob(void* start) const;
Expand Down

1 comment on commit 191fc6b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.