RFR: 8205459: Rename Access API flag decorators

[Per Liden]

Hi Kim,

On 06/21/2018 10:25 AM, Kim Barrett wrote:
> Please review these name and behavioral changes for some Access API
> decorators.  We want to recategorize these as orthogonal boolean
> flags.  Specifically,
> 
> OOP_NOT_NULL => IS_NOT_NULL
> AS_DEST_NOT_INITIALIZED => IS_DEST_UNINITIALIZED
> IN_HEAP_ARRAY => IN_ARRAY
> 
> In addition:
> 
> Remove OOP_DECORATOR_MASK.
> 
> Change IN_ARRAY (formerly IN_HEAP_ARRAY) to no longer imply IN_HEAP.
> Note that all of the places where IN_HEAP_ARRAY was being specified
> already also specified IN_HEAP.
> 
> Some cleanups, such as renaming local variables from "on_array" to
> "is_array", for consistency with the associated decorator name.
> 
> To aid reviewing, separate webrevs for each renaming are provided.
> 
> Each of these renamings involves a lot of mechanical text replacement,
> with some further manual changes.  Separate webrevs are also provided
> for the mechanical and manual parts of each renaming.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8205459
> 
> Testing:
> Mach5 tier1,2,3,hs-tier4,5.
> 
> Webrevs:
> 
> The entire set of changes:
> 
> http://cr.openjdk.java.net/~kbarrett/8205459/open.00/

Looks good, just two comments:

src/hotspot/share/oops/accessDecorators.hpp
-------------------------------------------
Looks like the decorator numbers got a bit mixed up:

  [...]
  155 const DecoratorSet AS_RAW                  = UCONST64(1) << 12;
  156 const DecoratorSet AS_NO_KEEPALIVE         = UCONST64(1) << 14;
  [...]
  185 const DecoratorSet IN_HEAP            = UCONST64(1) << 20;
  186 const DecoratorSet IN_NATIVE          = UCONST64(1) << 22;
  [...]
  196 const DecoratorSet IS_ARRAY              = UCONST64(1) << 21;
  197 const DecoratorSet IS_DEST_UNINITIALIZED = UCONST64(1) << 13;
  198 const DecoratorSet IS_NOT_NULL           = UCONST64(1) << 25;
  [...]

Stefan just told me you planned to fix this in a different patch?


src/hotspot/share/oops/access.hpp
---------------------------------
Could we turn this:

  123                                              IS_ARRAY | IS_NOT_NULL |
  124                                              IN_HEAP;

Into a single line, with IN_HEAP first, like:

  123                                              IN_HEAP | IS_ARRAY | 
IS_NOT_NULL;


cheers,
Per

> 
> That's made up of the following three changes, applied in order:
> 
> (1) Renaming OOP_NOT_NULL => IS_NOT_NULL
> 
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_oop_not_null/
> 
> which consists of these:
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_oop_not_null_mechanical/
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_oop_not_null_manual/
> 
> The mechanical part was made using the following bash commands,
> executed from src/hotspot:
> 
> find . -type f -name "*.[ch]pp" \
>    -exec grep -q OOP_NOT_NULL {} \; -print \
>    | xargs sed -i 's/OOP_NOT_NULL/IS_NOT_NULL/'
> 
> sed -i '/const DecoratorSet OOP_DECORATOR_MASK/d' share/oops/accessDecorators.hpp
> 
> find . -type f -name "*.[ch]pp" \
>    -exec grep -q OOP_DECORATOR_MASK {} \; -print \
>    | xargs sed -i 's/OOP_DECORATOR_MASK/IS_NOT_NULL/'
> 
> (2) Renaming AS_DEST_NOT_INITIALIZED to IS_DEST_UNINITIALIZED
> 
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_as_dest_not_initialized/
> 
> which consists of these:
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_as_dest_not_initialized_mechanical/
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_as_dest_not_initialized_manual/
> 
> The mechanical part was made using the following bash command,
> executed from src/hotspot:
> 
> find . -type f -name "*.[ch]pp" \
>    -exec grep -q AS_DEST_NOT_INITIALIZED {} \; -print \
>    | xargs sed -i 's/AS_DEST_NOT_INITIALIZED/IS_DEST_UNINITIALIZED/'
> 
> (3) Renaming IN_HEAP_ARRAY to IN_ARRAY
> 
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_in_heap_array/
> 
> which consists of these:
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_in_heap_array_mechanical/
> http://cr.openjdk.java.net/~kbarrett/8205459/rename_in_heap_array_manual/
> 
> The mechanical part was made using the following bash commands,
> executed from src/hotspot:
> 
> find . -type f -name "*.[ch]pp" \
>    -exec grep -q IN_HEAP_ARRAY {} \; -print \
>    | xargs sed -i 's/IN_HEAP_ARRAY/IS_ARRAY/'
> 
> cd cpu
> find . -type f -name "*.[ch]pp" \
>    -exec grep -q "bool on_array = " {} \; -print \
>    | xargs sed -i 's/on_array/is_array/'
> 
>