On 02/11/21 10:29 am, Jaikiran Pai wrote:
Hello Alan,
On 01/11/21 9:22 pm, Alan Bateman wrote:
On Mon, 1 Nov 2021 03:10:45 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
Can I please get a review for this change which fixes the issue
reported in https://bugs.openjdk.java.net/browse/JDK-8275509?
The `ModuleDescriptor.hashCode()` method uses the hash code of its
various components to compute the final hash code. While doing so
it ends up calling hashCode() on (collection of) various `enum`
types. Since the hashCode() of enum types is generated from a call
to `java.lang.Object.hashCode()`, their value isn't guaranteed to
stay the same across multiple JVM runs.
The commit here proposes to use the ordinal of the enum as the hash
code. As Alan notes in the mailing list discussion, any changes to
the ordinal of the enum (either due to addition of new value,
removal of a value or just reordering existing values, all of which
I think will be rare in these specific enum types) isn't expected
to produce the same hash code across those changed runtimes and
that should be okay.
The rest of the ModuleDescriptor.hashCode() has been reviewed too
and apart from calls to the enum types hashCode(), rest of the
calls in that implementation, either directly or indirectly end up
as calls on `java.lang.String.hashCode()` and as such aren't
expected to cause non-deterministic values.
A new jtreg test has been added to reproduce this issue and verify
the fix.
Jaikiran Pai has updated the pull request incrementally with one
additional commit since the last revision:
better method name in test class
Thanks for the update to the test. Instead of launching 50 VMs, what
would you think about tossing the use of ProcessTools and just
changing the test description so that the test runs twice, once with
the default, the second with CDS disabled, as, in:
@run ModuleDescriptorHashCodeTest
@run main/othervm -Xshare:off ModuleDescriptorHashCodeTest
When I started off with this test, I had thought of a similar
construct. However, in order to compare the hash code value generated
across JVM runs (i.e. the one generated in @run 1 against the one
generated in @run 2), I would need to somehow hold on to that dynamic
value/result for comparison. Using the @run construct wouldn't allow
me to do that. So I decided to use the ProcessTools library to have
more control over it. If I missed some way to still use @run for such
a test, do let me know, I'll change it accordingly.
Perhaps run 1 writing the hash code of each of the boot modules'
descriptor into a file and then run 2 reading that same file to compare
the hash codes would be one way to do it. But I think that would just
make this test more complex, which I think is avoidable.
-Jaikiran
