Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce hash collisions when interning type variable references #347

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading