RFR: 8240774: [REDO] G1DirtyCardQueue destructor has useless flush

[Kim Barrett]

On Tue, 30 May 2023 13:44:34 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Please review this change to VM initialization, to eliminate a workaround for
>> a bootstrapping problem.
>> 
>> A JavaThread must not touch oops and invoke barriers unless it is on the
>> threads list.  Barrier set operations on_thread_attach and on_thread_detach
>> bracket the time when a thread can do stuff with oops.
>> 
>> However, this restriction is currently violated by the main thread during VM
>> initialization.  It may (and indeed does) touch oops before it has been added
>> to the thread list, including executing post-barriers and enqueuing entries
>> in its DCQ.
>> 
>> That has led to two separate bootstrapping workarounds:
>> 
>> - ~G1DirtyCardQueue flushes the queue before proceeding to the base class
>> (PtrQueue) destructor. This is not normally needed, and is even
>> counter-productive, since it may hide a problem. But if an error is detected
>> during VM initialization that main thread gets deleted. Since it's not on the
>> threads list, there's no on_thread_detach to flush the queue. Without the
>> flush in the DCQ destructor that can lead to an assert in ~PtrQueue because
>> the queue isn't empty.
>> 
>> - on_thread_attach can't assert the queue is empty, even though normally it
>> must be.  But for the main thread, being added to the threads list (so calling
>> on_thread_attach) happens after it has enqueued entries in its DCQ.  So we
>> currently don't have those asserts.
>> 
>> To remove the need for these workarounds, this change moves the addition of
>> the main thread to the threads list earlier in the initialization sequence. It
>> needs to be before code that might execute barriers. After init_globals is too
>> late. But it can't be too early, before some relevant data structures are
>> initialized, so it can't be before init_globals. So this change splits
>> init_globals at a strategic point, and adds the main thread to the threads
>> list there. That point is just before the call to universe2_init, which calls
>> Universe::genesis.
>> 
>> With that change we can then remove the workarounds in ~G1DirtyCardQueue and
>> G1BarrierSet::on_thread_attach.
>> 
>> Testing:
>> mach5 tier1-5
>> 
>> Manually tested error handling for failures reported by init_globals and
>> init_globals2. This involved patching to force error reporting when
>> -XX:+UseNewCode. Handling errors from init_globals works as before, if a
>> failure happened in that part of the sequence. Handling errors from
>> init_globals2 runs afoul of pre-existing JDK-8308764.
>
> Marked as reviewed by ayang (Reviewer).

Thanks for reviews @albertnetymk , @tschatzl , and @dholmes-ora

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14119#issuecomment-1568624545