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

[coleen.phillimore at oracle.com]

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
>>