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

[Coleen Phillimore]

On Tue, 30 Mar 2021 03:53:54 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> 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.
>
> 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.

ok, yes that says why.

> 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.

It's sort of a question, because the above code that does the same thing for MethodData doesn't clear the exception.  In this case we should clear the exception I believe.  I'll remove 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".

Yes, this is better.

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

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