On 04/23/2018 11:18 PM, David Lloyd wrote:
FWIW I strongly doubt this will improve performance; probably the
opposite in fact, as IIRC an enum switch generates an extra class
(though perhaps this has changed).
switch on enum is basically a switch on Enum's ordinal mapped via the
int[] array constructed in a synthetic nested class of the class
containing the switch statement, so that separate compilation of an Enum
class that changes ordinal values of particular enum constants doesn't
change the semantics of the switch logic. For example...
// X.java
public enum X {
A, B, C
}
// Code.java
public class Code {
public void m(X x) {
switch (x) {
case A: // ... branch A
break;
case B: // ... branch B
break;
case C: // ... branch C
break;
default: // ... default branch
}
}
}
...Code.java translates to something like the following (modulo error
handling)...
public class Code {l
static class m$switch$1 {
static final int[] caseindex = new int[X.values().length];
static {
caseindex[X.A.ordinal()] = 1;
caseindex[X.B.ordinal()] = 2;
caseindex[X.C.ordinal()] = 3;
}
}
public void m(X x) {
switch (m$switch$1.caseindex[x.ordinal()]) {
case 1: // ... branch A
break;
case 2: // ... branch B
break;
case 3: // ... branch C
break;
default: // ... default branch }
}
}
Pefrormance-wise this is similar to switch on int. So pretty optimal.
Decision should only be made on the count of code clarity here.
While speaking of performance of enum switches, I have a question for a
more knowledgeable person...
Should JIT be able to fold above 'x' variable/parameter into a constant
(suppose m() was called in a loop with an explicitly specified constant
value such as X.A), it could further expand this decision to the
x.ordinal() value (if the final instance 'ordinal' field in the X enum
class was marked with @Stable), it could then further expand this
decision to the looked up value of the m$switch$1.caseindex[] slot if
caseindex array field in sytnhetic class was marked with @Stable,
therefore transforming the switch to a switch on int constant,
optimizing the JITed code to directly execute branch A and eliminate all
other branches from generated code.
The question is whether JIT is presently treating those fields (and
array) as @Stable or not?
Regards, Peter
The original code was quite
compact and utilized identity comparisons, and given there are only
three alternatives it probably was able to exploit branch prediction
as well (if such a thing even matters in this context).
I'm not a reviewer but this change seems pointless without supporting perf data.
On Mon, Apr 23, 2018 at 4:05 PM, Xueming Shen <xueming.s...@oracle.com> wrote:
I would assume in case of an exception thrown from
appendExpandedReplacement() we don't
want "text" to be pushed into the "sb".
-sherman
On 4/23/18, 1:49 PM, Isaac Levy wrote:
Thanks. Another small perf patch below -- maybe we can combine. Avoids a
StringBuilder allocation:
--- a/src/java.base/share/classes/java/util/regex/Matcher.java
+++ b/src/java.base/share/classes/java/util/regex/Matcher.java
@@ -993,13 +993,11 @@
public Matcher appendReplacement(StringBuilder sb, String replacement)
{
// If no match, return error
if (first < 0)
throw new IllegalStateException("No match available");
- StringBuilder result = new StringBuilder();
- appendExpandedReplacement(replacement, result);
// Append the intervening text
sb.append(text, lastAppendPosition, first);
// Append the match substitution
+ appendExpandedReplacement(replacement, sb);
- sb.append(result);
On Mon, Apr 23, 2018 at 4:31 PM, Xueming Shen <xueming.s...@oracle.com
<mailto:xueming.s...@oracle.com>> wrote:
this looks fine.
-sherman
