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(';');
         }
     }

Reply via email to