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

Reviewed-by: kvn, thartmann
  • Loading branch information
Thomas Schatzl committed Sep 27, 2023
1 parent 1be3557 commit edcc559
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 @@ -1770,17 +1770,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 @@ -1919,11 +1924,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

5 comments on commit edcc559

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@dcubed-ojdk
Copy link
Member

Choose a reason for hiding this comment

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

/tag jdk-22+17

@openjdk
Copy link

@openjdk openjdk bot commented on edcc559 Sep 28, 2023

Choose a reason for hiding this comment

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

@dcubed-ojdk The tag jdk-22+17 was successfully created.

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on edcc559 Dec 26, 2023

Choose a reason for hiding this comment

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

/backport jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on edcc559 Dec 26, 2023

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch backport-GoeLin-edcc559f in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit edcc559f from the openjdk/jdk repository.

The commit being backported was authored by Thomas Schatzl on 27 Sep 2023 and was reviewed by Vladimir Kozlov and Tobias Hartmann.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u-dev:

$ git fetch https://github.com/openjdk-bots/jdk21u-dev.git backport-GoeLin-edcc559f:backport-GoeLin-edcc559f
$ git checkout backport-GoeLin-edcc559f
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u-dev.git backport-GoeLin-edcc559f

Please sign in to comment.