On Thu, 7 Oct 2021 15:28:31 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> Also fixes: 8273956: Add checking for rank values >> >> This change does 3 things. I could separate them but this has all been >> tested together and most of the change is mechanical. The first is a simple >> rename of nonleaf => safepoint. The second change is to add the enum class >> Rank which only allows subtraction from a base rank, and some additional >> type checking so that rank arithmetic doesn't overlap with a different rank. >> Ie if you say nosafepoint-n, that doesn't overlap with oopstorage. The >> third change is to remove the _safepoint_check_always/never flag. That is >> now intrinsic in the ranking, as all nosafepoint ranks are lower than all >> safepoint ranks. >> >> Future changes could add rank names in the middle of nosafepoint and >> safepoint, like handshake. As of now, the rank subtractions are not >> unmanageable. There are actually not many nested Mutex. >> >> This is the last of the planned subtasks for Mutex ranking cleanup. If you >> have other ideas or suggestions, please let me know. >> >> Tested with tier1-8 at one point in time and just retested with tier1-6. > > Coleen Phillimore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix overlap error message printing and add a test. Hi Coleen, A couple of comments on tests, but otherwise okay. Thanks, David test/hotspot/gtest/runtime/test_mutex.cpp line 292: > 290: Monitor* monitor_rank_broken = new Monitor(Mutex::oopstorage-4, > "monitor_rank_broken"); > 291: monitor_rank_broken->lock_without_safepoint_check(); > 292: monitor_rank_broken->unlock(); This is dead code right - the assertion failure will stop us getting here. test/hotspot/gtest/runtime/test_mutex.cpp line 302: > 300: Monitor* monitor_rank_broken = new Monitor(Mutex::safepoint-40, > "monitor_rank_broken"); > 301: monitor_rank_broken->lock_without_safepoint_check(); > 302: monitor_rank_broken->unlock(); Ditto - dead code test/hotspot/gtest/runtime/test_mutex.cpp line 310: > 308: ThreadInVMfromNative invm(THREAD); > 309: > 310: Monitor* monitor_rank_broken = new Monitor(Mutex::safepoint-1, > "monitor_rank_broken"); This rank is not actually broken is it - otherwise we won't get to the next line. test/hotspot/gtest/runtime/test_mutex.cpp line 313: > 311: Monitor* monitor_rank_also_broken = new > Monitor(monitor_rank_broken->rank()-39, "monitor_rank_also_broken"); > 312: monitor_rank_also_broken->lock_without_safepoint_check(); > 313: monitor_rank_also_broken->unlock(); Dead code ------------- Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5845
