RFR (S) 8134758: Final String field values should be trusted as stable
per.liden at oracle.com
Tue Sep 1 13:04:04 UTC 2015
On 2015-09-01 14:31, Aleksey Shipilev wrote:
> Hi Per,
> Yes, I wondered how String Deduplication works with the String.value
> stability. I guessed that as long as dedup is idempotent (e.g. it
> switches the char to the semantically the same char), we are fine.
> Even if we inline the "old" reference to char into the generated code,
> it seems to become the GC root on its own.
We do actually have to handle this as little bit of a special case. As
you say, it's correct that we're semantically safe if we replace one
char with another char containing the same bytes. However, during
the string dedup development we could see a significant performance drop
for many applications, because the String.equals/compare intrinsic no
longer found a match in the fast path. To avoid this we're actively
deduplicating string before they get interned (see
StringTable::intern()) and hence before the char of a String constant
gets inlined into the code. I.e. we make sure deduplication never change
an interned String because those chars can be inlined.
> And yes, this particular change does nothing new to that mechanics.
I must admit that I don't fully understand all implications of this
change (if any). I just want to make sure we're not introducing some
hard to detect performance regression (e.g. a fast path is no longer
taken) which would need special treatment like the case I described above.
> On 09/01/2015 02:43 PM, Per Liden wrote:
>> Hi Aleksey,
>> I'm trying to figure out if this has any impact on String deduplication
>> (which can change the String.value field), but I can't see any obvious
>> problems. Dedup already handles the case where GraphKit treats it as a
>> stable field and I don't see that this adds any new cases. Just wanted
>> to double check that you reached the same conclusion?
>> On 2015-08-31 16:59, Aleksey Shipilev wrote:
>>> I would like to make a forward move and make VM to trust all final
>>> String fields. I cannot quickly find the scenario where it helps current
>>> JDK -- there is only String.value field, which components are not
>>> treated as constants anyway. But, it helps a lot the upcoming Compact
>>> Strings change, which introduces String.coder field.
>>> String.value is actually handled as stable in the GraphKit with
>>> UseImplicitStableValues, but it does not affect "normal" Java code.
>>> Therefore, in a way, this change extends the same behavior to the normal
>>> code. See more here:
>>> Here is a patch:
>>> Passes JPRT and eyeballed assembly looks fine on Linux x86_64.
>>> Does the change itself look generic enough to consider straight in the
>>> mainline? Otherwise, we can keep it in Compact Strings sandbox, but it
>>> will eventually arrive back entangled in a much larger code change, and
>>> shall still require review.
More information about the hotspot-compiler-dev