On Mon, 21 Nov 2022 12:17:02 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>> Erik Österlund has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix Richard comments > > I went through the changes and all looks good to me. Only minor comments. > > Thanks, > Patricio Thanks for the review @pchilano! I made the changes you requested. > src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1393: > >> 1391: // Guaranteed to be in young gen / newly allocated memory >> 1392: assert(!chunk->requires_barriers(), "Unfamiliar GC requires >> barriers on TLAB allocation"); >> 1393: _barriers = false; > > Do we need to explicitly set _barriers to false? It's already initialized to > be false (same above for the UseZGC case). That would also allow to simplify > the code a bit I think to be just an if statement that calls > requires_barriers() for the "ZGC_ONLY(!UseZGC &&) > (SHENANDOAHGC_ONLY(UseShenandoahGC ||) allocator.took_slow_path())" case, and > then ZGC and the fast path could use just separate asserts outside > conditionals. It's mainly there to improve readability at the moment. The simplification you have in mind applies well now, but unfortunately doesn't apply well for generational ZGC. And the main point of this PR is to prepare for generational ZGC integration. So I would prefer to leave it the way it is, if you are okay with that? ------------- PR: https://git.openjdk.org/jdk/pull/11111
