RFR: 8264149 BreakpointInfo::set allocates metaspace object in VM thread [v3]

[David Holmes]

On Tue, 30 Mar 2021 03:06:11 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change creates a Metaspace::allocate function that doesn't pass TRAPS to be used by MethodCounters.  TRAPS and exceptions shouldn't be thrown from non-JavaThreads.
>> 
>> Tested with tier1-7.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make which version of MethodCounters::allocate() is called clearer.

Hi Coleen,

Updated code is much clearer but I have a suggested refactoring to avoid the duplicated OOM code.

Thanks,
David

src/hotspot/share/oops/method.cpp line 570:

> 568:   if (current->is_Java_thread()) {
> 569:     Thread* THREAD = current;
> 570:     counters = MethodCounters::allocate(mh, THREAD);

Can you add a comment before this line:

// Use the TRAPS version for a JavaThread so it will adjust the GC  threshold if needed.

Thanks.

src/hotspot/share/oops/method.cpp line 572:

> 570:     counters = MethodCounters::allocate(mh, THREAD);
> 571:     if (HAS_PENDING_EXCEPTION) {
> 572:       CLEAR_PENDING_EXCEPTION;  // MethodData above doesn't clear exception

I don't understand the comment.

src/hotspot/share/oops/method.cpp line 575:

> 573:       CompileBroker::log_metaspace_failure();
> 574:       ClassLoaderDataGraph::set_metaspace_oom(true);
> 575:       return NULL;

You could factor this out for both cases by testing "counters == NULL".

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3207