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

David Holmes david.holmes at oracle.com
Tue Oct 8 21:24:04 UTC 2019


Hi Dan,

On 8/10/2019 11:28 pm, Daniel D. Daugherty wrote:
> On 10/7/19 9:58 PM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8231737
>> webrev: http://cr.openjdk.java.net/~dholmes/8231737/webrev/
> 
> Thanks for cleaning up this code!!

Thanks for taking a look. All issues below have been fixed.

Cheers,
David
-----

> src/hotspot/share/prims/jvmtiRawMonitor.hpp
>      L58:   QNode* volatile _entry_list;       // Threads blocked on 
> entry or reentry.
>      L61:   QNode* volatile _wait_set;         // Threads wait()ing on 
> the monitor
>          Comments no longer line up with their sibling comments.
> 
>      L62:   volatile jint _waiters;          // number of waiting threads
>          Not your bug, but this comment doesn't line up either.
> 
>      L95:   int raw_wait(jlong millis, bool interruptable, Thread* self);
>          Not your typo, but: s/interruptable/interruptible/
> 
> src/hotspot/share/prims/jvmtiRawMonitor.cpp
>      L133:     node._next  = _entry_list;
>      L134:     _entry_list  = &node;
>      L183:   node._t_state    = QNode::TS_WAIT;
>      L186:   node._next     = _wait_set;
>      L187:   _wait_set       = &node;
>          Extra space(s) before the '='.
> 
>      L346:     --_recursions;
>      L380:   _waiters++;
>      L387:   _waiters--;
>          Not your bug, but inconsistent styles here.
>          Personally, I prefer post increment and decrement.
> 
> Thumbs up! I only have nits so feel free to ignore those
> since you've fixed so many already. I made a pass via frames
> and a pass via udiffs and tried to spot any accidental
> changes and didn't find any.
> 
> Dan
> 
> 
>>
>> 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
> 


More information about the serviceability-dev mailing list