RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

[David Holmes]

On 8/10/2019 11:07 pm, coleen.phillimore at oracle.com wrote:
> It is.  And it's technically trivial so you can push it since lots of 
> people have seen it.

And everyone spots something else that needs adjusting :) But I think I 
have everything now that Dan has had a look.

Cheers,
David

> Coleen
> 
> On 10/8/19 9:06 AM, David Holmes wrote:
>> Thanks Coleen, I'll take this as a review :)
>>
>> David
>>
>> On 8/10/2019 10:59 pm, coleen.phillimore at oracle.com wrote:
>>>
>>> Thank you!!
>>>
>>> On 10/8/19 12:42 AM, David Holmes wrote:
>>>> Hi Serguei,
>>>>
>>>> On 8/10/2019 2:18 pm, serguei.spitsyn at oracle.com wrote:
>>>>> Hi David,
>>>>>
>>>>> Looks good in general.
>>>>
>>>> Thanks for looking at it.
>>>>
>>>>> A couple of places still need a cleanup.
>>>>>
>>>>> http://cr.openjdk.java.net/~dholmes/8231737/webrev/src/hotspot/share/prims/jvmtiRawMonitor.hpp.frames.html 
>>>>>
>>>>>
>>>>>
>>>>> QNode pointer is not unified:
>>>>>
>>>>> 47 QNode * volatile _next;
>>>>> 48 QNode * volatile _prev;
>>>>> 68 void simpleEnter(Thread * self) ;
>>>>> 69 void simpleExit(Thread * self) ;
>>>>> but:53 QNode(Thread* thread); 56 Thread* volatile _owner; // 
>>>>> pointer to owning thread 58 QNode* volatile _entryList; // Threads 
>>>>> blocked on entry or reentry.
>>>>>     61 QNode* volatile _waitSet; // Threads wait()ing on the 
>>>>> monitor There are more places where a space is placed or not 
>>>>> between the type name and the '*'.
>>>>
>>>> Okay I have switched everything to the:
>>>>
>>>> type* var
>>>>
>>>> style. It seems most prevelant based on a grep for "Thread* t" type 
>>>> usage.
>>>
>>> Yeah, this is the style chosen for the Hotspot sources, because the 
>>> star is part of the type, not part of the name.  But don't argue with 
>>> me about it because I'm repeating the rationale that people who felt 
>>> strongly about it gave.  It's important to me that it's consistent.
>>>
>>> thanks!
>>> Coleen
>>>>
>>>>> Missed space before ';':
>>>>>
>>>>> 68 void simpleEnter(Thread * self) ;
>>>>> 69 void simpleExit(Thread * self) ;
>>>>> 70 int simpleWait(Thread * self, jlong millis) ;
>>>>> 71 void simpleNotify(Thread * self, bool all) ;
>>>>
>>>> Fixed.
>>>>
>>>>> http://cr.openjdk.java.net/~dholmes/8231737/webrev/src/hotspot/share/prims/jvmtiRawMonitor.cpp.frames.html 
>>>>>
>>>>>
>>>>> Space is not needed after the cast: 280 jt = (JavaThread*) self; 
>>>>> 370 jt = (JavaThread*) self;
>>>>>
>>>>> Other places do not have it:
>>>>>
>>>>> 150 OrderAccess::release_store(&_owner, (Thread*)NULL); 288 
>>>>> contended = Atomic::cmpxchg(jt, &_owner, (Thread*)NULL); 291 
>>>>> contended = Atomic::cmpxchg(self, &_owner, (Thread*)NULL);
>>>>
>>>> Fixed.
>>>>
>>>> webrev updated in place.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>> On 10/7/19 18:58, David Holmes wrote:
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8231737
>>>>>> webrev: http://cr.openjdk.java.net/~dholmes/8231737/webrev/
>>>>>>
>>>>>> Stylistic code cleanup of JvmtiRawMonitor code as previously 
>>>>>> promised:
>>>>>> - Self -> self
>>>>>> - SimpleX -> simpleX
>>>>>> - Contended -> contended
>>>>>> - Node -> node
>>>>>> - TState -> tState (variable name)
>>>>>> - _WaitSet -> _waitSet
>>>>>> - _EntryList -> _entryList;
>>>>>> - All -> all
>>>>>> - remove extra space before ( in function calls
>>>>>> - remove extra space before ;
>>>>>> - remove extra space before ++ and --
>>>>>> - add spaces around binary operators
>>>>>> - use { } on all blocks
>>>>>> - use one statement per line.
>>>>>> - fix indent and alignment (ie remove artificial alignment)
>>>>>>
>>>>>> Probably simplest to look at new code and see if it looks okay 
>>>>>> rather than trying to spot each individual change. :)
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>
>>>
>