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

Frederic Parain frederic.parain at oracle.com
Wed Jul 22 14:08:17 UTC 2020



> On Jul 22, 2020, at 09:47, Harold Seigel <hseigel at openjdk.java.net> wrote:
> 
> 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.

OK. We’ll let the compiler team do the renaming.

> 
>> 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?


It doesn’t really matter if the name is changed in this patch or in the
next one. 

Thank you,

Fred


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



More information about the valhalla-dev mailing list