Skip to content

Commit

Permalink
Merge pull request #347 from Ladicek/type-variable-reference-intern-h…
Browse files Browse the repository at this point in the history
…ash-collisions

Reduce hash collisions when interning type variable references
  • Loading branch information
Ladicek authored May 13, 2024
2 parents 570ebb9 + 8185973 commit ab584f3
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 15 deletions.
10 changes: 6 additions & 4 deletions core/src/main/java/org/jboss/jandex/GenericSignatureParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class GenericSignatureParser {
private Map<String, TypeVariable> typeParameters;
private Map<String, TypeVariable> elementTypeParameters = new HashMap<String, TypeVariable>();
private Map<String, TypeVariable> classTypeParameters = new HashMap<String, TypeVariable>();
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
Expand Down Expand Up @@ -248,7 +249,8 @@ public String toString() {

}

void beforeNewClass() {
void beforeNewClass(DotName className) {
this.currentClassName = className;
this.classTypeParameters.clear();
this.elementTypeParameters.clear();
}
Expand All @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
8 changes: 6 additions & 2 deletions core/src/main/java/org/jboss/jandex/IndexReaderV2.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -432,8 +432,12 @@ private Type readTypeEntry(PackedDataInputStream stream, Map<TypeVariableReferen
case TYPE_VARIABLE_REFERENCE: {
String identifier = stringTable[stream.readPackedU32()];
int position = stream.readPackedU32();
DotName className = null;
if (version >= 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;
}
Expand Down
6 changes: 5 additions & 1 deletion core/src/main/java/org/jboss/jandex/IndexWriterV2.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions core/src/main/java/org/jboss/jandex/Indexer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
22 changes: 17 additions & 5 deletions core/src/main/java/org/jboss/jandex/TypeVariableReference.java
Original file line number Diff line number Diff line change
@@ -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<T>},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -86,6 +94,10 @@ public TypeVariable follow() {
return target;
}

DotName internalClassName() {
return internalClassName;
}

@Override
public Kind kind() {
return Kind.TYPE_VARIABLE_REFERENCE;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
}
3 changes: 3 additions & 0 deletions doc/modules/ROOT/pages/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit ab584f3

Please sign in to comment.