[8u] RFR(XS) JFR generate same symbol ID for different symbols (high probabily with CDS)

[Andrew Dinn]

Hi Hui,

On 03/10/2020 02:36, kalinshi(施慧) wrote:
> Please help review this JFR issue in 8u backport.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8253837
> 
> Webrev: http://cr.openjdk.java.net/~hshi/8253837/webrev/
Thanks for reporting this problem.

This does not look like a backport request. It is not linked to any
related upstream change. It seems like you are just trying to apply a
single line patch to jdk8u to fix a problem you have identified in the
hash table code.

I can see that it is possible that two different symbols might generate
the same hash, which is what appears to motivate your patch. However,
you have not really explained in what circumstances that problem will
happen. That makes it impossible to know whether the patch is really needed.

Also, if it is needed then it is important to know whether this is the
only thing that needs fixing or if more needs to be done. At the very
least I think you will need to patch both JfrSymbolId::equals methods.

I checked upstream to see if there was any precedent for making your
suggested change. I looked at the jdk11u code and the change to method
equals that you are requesting has actually been made as part of a
larger patch for this issue:

  https://bugs.openjdk.java.net/browse/JDK-8231081

Issue 8231081 was raised and fixed in jdk14 and then backported to
jdk11u and jdk13u. As you might expect it modifies both
JfrSymbolId::equals methods (except that in the upstream code the method
is called on_equals). However, it also makes a lot of related changes to
deal with leak profiling.

So, we need to decide whether the whole patch is needed or if we just
need to fix the hash table code. That means we really need to know more
about what is going wrong on jdk8u. Could you provide more details of
what caused the problem this patch is trying to fix and, if possible,
something that can be run to reproduce the problem?

Also, it might be a good idea for someone involved in applying the
original patch or backporting it to jdk11u to take a look at the jdk8u
code and see if 1) the whole patch is needed or 2) it would make sense
just to backport the changes to the equals methods. Perhaps Jaroslav? Or
maybe Goetz/Christoph who seem to have been involved in approving the
jdk11u backport

regards,


Andrew Dinn
-----------