RFR (S) JDK-8178712: ResourceMark may be missing inside initialize_[vi]table

[Lois Foltan]

On 7/10/2018 4:17 PM, Calvin Cheung wrote:

>
>
> On 7/10/18, 12:34 PM, Lois Foltan wrote:
>> On 7/10/2018 3:10 PM, Calvin Cheung wrote:
>>
>>> Hi Lois,
>>>
>>> I'm wondering if the ResourceMark in the following function in 
>>> universe.cpp could be removed?
>>> If I understand the code correctly, the ResourceMark is necessary 
>>> for Universe::reinitialize_itables() which calls into 
>>> klassItable::initialize_itable() where you've added ResourceMark 
>>> with your change.
>>>
>>> bool universe_post_init() {
>>>   assert(!is_init_completed(), "Error: initialization not yet 
>>> completed!");
>>>   Universe::_fully_initialized = true;
>>>   EXCEPTION_MARK;
>>>   { ResourceMark rm;
>>>     Interpreter::initialize();      // needed for interpreter entry 
>>> points
>>>     if (!UseSharedSpaces) {
>>>       HandleMark hm(THREAD);
>>>       Klass* ok = SystemDictionary::Object_klass();
>>>       Universe::reinitialize_vtable_of(ok, CHECK_false);
>>>       Universe::reinitialize_itables(CHECK_false);
>>>     }
>>>   }
>>
>> Thanks Calvin for the review!  I wondered that as well, but I think 
>> the ResourceMark may be needed for the Interpreter::initialize(). For 
>> example, it calls TemplateTable::initialize() which logs timer 
>> information which I suspect may need a ResourceMark.  So, it wasn't 
>> clear that the ResourceMark in universe_post_init() was solely needed 
>> for the reinitialize_vtable and itables.
> In timerTrace.hpp:
> // TraceTime is used for tracing the execution time of a block
> // Usage:
> //  {
> //    TraceTime t("some timer", TIMERTRACE_LOG(Info, startuptime, 
> tagX...));
> //    some_code();
> //  }
> //
>
> I looked at several usage of TraceTime and they all don't have 
> ResourceMark before it.
>
> I'm fine with leaving the ResourceMark in universe_post_init() if you 
> want to play it safe.
Thanks Calvin for looking at this! I think I am going to leave it as is.
Lois

>
> thanks,
> Calvin
>
>>
>> Thanks,
>> Lois
>>
>>>
>>> It looks good otherwise.
>>>
>>> thanks,
>>> Calvin
>>>
>>> On 7/10/18, 10:19 AM, Lois Foltan wrote:
>>>> Please review this clean up change to correctly set ResourceMark 
>>>> from within klassVtable::initialize_vtable() and 
>>>> klassItable::initialize_itable() when applicable, instead of having 
>>>> all instances of calls to these two methods establish a 
>>>> ResourceMark unnecessarily prior to.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~lfoltan/bug_jdk8178712/
>>>> bug link at https://bugs.openjdk.java.net/browse/JDK-8178712
>>>>
>>>> Testing: hs-tier1-3, jdk-tier1-3 (complete)
>>>>                hs-tier4-5 (in progress)
>>>>
>>>> Thanks,
>>>> Lois
>>