Request for review (XS): 7030435: Some oop_oop_iterate_m() methods iterate outside of specified memory bounds

Y. Srinivas Ramakrishna y.s.ramakrishna at oracle.com
Thu Mar 24 19:10:35 UTC 2011


John Coomes brought up the important question
of safety of this change in that we might either
scan too little now, or were scanning too much before.
I thought i'd share the discussion here, and I'd also
promised to follow up on (2) below (in terms of checking
whether those field updates did card-marks).

(1) does scanning outside of the mem-interval cause
     issues with closures that do not expect to do so:
     besides the newly modified verification closure,
     there are scavenging closures that expect that no
     old-to-new reference is scanned more than once.
     However, because in this case all of the objects
     involved are meta-data in perm gen, we are safe
     since these are not cross-generational into the
     young gen.

(2) does not scanning outside the mem-interval now
     cause not enough oops to be marked? Recall that
     the memory-range-delimited iterators are used currently
     only for card-scanning (either for cross-generational
     pointers or, as used by CMS, in the form of an incremental
     update barrier that adds to a modified reference set).
     For the cross-generational reference case, we are fine
     because as we noted above the targets are in the same
     perm-space (at least as of today).
     If these fields are updated by means of an oop_store()
     we would be fine even if these were cross-space pointers.
     It turns out that all of these field updates use either
     oop_store() (so safe even in the cross-space case), or
     they use oop_store_wirthout_check(). The latter would be
     unsafe for cross-space pointers, but is safe for incremental
     update barriers that collect modified references because
     in those cases we set AlwaysDoUpdateBarrier which does
     a card-mark. (If and when some of those using the _without_check()
     form move to a different space, they might need the _with_check()
     form for safety, should they need to update remembered set
     information to allow independent collection.)

In summary, the changes are safe today for both our stop-world
and for our concurrent CMS collector. However, it's also
true, as I noted in the CR, that we didn't really have
a correctness issue before; may be perhaps a negligible
theoretical (but not real) performance issue wrt sometimes
over-scanning by a wee bit. However, fixing the iterators
so they stay within the interval specified beings them in
line with expectations, intention and specification,
and makes for safety and hygiene of client code as it
evolves.

Many thanks again to everyone for their reviews, and i'll make
the modifications stemming from the reviews before pushing
these changes.
-- ramki

  On 3/24/2011 1:39 AM, Y. Srinivas Ramakrishna wrote:
>
> This fixes the issues found by the newly added
> assert in the card-table verification closure, apropos
> of my last code review request.
>
> Tested in the same manner as described in the code
> review request for 7029036 that i sent a little while
> ago.
>
> webrev: http://cr.openjdk.java.net/~ysr/7030435/webrev.00/
>
> Many thanks for your reviews!
> -- ramki




More information about the hotspot-gc-dev mailing list