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