RFR: 8288327: Executable.hasRealParameterData should not be volatile [v2]
liach
duke at openjdk.org
Fri Jun 17 06:30:49 UTC 2022
On Fri, 17 Jun 2022 06:14:32 GMT, liach <duke at openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/reflect/Executable.java line 453:
>>
>>> 451:
>>> 452: private transient boolean hasRealParameterData;
>>> 453: private transient volatile Parameter[] parameters;
>>
>> These can probably be annotated with `@Stable` (see also [GH‑8742]):
>> Suggestion:
>>
>> private transient @Stable boolean hasRealParameterData;
>> private transient @Stable volatile Parameter[] parameters;
>>
>>
>> [GH‑8742]: https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/8742__;!!ACWV5N9M2RV99hQ!N5tsYl_TBRKmfUUQW8y4mWPDosNfRCnScec50IJaZ9-dE66PRrTOjlPZLtH9cYdAW6OVve-XK-uOXps9cx5N$
>
> Another approach would be to keep the parameter array and the boolean in a record, and mark the record field as stable, possibly like what's done in #6889. Keeping them in a record like:
>
> record ParameterData(Parameter[] parameters, boolean real) {}
>
> private transient @Stable ParameterData parameterData;
>
> private ParameterData parameterData() {
> ParameterData data = parameterData;
> if (data != null)
> return data;
> // Create parameter data, cache and return
> }
>
> boolean hasRealParameterData() { return parameterData().real(); }
> private Parameter[] privateGetParameters() { return parameterData().parameters(); }
>
>
> This code should be more maintainable in the long run as well, with more straightforward concurrency and caching model.
If my record approach is used, `hasRealParameterData()` and `privateGetParameters()` should be manually inlined instead, for code clarity. Alternatively, you can rename the components to like `sharedArray`/`sharedParameters`, `isReal` to make the call sites like `parameterData().sharedParameters().clone()` or `parameterData().isReal()` more descriptive.
Currently, `hasRealParameterData` is only used in `Executable::getAllGenericParameterTypes`, and `privateGetParameters` only in `Executable::getParameters` (except the unintuitive usage to initialize the parameter data), so the impact of such a refactor and inline would be small.
-------------
PR: https://git.openjdk.org/jdk/pull/9143
More information about the core-libs-dev
mailing list