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

[David Holmes]

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.

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