RFR: 8373039: Remove Incorrect Asserts in shenandoahScanRemembered

Y. Srinivas Ramakrishna ysr at openjdk.org
Wed Dec 3 18:30:30 UTC 2025


On Wed, 3 Dec 2025 18:24:58 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp line 384:
>> 
>>> 382:   oop obj = cast_to_oop(p);
>>> 383:   assert(oopDesc::is_oop(obj), "Should be an object");
>>> 384:   assert(p <= left, "p should start at or before left end of card");
>> 
>> I think it's fine to take out this loop, but the assert on 384 now seems redundant to the assert on 363. I'm also not sure if the assert on 385 necessarily holds because `p` is no longer increased in the loop. Maybe remove this whole `#ifdef ASSERT` block, or leave in the loop and just take out the `Klass::is_valid` usage.
>
> I agree. In addition, the comment should be updated so it doesn't make the confusing reference to "the loop that follows", which just went away, etc. It's fine to leave a suitably modified comment as to why it is safe to query the size of the object at the oop being returned.

> I'm also not sure if the assert on 385 necessarily holds because p is no longer increased in the loop.

It should hold for the oop/object being returned here. It's a post-condition of the method which should have been stated in its API spec I think.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28642#discussion_r2586190041


More information about the shenandoah-dev mailing list