RFR: 8075401: Remove DiscoveredListIterator::update_discovered()

[Kim Barrett]

On Mar 18, 2015, at 11:10 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
> 
> 
> Hi Kim,
> 
> On 2015-03-18 01:20, Kim Barrett wrote:
>> Please review this change to remove unnecessary calls to
>> DiscoveredListIterator::update_discovered(), and remove the now unused
>> function. Details are in the CR.
>> 
>> I will need a sponsor for this change.
>> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8075401
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8075401/webrev.00/
> 
> As far as I can tell this is fine.

Thanks for your review.

>  t is always a bit hairy to follow the state transitions in the reference processing…

No kidding!

That's why I ended up separating this change out from the earlier
JDK-8066827 - Remove ReferenceProcessor::clean_up_discovered_references().
That one was "relatively" straight-forward except for the removal of
update_discovered, and was blocking a possible solution to a different
bug I was working on.

> One question. After your change the only use of _prev_next is in DiscoveredListIterator::remove(). Would it be possible to re-write that method to completely remove the _prev_next field? I didn't really have time to follow up how _prev, _next and _prev_next all work together, but it might be worth investigating. The one who added this comment would probably be happy if we make remove be more transparent with what it does:
> 
> // First _prev_next ref actually points into DiscoveredList (gross).

I don't think so.  We have a singly-linked list from which we want to
be able to remove the current item.  That requires keeping track of
the list pointer to the current item, which is what _prev_next is.

We could eliminate the "gross" pointer into the DiscoveredList by
using NULL as a sentinal for the head of the list, but I'm pretty sure
that would end up needing some additional tests for that special case.