On Thu, 21 Jan 2021 22:43:50 GMT, Naoto Sato wrote:
>> Claes Redestad has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Reduce code duplication in getBytes/getBytesNoRepl
>
> Marked as reviewed by naoto (Reviewer).
Passed testing tiers 1-
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Thu, 21 Jan 2021 20:48:33 GMT, Claes Redestad wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> over
On Thu, 21 Jan 2021 20:48:33 GMT, Claes Redestad wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> over
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Mon, 18 Jan 2021 09:26:07 GMT, Claes Redestad wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> over
On Mon, 18 Jan 2021 09:26:07 GMT, Claes Redestad wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> over
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Sun, 17 Jan 2021 15:51:58 GMT, Peter Levart wrote:
> `newStringUTF8NoRepl()` does not contain this optimization.
Good catch: this optimization was in the original code for `newStringNoRepl`
but not `newStringUTF8NoRepl`. I'll put it back to avoid a regression, but this
time into `newStr
On Sun, 17 Jan 2021 14:56:40 GMT, Peter Levart wrote:
>> Claes Redestad has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Simplify lookupCharset
>
> This looks good.
> Are you planning to do similar things for encoding too?
I already appr
On Sun, 17 Jan 2021 12:07:03 GMT, Claes Redestad wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> over
On Sun, 17 Jan 2021 09:21:27 GMT, Peter Levart wrote:
>> This looks much better now. Maybe just one small suggestion:
>> `java.lang.StringCoding#lookupCharset` is used in two places and in both
>> places the same exception handling/rethrowing logic is wrapped around the
>> invocation. So you c
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Sun, 17 Jan 2021 09:03:30 GMT, Peter Levart wrote:
>>> Do you think this code belongs more to String than to StringCoding?
>>
>> I consider StringCoding an implementation detail of String, so I'm not sure
>> there's (much) value in maintaining the separation of concern if it is cause
>> for
On Sat, 16 Jan 2021 01:02:20 GMT, Claes Redestad wrote:
>> Some common logic is now extracted into methods. That's good. But much of
>> the decoding logic still remains in String. I still think all of static
>> methods can be moved to StringCoding directly as they are now while the
>> private
On Sat, 16 Jan 2021 00:25:24 GMT, Peter Levart wrote:
> Do you think this code belongs more to String than to StringCoding?
I consider StringCoding an implementation detail of String, so I'm not sure
there's (much) value in maintaining the separation of concern if it is cause
for a performance
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Fri, 15 Jan 2021 22:03:31 GMT, Claes Redestad wrote:
>> Hi Claes,
>> This is quite an undertaking in re-factoring!
>> I think that decoding logic that you produced can still be kept in
>> StringCoding class though. The private constructor that you created for
>> UTF-8 decoding is unnecessary
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Fri, 15 Jan 2021 21:39:00 GMT, Peter Levart wrote:
> WDYT?
I get that the approach I took got a bit messy, but I've just spent some time
cleaning it up. Please have a look at the latest, which outlines much of the
logic and consolidates the replace/throw logic in the UTF8 decode paths. I've
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
> got a bit worse after JDK-8258596 which addresses a leak
On Fri, 15 Jan 2021 19:14:06 GMT, Naoto Sato wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> overhead
On Fri, 15 Jan 2021 20:05:17 GMT, Claes Redestad wrote:
>> src/java.base/share/classes/java/lang/String.java line 544:
>>
>>> 542: return;
>>> 543: }
>>> 544: if (charset == UTF_8) {
>>
>> The constructor is getting big. Might be better to keep the original private
On Fri, 15 Jan 2021 20:14:48 GMT, Roger Riggs wrote:
>> Since we're calculating two final values, that split was what necessitated a
>> `Result` object. Until valhalla I don't think there's a way to get rid of
>> the performance cost here without putting the bulk of the logic into the
>> const
On Fri, 15 Jan 2021 19:11:38 GMT, Naoto Sato wrote:
>> The `StringCoding.resultCached` mechanism is used to remove the allocation
>> of a `StringCoding.Result` object on potentially hot paths used in some
>> `String` constructors. Using a `ThreadLocal` has overheads though, and the
>> overhead
On Fri, 15 Jan 2021 14:33:19 GMT, Claes Redestad wrote:
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
>
On Fri, 15 Jan 2021 16:49:57 GMT, Roger Riggs wrote:
> Interesting, I was/am in the middle of converting Result to be a Valhalla
> primitive class to reduce the memory pressure and had written some new jmh
> tests too.
> And to reduce the dependency on ThreadLocals.
Ok, I expect that would end
On Fri, 15 Jan 2021 14:33:19 GMT, Claes Redestad wrote:
> The `StringCoding.resultCached` mechanism is used to remove the allocation of
> a `StringCoding.Result` object on potentially hot paths used in some `String`
> constructors. Using a `ThreadLocal` has overheads though, and the overhead
>
32 matches
Mail list logo