RFR: 8302760: Improve liveness/remembered set verification for G1 [v4]

Thomas Schatzl tschatzl at openjdk.org
Wed Feb 22 16:09:44 UTC 2023


> Hi all,
> 
>   can I have reviews for this refactoring/cleanup of the liveness/remembered set verification code for G1?
> 
> Here is an overview of the changes
>  - previously liveness and remembered set verification could be done separately. Thismade the code a bit more complicated than necessary; now that remembered set verification always follows liveness verification, the code can assume that and do some minor cleanup. I kept the remembered verification a separate closure (`VerifyRemSetClosure`) because the additional boiler plate code is negligible compared to the other duplication that would have been needed around failure count tracking.
>  - factored out liveness and remembered set verification from `HeapRegion::verify`, and heavily cleaned up the resulting `HeapRegion::verify_liveness_and_remembered_set`. This includes some removal of code duplication wrt to the failure count tracking.
> - factored out a special `G1VerificationClosure::is_oop_safe` method that adds more checking of the `Klass`; I hesitated to put this into `Universe::is_oop` as this is much more extensive checking (and I wanted implicit error reporting :))
> - CollectedHeap::is_oop() should use `klass_raw` because otherwise it will immediately assert on oops with garbage Klass values (the new code will just return `false` as expected). 
> - some special `oopDesc::print_name_on()` which does not crash on bad oops, similar to existing related debugging helpers.
> 
> Thanks,
>   Thomas

Thomas Schatzl has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:

 - Merge branch 'master' into 8302760-improve-liveness-rset-verification
 - kbarrett review
 - Refactoring after 8302975
 - initial attempts, nothing done
   
   further cleanup
   
   more cleanup
   
   factor out changes that move the verifylivenessclosure
   
   fix compilation
   
   kbarrett review, indentation
   
   Minor removal of unnecessary code
   
   initial version
   
   more cleanup
   
   More improvements
   
   More removal of stuff
   
   More cleanup
   
   More^2 cleanup
   
   Remove some changes
   
   More cleanups
   
   fix compilation
   
   Cleanup
 - removal

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/12660/files
  - new: https://git.openjdk.org/jdk/pull/12660/files/5f35120b..1f16ee0a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12660&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12660&range=02-03

  Stats: 1739 lines in 149 files changed: 1220 ins; 247 del; 272 mod
  Patch: https://git.openjdk.org/jdk/pull/12660.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12660/head:pull/12660

PR: https://git.openjdk.org/jdk/pull/12660


More information about the hotspot-gc-dev mailing list