Hello,
several days ago I’ve proposed some improvements into asm library:
- https://gitlab.ow2.org/asm/asm/commit/ef945457bc19dbc09c02bd21b52f1950990f33f7
- https://gitlab.ow2.org/asm/asm/commit/9b914d524b7fe7ea694fc11ec784e67133ba383f
The patches were accepted and then it turned out the code I’ve patched is
similar to what we have in JDK.
So I propose the same changes to be accepted into JDK 12. They improve code by
using more simple constructions and replacing redundant hand-written code with
JDK-provided API.
As of performance, usage of String::replace instead of handwritten replacement
(see Type::appendDescriptor) is much faster and memory-effective (tested with
JDK 11) even on simplest class names like java.lang.String:
Benchmark Mode Cnt
Score Error Units
CharacterReplaceBenchmark.manualReplace avgt 25
57,036 ± 1,132 ns/op
CharacterReplaceBenchmark.stringReplace avgt 25
17,648 ± 0,699 ns/op
CharacterReplaceBenchmark.manualReplace:·gc.alloc.rate.norm avgt 25
112,000 ± 0,001 B/op
CharacterReplaceBenchmark.stringReplace:·gc.alloc.rate.norm avgt 25
56,000 ± 0,001 B/op
Corresponding benchmark available here:
https://github.com/stsypanov/benchmarks/blob/master/benchmark-runners/src/main/java/com/luxoft/logeek/benchmark/string/CharacterReplaceBenchmark.java
https://github.com/stsypanov/benchmarks/blob/master/benchmark-source/src/main/java/com/luxoft/logeek/string/CharacterReplacer.java
Usage of String::substring instead of StringBuilder.append(CharSequence, int,
int) (see Type::getDescriptor) is also faster and memory-effective, results
described here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-January/057834.html
Also, is there any workload to measure effect of attached patch?
Regards,
Sergey Tsypanov
diff --git a/src/java.base/share/classes/jdk/internal/org/objectweb/asm/Type.java b/src/java.base/share/classes/jdk/internal/org/objectweb/asm/Type.java
--- a/src/java.base/share/classes/jdk/internal/org/objectweb/asm/Type.java
+++ b/src/java.base/share/classes/jdk/internal/org/objectweb/asm/Type.java
@@ -492,11 +492,7 @@
case DOUBLE:
return "double";
case ARRAY:
- StringBuilder stringBuilder = new StringBuilder(getElementType().getClassName());
- for (int i = getDimensions(); i > 0; --i) {
- stringBuilder.append("[]");
- }
- return stringBuilder.toString();
+ return getElementType().getClassName() + "[]".repeat(Math.max(0, getDimensions()));
case OBJECT:
case INTERNAL:
return valueBuffer.substring(valueBegin, valueEnd).replace('/', '.');
@@ -536,11 +532,7 @@
if (sort == OBJECT) {
return valueBuffer.substring(valueBegin - 1, valueEnd + 1);
} else if (sort == INTERNAL) {
- return new StringBuilder()
- .append('L')
- .append(valueBuffer, valueBegin, valueEnd)
- .append(';')
- .toString();
+ return 'L' + valueBuffer.substring(valueBegin, valueEnd) + ';';
} else {
return valueBuffer.substring(valueBegin, valueEnd);
}
@@ -663,12 +655,7 @@
stringBuilder.append(descriptor);
} else {
stringBuilder.append('L');
- String name = currentClass.getName();
- int nameLength = name.length();
- for (int i = 0; i < nameLength; ++i) {
- char car = name.charAt(i);
- stringBuilder.append(car == '.' ? '/' : car);
- }
+ stringBuilder.append(getInternalName(currentClass));
stringBuilder.append(';');
}
}