From 81859734517f338a9e4998d4b27c8964692e951e Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Thu, 24 Aug 2023 13:37:58 +0200 Subject: [PATCH] reduce hash collisions when interning type variable references --- .../jboss/jandex/GenericSignatureParser.java | 10 +++++---- .../java/org/jboss/jandex/IndexReaderV2.java | 8 +++++-- .../java/org/jboss/jandex/IndexWriterV2.java | 6 ++++- .../main/java/org/jboss/jandex/Indexer.java | 7 +++--- .../jboss/jandex/TypeVariableReference.java | 22 ++++++++++++++----- doc/modules/ROOT/pages/index.adoc | 3 +++ 6 files changed, 41 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/jboss/jandex/GenericSignatureParser.java b/core/src/main/java/org/jboss/jandex/GenericSignatureParser.java index 582f7534..127fe1e4 100644 --- a/core/src/main/java/org/jboss/jandex/GenericSignatureParser.java +++ b/core/src/main/java/org/jboss/jandex/GenericSignatureParser.java @@ -124,6 +124,7 @@ class GenericSignatureParser { private Map typeParameters; private Map elementTypeParameters = new HashMap(); private Map classTypeParameters = new HashMap(); + private DotName currentClassName; // used to track enclosing type variables when determining if a type is recursive // and when patching type variable references; present here to avoid allocating @@ -248,7 +249,8 @@ public String toString() { } - void beforeNewClass() { + void beforeNewClass(DotName className) { + this.currentClassName = className; this.classTypeParameters.clear(); this.elementTypeParameters.clear(); } @@ -257,8 +259,8 @@ void beforeNewElement() { this.elementTypeParameters.clear(); } - ClassSignature parseClassSignature(String signature) { - beforeNewClass(); + ClassSignature parseClassSignature(String signature, DotName className) { + beforeNewClass(className); this.signature = signature; this.typeParameters = this.classTypeParameters; this.pos = 0; @@ -567,7 +569,7 @@ private Type resolveType(Type type, boolean isRecursive) { if (type.kind() == Type.Kind.UNRESOLVED_TYPE_VARIABLE) { String identifier = type.asUnresolvedTypeVariable().identifier(); if (isRecursive && typeParameters.containsKey(identifier)) { - return new TypeVariableReference(identifier); + return new TypeVariableReference(identifier, currentClassName); } else if (elementTypeParameters.containsKey(identifier)) { return elementTypeParameters.get(identifier); } else if (classTypeParameters.containsKey(identifier)) { diff --git a/core/src/main/java/org/jboss/jandex/IndexReaderV2.java b/core/src/main/java/org/jboss/jandex/IndexReaderV2.java index 2265340d..4fd7bc36 100644 --- a/core/src/main/java/org/jboss/jandex/IndexReaderV2.java +++ b/core/src/main/java/org/jboss/jandex/IndexReaderV2.java @@ -50,7 +50,7 @@ */ final class IndexReaderV2 extends IndexReaderImpl { static final int MIN_VERSION = 6; - static final int MAX_VERSION = 11; + static final int MAX_VERSION = 12; private static final byte NULL_TARGET_TAG = 0; private static final byte FIELD_TAG = 1; private static final byte METHOD_TAG = 2; @@ -432,8 +432,12 @@ private Type readTypeEntry(PackedDataInputStream stream, Map= 12) { + className = nameTable[stream.readPackedU32()]; + } AnnotationInstance[] annotations = readAnnotations(stream, null); - TypeVariableReference reference = new TypeVariableReference(identifier, null, annotations); + TypeVariableReference reference = new TypeVariableReference(identifier, null, annotations, className); references.put(reference, position); return reference; } diff --git a/core/src/main/java/org/jboss/jandex/IndexWriterV2.java b/core/src/main/java/org/jboss/jandex/IndexWriterV2.java index 6af4b758..4b876eb8 100644 --- a/core/src/main/java/org/jboss/jandex/IndexWriterV2.java +++ b/core/src/main/java/org/jboss/jandex/IndexWriterV2.java @@ -54,7 +54,7 @@ */ final class IndexWriterV2 extends IndexWriterImpl { static final int MIN_VERSION = 6; - static final int MAX_VERSION = 11; + static final int MAX_VERSION = 12; // babelfish (no h) private static final int MAGIC = 0xBABE1F15; @@ -877,6 +877,9 @@ private void writeTypeEntry(PackedDataOutputStream stream, Type type) throws IOE TypeVariableReference reference = type.asTypeVariableReference(); stream.writePackedU32(positionOf(reference.identifier())); stream.writePackedU32(positionOf(reference.follow())); + if (version >= 12) { + stream.writePackedU32(positionOf(reference.internalClassName())); + } } break; } @@ -1103,6 +1106,7 @@ private void addType(Type type) { break; case TYPE_VARIABLE_REFERENCE: addString(type.asTypeVariableReference().identifier()); + addClassName(type.asTypeVariableReference().internalClassName()); // do _not_ add the referenced type, it will be added later // and adding it recursively here would result in an infinite regress break; diff --git a/core/src/main/java/org/jboss/jandex/Indexer.java b/core/src/main/java/org/jboss/jandex/Indexer.java index 24fc9253..ee0f345b 100644 --- a/core/src/main/java/org/jboss/jandex/Indexer.java +++ b/core/src/main/java/org/jboss/jandex/Indexer.java @@ -1814,7 +1814,7 @@ private void applySignatures() { int end = signatures.size(); // Class signature should be processed first to establish class type parameters - signatureParser.beforeNewClass(); + signatureParser.beforeNewClass(currentClass.name()); if (classSignatureIndex >= 0) { String elementSignature = (String) signatures.get(classSignatureIndex); Object element = signatures.get(classSignatureIndex + 1); @@ -1844,7 +1844,7 @@ private void applySignatures() { private void parseClassSignature(String signature, ClassInfo clazz) { GenericSignatureParser.ClassSignature classSignature; try { - classSignature = signatureParser.parseClassSignature(signature); + classSignature = signatureParser.parseClassSignature(signature, clazz.name()); } catch (Exception e) { // invalid generic signature // let's just pretend that no signature exists @@ -2720,7 +2720,8 @@ private Type propagateOneTypeParameterBound(Type type, Type[] allTypeParams, Ann private Type deepCopyTypeIfNeeded(Type type) { if (type.kind() == Type.Kind.TYPE_VARIABLE_REFERENCE) { // type variable references must be patched by the caller, so no need to set target here - return new TypeVariableReference(type.asTypeVariableReference().identifier(), null, type.annotationArray()); + return new TypeVariableReference(type.asTypeVariableReference().identifier(), null, type.annotationArray(), + type.asTypeVariableReference().internalClassName()); } else if (type.kind() == Type.Kind.TYPE_VARIABLE) { TypeVariable typeVariable = type.asTypeVariable(); Type[] bounds = typeVariable.boundArray(); diff --git a/core/src/main/java/org/jboss/jandex/TypeVariableReference.java b/core/src/main/java/org/jboss/jandex/TypeVariableReference.java index df7936cc..93e427e5 100644 --- a/core/src/main/java/org/jboss/jandex/TypeVariableReference.java +++ b/core/src/main/java/org/jboss/jandex/TypeVariableReference.java @@ -1,5 +1,7 @@ package org.jboss.jandex; +import java.util.Objects; + /** * Represents a reference to a type variable in the bound of a recursive type parameter. * For example, if a class or method declares a type parameter {@code T extends Comparable}, @@ -40,14 +42,20 @@ public final class TypeVariableReference extends Type { private final String name; private TypeVariable target; - TypeVariableReference(String name) { - this(name, null, null); + // name of the class in which this type variable reference exists + // this is only to reduce interning hash collisions, doesn't serve any other purpose + private final DotName internalClassName; + + TypeVariableReference(String name, DotName internalClassName) { + this(name, null, null, internalClassName); } - TypeVariableReference(String name, TypeVariable target, AnnotationInstance[] annotations) { + TypeVariableReference(String name, TypeVariable target, AnnotationInstance[] annotations, DotName internalClassName) { super(DotName.OBJECT_NAME, annotations); this.name = name; this.target = target; + + this.internalClassName = internalClassName; } @Override @@ -86,6 +94,10 @@ public TypeVariable follow() { return target; } + DotName internalClassName() { + return internalClassName; + } + @Override public Kind kind() { return Kind.TYPE_VARIABLE_REFERENCE; @@ -98,7 +110,7 @@ public TypeVariableReference asTypeVariableReference() { @Override Type copyType(AnnotationInstance[] newAnnotations) { - return new TypeVariableReference(name, target, newAnnotations); + return new TypeVariableReference(name, target, newAnnotations, internalClassName); } void setTarget(TypeVariable target) { @@ -155,6 +167,6 @@ boolean internEquals(Object o) { @Override int internHashCode() { // must produce predictable hash code (for reproducibility) consistent with `internEquals` - return name.hashCode(); + return Objects.hash(name, internalClassName); } } diff --git a/doc/modules/ROOT/pages/index.adoc b/doc/modules/ROOT/pages/index.adoc index 11665fa0..034baa3d 100644 --- a/doc/modules/ROOT/pages/index.adoc +++ b/doc/modules/ROOT/pages/index.adoc @@ -62,6 +62,9 @@ It is also a maximum persistent index format version the given Jandex version ca |=== |Jandex version |Persistent format version +|Jandex 3.2.x +|12 + |Jandex 3.0.x, 3.1.x |11