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
