Code (Pre-)Review for JEP 280: Indify String Concat
Andrej Golovnin
andrej.golovnin at gmail.com
Sat Nov 28 09:05:05 UTC 2015
Hi Aleksey,
>> http://cr.openjdk.java.net/~shade/8085796/webrev.jdk.04/
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
356 ARG,
The comma is not needed.
548 // Mock the recipe to reuse the concat generator code
549 StringBuilder sb = new StringBuilder();
550 for (int c = 0; c < concatType.parameterCount(); c++) {
551 sb.append(TAG_ARG);
552 }
553 recipe = sb.toString();
This can be rewritten as:
char[] value = new char[concatType.parameterCount()];
Arrays.fill(value, TAG_ARG);
recipe = new String(value);
And when you add to the JavaLangAccess interface a new method
newStringUnsafe(byte[] value, byte coder), then you can even use the
package private constructor String(byte[], byte) to avoid copying of
bytes. But I'm not sure if it is worthwhile.
558 int cCount = 0;
559 int oCount = 0;
560 for (int i = 0; i < recipe.length(); i++) {
561 char c = recipe.charAt(i);
562 if (c == TAG_CONST) cCount++;
563 if (c == TAG_ARG) oCount++;
564 }
This loop is only needed, when the recipe is already known. When you
move the lines 558 and 559 before the line
547 if (generateRecipe) {
then you can initialize oCount directly after the line 547:
oCount = concatType.parameterCount();
and move the for-loop (lines 560-564) into the else clause of the "if
(generateRecipe)" statement.
Btw. I would rename the variables cCount and oCount to constCount and argCount.
601 Key key = new Key(mt, rec);
602 MethodHandle mh = CACHE_ENABLE ? CACHE.get(key) : null;
If you rewrite this lines as:
601 Key key = null;
602 MethodHandle mh = CACHE_ENABLE ? CACHE.get(key = new
Key(mt, rec)) : null;
then you can avoid creation of Key objects when caching is disabled.
669 SIZED_EXACT(true, true),
The comma is not needed.
672 boolean sized, exact;
sized and exact should be final.
1619 private static Class<?> STRING_HELPER;
1620
1621 static {
1622 try {
1623 STRING_HELPER =
Class.forName("java.lang.StringConcatHelper");
1624 } catch (ClassNotFoundException e) {
1625 throw new AssertionError(e);
1626 }
Why STRING_HELPER is not final?
I have a small question to the Recipe class. Is it really better to
create elementsRev instead of using the old-school for-loop and
List.get(int) to loop over the list of RecipeElements in the reverse
order? If there is any reason, then could you please document it to
avoid stupid questions in the future? :-)
src/java.base/share/classes/java/lang/invoke/StringConcatException.java
31 private static final long serialVersionUID = 292L + 9L;
I see that the pattern for serialVersionUID like this is already used
in four other classes (BootstrapMethodError, MethodType,
WrongMethodTypeException, LambdaConversioinException). Does it have
some special meaning? I'm just curious.
Best regards,
Andrej Golovnin
More information about the core-libs-dev
mailing list