[lworld] RFR: jdk-8249555 Rename valueArray* source files to flatArray*

Harold Seigel hseigel at openjdk.java.net
Wed Jul 22 13:47:57 UTC 2020


On Wed, 22 Jul 2020 13:27:23 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> Please review this change which renames the appropriate hotspot runtime source files, methods, fields, etc. from
>> "valueArray*" to "flatArray*".  The change does not rename any gc or jit source files.
>> The change was tested with tiers 1 and 2 on Windows, Mac, and Linux x64, and tiers 3-5 on Linux x64.
>> 
>> Thanks! Harold
>
> Harold,
> 
> Thank you for doing this fastidious renaming work a second time.
> Overall it looks good and new names sound explicit to me.
> Just three comments about names that could be improved.
> 
> Looks good to me.
> 
> Fred

Thanks Fred for reviewing these changes!

> src/hotspot/share/ci/ciTypeFlow.hpp line 345:
> 
>> 344:       // Value type arrays may contain oop or flattened representation
>> 345:       assert(array->is_obj_array_klass() || (FlatArrayFlatten && array->is_value_array_klass()),
>> 346:           "must be value or object array type");
> 
> Shouldn't is_value_array_klass() be is_flat_array_klass()?

I think is_value_array_klass() is correct because array is a ciType*, and I did not rename ci related functions.

> src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 813:
> 
>> 812:   index_check(r0, r1); // leaves index in r1, kills rscratch1
>> 813:   if (FlatArrayFlatten) {
>> 814:     Label is_flat_array, done;
> 
> FlatArrayFlatten is not very explicit.
> What about UseFlatArray?

Sure.  That's a better name.

> src/hotspot/share/runtime/globals.hpp line 769:
> 
>> 768:                                                                             \
>> 769:   product(intx, InlineArrayElemMaxFlatSize, -1,                             \
>> 770:           "Max size for flattening inline array elements, <0 no limit")     \
> 
> InlineArrayElementMaxFlatSize -> FlatArrayElementMaxSize ?

How about if I change InlineArrayElementMaxFlatSize and InlineArrayElemMaxFlatOops in the next change because changing
them affects a lot of tests and would make this change even bigger?

-------------

PR: https://git.openjdk.java.net/valhalla/pull/115


More information about the valhalla-dev mailing list