[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