RFR: 8294982: Implementation of Classfile API [v12]

Maurizio Cimadamore mcimadamore at openjdk.org
Tue Feb 7 11:51:53 UTC 2023


On Mon, 6 Feb 2023 13:55:59 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/classfile/AttributedElement.java line 94:
>> 
>>> 92:      * are permitted.
>>> 93:      */
>>> 94:     enum Kind {
>> 
>> Not sure how to interpret this. This seems to refer to an attribute - e.g. where is an attribute allowed - rather than to the element (e.g. the declaration to which the attribute is attached). All the constants are unused, so hard to make sense of how this is used.
>
> There are at least 72 usages of AttributedElement.Kind across the Classfile API.
> Do you suggest to rename it to something else (for example Location)?

Uhm - I can't see these usages... something seems to be off with my IDE configuration. I did a grep and I now saw the uses. That said, having the Kind/Location inside AttributedElement still looks weird to me. The "places where an attribute can appear" is a property of an `Attribute`, not of an attributed element (which is used to model elements which can have attributes).

>> src/java.base/share/classes/jdk/internal/classfile/Attributes.java line 774:
>> 
>>> 772:      */
>>> 773:     public static AttributeMapper<?> standardAttribute(Utf8Entry name) {
>>> 774:         int hash = name.hashCode();
>> 
>> If we have a map, why do we need this "inlined" map here? Performance reasons?
>
> Yes, performance is the main reason.
> I'll note to do a fresh differential performance benchmarks with a HashMap.

thanks

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

PR: https://git.openjdk.org/jdk/pull/10982



More information about the build-dev mailing list