RFR: 8272579: G1: remove unnecesary null check in G1ParScanThreadStateSet::flush
Albert Mingkun Yang
ayang at openjdk.java.net
Wed Aug 18 13:04:28 UTC 2021
On Wed, 18 Aug 2021 12:48:20 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:
>> Simple change of replacing a null-check with an assertion.
>>
>> Test: hotspot_gc
>
> src/hotspot/share/gc/g1/g1ParScanThreadState.cpp line 561:
>
>> 559: for (uint worker_id = 0; worker_id < _n_workers; ++worker_id) {
>> 560: G1ParScanThreadState* pss = _states[worker_id];
>> 561: assert(pss != nullptr, "must be initialized");
>
> Is it guaranteed that all workers are initialized? As you state in the bug, they are lazily initialized, so I wonder if there are use cases where a worker would not get initialized during the collection. Something like, very little work and a lot of workers.
>
> If there is a guarantee here, does it hold below in `record_unused_optional_region()` as well? We are using the same construct with `continue`there.
In `G1CollectedHeap::evacuate_initial_collection_set`:
G1ParScanThreadStateSet per_thread_states(this,
&rdcqs,
workers()->active_workers(), <---- setting up states for workers
collection_set()->young_region_length(),
collection_set()->optional_region_length());
...
evacuate_initial_collection_set(&per_thread_states, may_do_optional_evacuation);
\
-- task_time = run_task_timed(&g1_par_task); <--- launching all workers
Therefore, I think the assertion is safe.
> does it hold below in record_unused_optional_region() as well?
I think so.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5145
More information about the hotspot-gc-dev
mailing list