RFR(S): 8211176: Initialize ObjectMonitor eagerly

[David Holmes]

That looks good to me.

Thanks,
David

On 27/09/2018 4:14 PM, Mikael Vidstedt wrote:
> 
> Thanks David for the offline discussion!
> 
> In my original change it may have looked like I moved around 
> initialization because I also renamed the DeferredInitialize and 
> Initialize methods to init() and init_2() respectively. The only “real” 
> change was to call DeferredInitialize eagerly, immediately after 
> os::init() in thread.cpp. However that’s all moot because as David 
> points out all of the logic in DeferredInitialize can move to 
> Initialize. The spin counters which are currently set up in 
> ObjectMonitor::DeferredInitialize() are only used when the first object 
> monitor is created and used, and that happens after 
> Threads::initialize_java_lang_classes(), which in turn is after 
> ObjectMonitor::Initialize().
> 
> Here’s the new webrev I’m trying out now, please review:
> 
> http://cr.openjdk.java.net/~mikael/webrevs/8211176/webrev.01/open/webrev/ <http://cr.openjdk.java.net/%7Emikael/webrevs/8211176/webrev.01/open/webrev/>
> 
> Testing so far looks good.
> 
> Cheers,
> Mikael
> 
> 
>> On Sep 27, 2018, at 11:05 AM, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> Hi Mikael,
>>
>> As discussed offline I think everything actually in 
>> deferredInitialized() can just be moved into the initialize() method. 
>> The deferral has existed since the code was added in 2005 and I can't 
>> see any obvious reason for it being that way.
>>
>> Thanks,
>> David
>>
>> On 26/09/2018 5:29 PM, Mikael Vidstedt wrote:
>>> Please review this change which removes 
>>> ObjectMonitor::DeferredInitialize in favor of explicit/eager 
>>> initialization:
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8211176
>>> webrev: 
>>> http://cr.openjdk.java.net/~mikael/webrevs/8211176/webrev.00/open/webrev/ 
>>> <http://cr.openjdk.java.net/%7Emikael/webrevs/8211176/webrev.00/open/webrev/> 
>>> <http://cr.openjdk.java.net/~mikael/webrevs/8211176/webrev.00/open/webrev/ 
>>> <http://cr.openjdk.java.net/%7Emikael/webrevs/8211176/webrev.00/open/webrev/>>
>>> * Background (from the issue)
>>> ObjectMonitor::DeferredInitialize is called lazily/on demand when the 
>>> first object monitor instance is used.
>>> Before JDK-8210848 it contained code to process command line 
>>> arguments etc., but after JDK-8210848 the only part that remains is 
>>> setting up some spin related settings. The only dependency that code 
>>> has is that os::is_MP() returns the actual number of processors, 
>>> which is done in os::init().
>>> The ObjectMonitor initialization can and should be done explicitly 
>>> instead.
>>> I have manually verified that os::init() sets up os::_processor_count 
>>> on all the different platforms. I have tried my best to verify that 
>>> anything happening between setting up that variable, and the call to 
>>> ObjectMonitor::init() does not create and/or use ObjectMonitors. The 
>>> InitDone asserts in ObjectMonitor should help capture unless there is 
>>> some race during startup, but since this happens very early during 
>>> startup it seems unlikely. Please let me know if there’s something 
>>> I’m missing.
>>> * Testing
>>> I have run some basic (tier1) testing, and I’m in the process of 
>>> running additional tests (tier2 + tier3). Let me know if you have 
>>> suggestions on additional tests to run.
>>> Cheers,
>>> Mikael
>