RFR JDK-8206394 : ResourceMark in AOTCompiledMethod::metadata_do, AOTCompiledMethod::clear_inline_caches , CompiledMethod::clear_ic_stubs , CompiledMethod::cleanup_inline_caches_impl - was : RE: ResourceMarks when CompiledIC_at is used ?

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Thu Jul 5 17:29:57 UTC 2018 

Some ResourceMarks are missing because they might clear other resource 
allocated memory when they return.  This usually occurs with the 
print_on(outputStream*) methods because logStream or stringStream may be 
expanded in the print function and you don't want it cleared on return.  
There may also be a ResourceMark further up the call chain for these 
cases.  So you have to be careful with this.


On 7/5/18 12:18 PM, Baesken, Matthias wrote:
> Great , thanks for your help !
>
>
> Btw.  while looking a bit more at the  ResourceMark usages  , I found  that the    name_and_sig_as_C_string()   calls  are mostly
>     in the codebase with a  related ResourceMark .
>
> However it is not done always,  for example ;   do you think it is missing at these places or not needed?
>
>
> hotspot/share/classfile/verifier.cpp#626
>
> 623void ClassVerifier::verify_method(const methodHandle& m, TRAPS) {
> 624  HandleMark hm(THREAD);

The HandleMark is probably not needed here though.  A ResourceMark does 
seem like a good idea here.

Coleen
> 625  _method = m;   // initialize _method
> 626  log_info(verification)("Verifying method %s", m->name_and_sig_as_C_string());
>
>
> hotspot/share/classfile/classLoader.cpp#2045
>
> void ClassLoader::compile_the_world_in(char* name, Handle loader, TRAPS) {
>
> 2058                if (HAS_PENDING_EXCEPTION) {
> 2059                  clear_pending_exception_if_not_oom(CHECK);
> 2060                  tty->print_cr("CompileTheWorld (%d) : Skipping method: %s", _compile_the_world_class_counter, m->name_and_sig_as_C_string());
> 2061                } else {
> 2062                  _compile_the_world_method_counter++;
> 2063                }
> 2064              }
> 2065            } else {
> 2066              tty->print_cr("CompileTheWorld (%d) : Skipping method: %s", _compile_the_world_class_counter, m->name_and_sig_as_C_string());
> 2067            }
>
> Best regards, Matthias
>
>
>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Donnerstag, 5. Juli 2018 18:07
>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>> Subject: Re: RFR JDK-8206394 : ResourceMark in
>> AOTCompiledMethod::metadata_do,
>> AOTCompiledMethod::clear_inline_caches ,
>> CompiledMethod::clear_ic_stubs ,
>> CompiledMethod::cleanup_inline_caches_impl - was : RE: ResourceMarks
>> when CompiledIC_at is used ?
>>
>> Looks good. I will test it and push if it passed.
>>
>> Thanks,
>> Vladimir
>>
>> On 7/5/18 2:14 AM, Baesken, Matthias wrote:
>>> Hi Vladimir, thanks for looking into it !
>>>
>>> Please review :
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8206394/
>>>
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8206394
>>>
>>>
>>> Best regards, Matthias
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>> Sent: Mittwoch, 4. Juli 2018 19:43
>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>>>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>>>> Subject: Re: ResourceMarks when CompiledIC_at is used ?
>>>>
>>>> Thank you, Matthias
>>>>
>>>> Yes, I checked all these places and they have to be fixed. Please, file
>>>> a bug for jdk11.
>>>>
>>>> Vladimir
>>>>
>>>> On 7/4/18 7:44 AM, Baesken, Matthias wrote:
>>>>> Hello,   I recently looked at   8164293: HotSpot leaking memory in long-
>>>> running requests
>>>>> http://hg.openjdk.java.net/jdk8u/jdk8u/hotspot/rev/8dfbb002197a
>>>>>
>>>>> where a number of missing ResourceMarks were added .
>>>>> I think some of those ResourceMarks were added  because of
>>>> CompiledIC_at  in the changed  functions .
>>>>> I checked for  other CompiledIC_at    in the hs coding ,  and found some
>>>> other places (see below) where  ResourceMarks  are missing as well.
>>>>> Should they be added ?
>>>>>
>>>>> Best regards, Matthias
>>>>>
>>>>>
>>>>>
>>>>> src/hotspot/share/aot/aotCompiledMethod.cpp
>>>>>
>>>>> void AOTCompiledMethod::metadata_do(void f(Metadata*)) {
>>>>> .....
>>>>> 274      } else if (iter.type() == relocInfo::virtual_call_type) {
>>>>> 275        // Check compiledIC holders associated with this nmethod
>>>>> 276        CompiledIC *ic = CompiledIC_at(&iter);    <---------------------------
>> ----
>>>> -----
>>>>>
>>>>>        and
>>>>>
>>>>> 441void AOTCompiledMethod::clear_inline_caches() {
>>>>> 442  assert(SafepointSynchronize::is_at_safepoint(), "cleaning of IC's
>> only
>>>> allowed at safepoint");
>>>>> 443  if (is_zombie()) {
>>>>> 444    return;
>>>>> 445  }
>>>>> 446
>>>>> 447  RelocIterator iter(this);
>>>>> 448  while (iter.next()) {
>>>>> 449    iter.reloc()->clear_inline_cache();
>>>>> 450    if (iter.type() == relocInfo::opt_virtual_call_type) {
>>>>> 451      CompiledIC* cic = CompiledIC_at(&iter);    <---------------------------
>> ----
>>>> -----
>>>>> 452      assert(cic->is_clean(), "!");
>>>>> 453      nativePltCall_at(iter.addr())->set_stub_to_clean();
>>>>> 454    }
>>>>> 455  }
>>>>> 456}
>>>>>
>>>>> src/hotspot/share/code/compiledMethod.cpp
>>>>>
>>>>> 326void CompiledMethod::clear_ic_stubs() {
>>>>> 327  assert_locked_or_safepoint(CompiledIC_lock);
>>>>> 328  RelocIterator iter(this);
>>>>> 329  while(iter.next()) {
>>>>> 330    if (iter.type() == relocInfo::virtual_call_type) {
>>>>> 331      CompiledIC* ic = CompiledIC_at(&iter);      <---------------------------
>> ----
>>>> -----
>>>>> 332      ic->clear_ic_stub();
>>>>> 333    }
>>>>> 334  }
>>>>> 335}
>>>>>
>>>>>
>>>>> 547bool CompiledMethod::cleanup_inline_caches_impl(bool parallel,
>> bool
>>>> unloading_occurred, bool clean_all) {
>>>>> 548  assert_locked_or_safepoint(CompiledIC_lock);
>>>>> 549  bool postponed = false;
>>>>> 550
>>>>> 551  // Find all calls in an nmethod and clear the ones that point to non-
>>>> entrant,
>>>>> 552  // zombie and unloaded nmethods.
>>>>> 553  RelocIterator iter(this, oops_reloc_begin());
>>>>> 554  while(iter.next()) {
>>>>> 555
>>>>> 556    switch (iter.type()) {
>>>>> 557
>>>>> 558    case relocInfo::virtual_call_type:
>>>>> 559      if (unloading_occurred) {
>>>>> 560        // If class unloading occurred we first clear ICs where the cached
>>>> metadata
>>>>> 561        // is referring to an unloaded klass or method.
>>>>> 562        clean_ic_if_metadata_is_dead(CompiledIC_at(&iter));        <------
>> ---
>>>> ---------------------------

More information about the hotspot-dev mailing list