RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v4]

[Daniel D.Daugherty]

On Mon, 14 Sep 2020 20:25:39 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> See this comment from Coleen and the replies:
>> https://github.com/openjdk/jdk/pull/135#discussion_r487300636
>> 
>> Please let me know if that resolved this comment for you.
>
> Sorry, no. Maybe it's too late here and I shall think about it tomorrow morning instead ;-) Or maybe you can explain it
> again in the context of that change. How's the assert even relevant when moved in the else-branch?

Sorry, I confused myself switching between this review and a
preliminary review thread.

Here's the original code:

165   while (cur_obj < scan_limit) {
166     assert(!space->scanned_block_is_obj(cur_obj) ||
167            oop(cur_obj)->mark_raw().is_marked() || oop(cur_obj)->mark_raw().is_unlocked() ||
168            oop(cur_obj)->mark_raw().has_bias_pattern(),
169            "these are the only valid states during a mark sweep");
170     if (space->scanned_block_is_obj(cur_obj) && oop(cur_obj)->is_gc_marked()) {

and here's the code after it was moved and rewritten:

173     } else {
174       assert(!space->scanned_block_is_obj(cur_obj) || oop(cur_obj)->mark_raw().is_unlocked() ||
175              oop(cur_obj)->mark_raw().has_bias_pattern() || oop(cur_obj)->mark_raw().has_monitor(),
176              "these are the only valid states during a mark sweep");


Where the assert() was worked fine when the ObjectMonitor had a regular oop,
but after it was changed into a weak handle, that location became exposed to
the fact that the object reference could be GC'ed. The original code assumes
that the ObjectMonitor oop reference is stable and unchanging and that's no
longer the case with the weak handle.

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

PR: https://git.openjdk.java.net/jdk/pull/135