Improve performance in CharOperation #3386
Conversation
...ore.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CharOperationTest.java
Show resolved
Hide resolved
robstryker
force-pushed
the
CharOperation_replace_perf
branch
from
4a3ee4d to
fb37692
Compare
| next : for (int i = 0; i < max;) { | ||
| int index = indexOf(toBeReplaced, array, true, i); | ||
| if (index == -1) { | ||
| i++; |
There was a problem hiding this comment.
The actual bug is here: If index == -1, there is not hit. No need to scan again from the next offset.
What I like about this impl here: it's garbage-free whereas the proposed solution needs to encode the char arrays as ISO or UTF-8.
Do you have time to measure the pressure on the GC if your version of replace is used in tight loops?
There was a problem hiding this comment.
agree, that sounds like a better fix. One should compare the measurement for both solutions.
There was a problem hiding this comment.
If you guys are happier with a smaller change, then I'm fine with it. I'll just abort the loop at that point and be done with it.
There was a problem hiding this comment.
using "break" instead of "continue" and removing the "next" label would be prettier, but i am OK with this fix
robstryker
force-pushed
the
CharOperation_replace_perf
branch
2 times, most recently
from
2b0b3d2 to
c721a10
Compare
Signed-off-by: Rob Stryker <stryker@redhat.com>
robstryker
force-pushed
the
CharOperation_replace_perf
branch
from
3fe43e5 to
7dc30ce
Compare
When no matching characters / replacements are found in the string, the current replace method seems to be performing at an n^2 rate. The performance is actually really really bad.
Specifically, if the indexOf(etc) call returns -1, it clearly means there's no more work left to do here, but instead the existing logic continues to loop, and the indexOf(etc) call itself has a loop.
It seems not only needlessly complicated, but also inefficient.
This code seems to have originated in org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/pdom/util/CharArrayUtils.java, a file that was later renamed to org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/nd/util/CharArrayUtils.java.
The replace(etc) method in CharArrayUtils differs from that in CharOperation. The performance of the version in CharArrayUtils appears normal in comparison to that in CharOperation.
However, I can't tell how long ago this code was written, but, it would appear to me that simply using String functions performs comparably, so there's no reason whatsoever for all this complicated code?
What it does
How to test
Author checklist