RFR: JDK-8184947:,ZipCoder performance improvements (original) (raw)

Claes Redestad claes.redestad at oracle.com
Tue Dec 12 17:58:31 UTC 2017

Hi Sherman,

On 2017-12-11 05:08, Xueming Shen wrote:

http://cr.openjdk.java.net/~sherman/8184947/webrev

thanks for incorporating my suggestion!

It looks pretty good to me.  While many parts is just code that has been moved, this is still a pretty big change, so I hope we can get at least another pair of eyes on it.

StringCoding.java:

private static void throwMalformed(int nb) { throw new IllegalArgumentException("malformed input length : " + nb); }

nb is the number of bytes of the first offending chunk of bytes(?); is this information generally useful? I think the error message can be improved as input length is ambiguous in this context, but I wouldn't mind if the nb parameter was dropped along with a simplification of the error message.

Indentation errors at lines 321, 324, 728, 746, 913

TestStringCoding.java:

Are the added System.out.println's intentional? Indentation.

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.

Right, as this can be cause for significant GC pressure then I see how a bit of ThreadLocal overhead is fine until we come up with something better.

Thanks!

/Claes

thanks, sherman

More information about the core-libs-dev mailing list