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