On Tue, 15 Oct 2024 19:42:51 GMT, Chen Liang <li...@openjdk.org> wrote:
> I recommend adding javadoc or block comments indicating that `runtimeSetup`,
> `resetArchivedStates`, etc. are called by VM CDS code so people don't
> accidentally rename this and have unintended impact.
Done.
> src/java.base/share/classes/java/lang/invoke/MethodType.java line 404:
>
>> 402: MethodType primordialMT = new MethodType(rtype, ptypes);
>> 403: if (archivedMethodTypes != null) {
>> 404: MethodType mt = archivedMethodTypes.get(primordialMT);
>
> The use of `HashMap` naturally feels alarming given this method is invoked in
> a concurrent context. I just want to make sure that:
>
> 1. `HashMap` will never have structural modifications when read access is
> performed (some data structures like treap do). Don't know if we have a test
> to verify such an intended usage, as we would hurt badly if this implied
> property suddenly no longer exist.
> 2. The `archivedMethodTypes` must be properly published so we won't read a
> partial `HashMap`. I've heard of talks about concurrent VM bootstraping so I
> need some help to understand how this is ensured in bootstrap.
I added asserts to check that the HashMap is written only in the CDS dumping
process, so at least we know it's not being modified when we read from it.
The CDS archived heap is either mmaped or read into memory while the VM is
still in single threaded mode, so the entire content of the archived heap
should be published for to all future threads.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21143#issuecomment-2418466360
PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1804080520
