Am 21.03.2013 00:22, schrieb Martin Buchholz:
On Wed, Mar 20, 2013 at 7:09 AM, Ulf Zibis <ulf.zi...@cosoco.de
<mailto:ulf.zi...@cosoco.de>> wrote:
AbstractStringBuilder:
Instead
270 public int codePointBefore(int index) {
271 int i = index - 1;
272 if ((i < 0) || (i >= count)) {
273 throw new StringIndexOutOfBoundsException(index);
274 }
I suggest
270 public int codePointBefore(int index) {
271 if ((--index < 0) || (index >= count)) {
272 throw new StringIndexOutOfBoundsException(index);
273 }
, because if e.g. the initial value of index is 0, then -1 reflects the
out-of-bound
condition, but not the initial 0 to report in the
StringIndexOutOfBoundsException.
(Hopefully the following redundant i < 0 bounds check from value[i] becomes
elimited by JIT.)
I was impressed that hotspot could indeed remove the redundant bounds checks.
Only with -Xint was I able to demonstrate a performance win.
Thanks!
But what's about the "wrong" exception index reporting, e.g. 0 as "out of
bounds"?
If there is some register pressure, you could avoid potential register
swapping for temp,
temp2, hasSurrogate, j and n - j if you would reorder following lines to:
1390 char temp = value[j];
1391 char temp2 = value[j] = value[n - j];
1397 value[n - j] = temp;
1392 if (!hasSurrogate) {
1393 hasSurrogate = (Character.isSurrogate(temp) ||
1394 Character.isSurrogate(temp2));
1395 }
(Nomination for "performance expert II" ?)
Thanks for making me beat my head against the hotspot optimization wall!
The latest version of reverse in my webrev, after way too much fiddling, is 20%
faster,
even though I'm not quite sure why.
No surprise to me. In ordinary strings (no surrogate pairs), always 3 checks should be processed
each loop:
- if (!hasSurrogate)
- Character.isSurrogate(temp)
- Character.isSurrogate(temp2)
The "if (!hasSurrogate)" only helps a little, if a surrogate pair was found in 1st half of the
string, but the gain should be small in relation to the extra work, scanning the string again for
reversing them.
Did you try the more smart syntax? :
hasSurrogate =
(Character.isSurrogate(cj) || Character.isSurrogate(ck));
If you write tests, I will incorporate into this changeset!
I have no java development environment here and small time, so depends on your patience to commit
the changeset.
-Ulf
