RFR: 8075401: Remove DiscoveredListIterator::update_discovered()

[Bengt Rutisson]

Hi Kim,

On 2015-03-18 18:51, Kim Barrett wrote:
> 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.

Ok. That's fine. Let's leave it as it is.

Reviewed.

Bengt

>