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

[coleen . phillimore]

Thank you!!

On 10/8/19 12:42 AM, David Holmes wrote:
Hi Serguei,

On 8/10/2019 2:18 pm, serguei.spit...@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