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

[coleen.phillimore at oracle.com]

It is.  And it's technically trivial so you can push it since lots of 
people have seen it.
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
>>>>
>>