On Wed, 26 Apr 2023 15:04:50 GMT, Adam Sotona <asot...@openjdk.org> wrote:
> Following improvements implemented:
> - Switch over `String` replaced with switch single char
> - Binary search for frames in `StackMapGenerator`
> - `StackMapGenerator.rawHandlers` with pre-calculated offsets
> - `ClassEntry` is caching `ClassDesc` symbol
> - Caching of type symbols in `NameAndTypeEntry` and `MethodTypeEntry`
> - Caching `MethodTypeDesc` in `MethodInfo` implementations
> - `StackMapGenerator` and `Utils` delegating to cached `MethodTypeDesc`
> instead of custom parsing
>
> No API change.
>
> Benchmarks show stack map generation improved by 21% and code generation from
> symbols improved by 30%.
>
>
> Benchmark Mode Cnt Score Error Units
> GenerateStackMaps.benchmark thrpt 10 407931.202 ± 13101.023 ops/s
> RebuildMethodBodies.shared thrpt 4 10258.597 ± 383.699 ops/s
> RebuildMethodBodies.unshared thrpt 4 7224.543 ± 256.800 ops/s
>
>
>
> Benchmark Mode Cnt Score Error Units
> GenerateStackMaps.benchmark thrpt 10 495101.110 ± 2389.628 ops/s
> RebuildMethodBodies.shared thrpt 4 13380.272 ± 810.113 ops/s
> RebuildMethodBodies.unshared thrpt 4 9399.863 ± 557.060 ops/s
In order to avoid eager conversion of a `ClassDesc` to internal name, I suggest
extending `AbstractPoolEntry.Utf8EntryImpl` with `State.CLASS_DESC`:
class AbstractPoolEntry.Utf8EntryImpl extends AbstractPoolEntry implements
Utf8Entry {
enum State { CLASS_DESC, ... }
// Set in any state other than RAW or CLASS_DESC
private int hash;
// Set in any state other than RAW
private int charLen;
// Only set in CLASS_DESC state, must be an L-type Class descriptor
private ClassDesc classDesc;
Utf8EntryImpl(ConstantPool cpm, int index, ClassDesc cd) {
super(cpm, Classfile.TAG_UTF8, index, 0);
this.rawBytes = null;
this.offset = 0;
this.rawLen = 0;
String descriptor = cd.descriptorString();
if (cd.isArray()) {
this.state = State.STRING;
this.stringValue = descriptor;
this.charLen = descriptor.length();
this.hash = hashString(descriptor.hashCode());
} else {
assert cd.isClassOrInterface() : cd;
this.state = State.CLASS_DESC;
this.classDesc = cd;
this.charLen = descriptor.length() - 2;
}
}
private void inflateClassDesc() {
String internalName = Utils.toInternalName(this.classDesc);
assert this.charLen = internalName.length();
this.stringValue = internalName;
this.hash = hashString(internalName.hashCode());
this.state = State.STRING;
}
@Override
public Utf8EntryImpl clone(ConstantPoolBuilder cp) {
if (cp.canWriteDirect(constantPool))
return this;
if (state = State.CLASS_DESC) {
return (Utf8EntryImpl) cp.utf8Entry(classDesc);
}
return (state == State.STRING && rawBytes == null)
? (Utf8EntryImpl) cp.utf8Entry(stringValue)
: ((SplitConstantPool) cp).maybeCloneUtf8Entry(this);
}
@Override
public int hashCode() {
if (state == State.CLASS_DESC) {
inflateClassDesc();
}
...
}
@Override
public String toString() {
if (state == State.CLASS_DESC) {
inflateClassDesc();
}
...
}
@Override
public char charAt(int index) {
if (state == State.CLASS_DESC) {
return classDesc.descriptorString()
.charAt(Objects.checkIndex(index, charLen) + 1);
}
...
}
@Override
public boolean equalsString(String s) {
if (state == State.CLASS_DESC) {
inflateClassDesc();
}
...
}
@Override
public void writeTo(BufWriter pool) {
if (rawBytes != null) {
...
}
else {
if (state == State.CLASS_DESC) {
inflateClassDesc();
}
...
}
}
}
src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java
line 586:
> 584: sym = Util.toClassDesc(asInternalName());
> 585: }
> 586: return sym;
Suggestion:
var sym = this.sym;
if (sym != null) {
return sym;
}
return this.sym = Util.toClassDesc(asInternalName());
src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java
line 988:
> 986: sym =
> MethodTypeDesc.ofDescriptor(descriptor().stringValue());
> 987: }
> 988: return sym;
Suggestion:
var sym = this.sym;
if (sym != null) {
return sym;
}
return this.sym =
MethodTypeDesc.ofDescriptor(descriptor().stringValue());
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1527967835
PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1178367689
PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1178369318
