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).  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
>
>



-- 
- DML

Reply via email to