On 12/10/17, 3:12 PM, Claes Redestad wrote:
Hi Sherman,
On 2017-12-09 00:09, Xueming Shen wrote:
Hi,
Please help review the changes for j.u.z.ZipCoder/JDK-8184947 (which
also includes
cleanup/improvement work in java.lang.StringCoding.java to speed up
general String
coding performance, especially for UTF8).
issue: https://bugs.openjdk.java.net/browse/JDK-8184947
webrev: http://cr.openjdk.java.net/~sherman/8184947/webrev
I've not fully reviewed this yet, but something struck me halfway
through: As the ASCII
fast-path is what's really important here, we could write that part
without ever having
to go via a StringCoding.Result.
On four of your ZipCodingBM micros this improves performance a bit
further (~10%):
diff -r 848591d85052
src/java.base/share/classes/java/lang/StringCoding.java
--- a/src/java.base/share/classes/java/lang/StringCoding.java Sun
Dec 10 18:48:21 2017 +0100
+++ b/src/java.base/share/classes/java/lang/StringCoding.java Sun
Dec 10 18:55:38 2017 +0100
@@ -937,7 +937,13 @@
* Throws iae, instead of replacing, if malformed or unmmappble.
*/
static String newStringUTF8NoRepl(byte[] bytes, int off, int len) {
- Result ret = decodeUTF8(bytes, off, len, false);
+ if (COMPACT_STRINGS && !hasNegatives(bytes, off, len)) {
+ return new String(Arrays.copyOfRange(bytes, off, off +
len), LATIN1);
+ }
+ Result ret = decodeUTF8_0(bytes, off, len, false);
return new String(ret.value, ret.coder);
}
Hi Claes,
You're definite right on this one. We dont need the Result for the ASCII
case.
webrev has been updated accordingly.
http://cr.openjdk.java.net/~sherman/8184947/webrev
Yes, understood the threadlocal might not be the best choice here. But
just feel
something need to be done for the temporary Result object after observed its
usage with the jfr in #8184947. It is taking as many spaces as the
overall String
objects do. Sure, it's in young-gen, should be wiped with quickly. But
my take is
it might be worth the tradeoff of having each/every new String/cs) get a
little slower
instead of having the "global" vm has to do some extra clean up for this
extra
Result object, for now.
thanks,
sherman