Hi David,

Looks good in general.

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 '*'.

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


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

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

Reply via email to