[PATCH] Implement a noop clear() for Collections#EMPTY_LIST

[Paul Sandoz]

Hi,

Late to this thread…

I think for consistency it’s ok to apply in all three cases. I will push today.

—

Another trivial fix in Collections would be to go through the nested classes and mark appropriate classes as final.

Paul.

> On 28 May 2016, at 11:27, Louis Wasserman <lowasser at google.com> wrote:
> 
> Do you actually expect that to represent a significant performance difference?  All those calls sound like things easy to JIT.
> 
> 
> On Sat, May 28, 2016, 11:26 AM Mohamed Naufal <naufal11 at gmail.com <mailto:naufal11 at gmail.com>> wrote:
> Hi,
> 
> You're right of course, I should have been clearer, no allocation happens for clear() on EmptyMap or EmptySet, but a lot of unnecessary calls are made.
> 
> The call stack for clear() on EmptySet looks something like this:
> AbstractCollection#clear() ->EmptySet#iterator() -> Collections#emptyIterator() -> returns the singleton EmptyIterator#EMPTY_ITERATOR on which hasNext() is called.
> 
> And for EmptyMap:
> AbstractMap#clear() -> EmptyMap#entrySet() -> Collections#emptySet(), from which point onwards, it's similar to that of EmptySet.
> 
> This could be avoided with the direct override.
> 
> Thanks,
> Naufal
> 
> On 28 May 2016 at 22:32, Louis Wasserman <lowasser at google.com <mailto:lowasser at google.com>> wrote:
> Is it?  IIRC EmptyMap and EmptySet will use a singleton unmodifiable empty Iterator, so they won't incur any allocation, and the clear() will finish immediately with no work anyway.
> 
> 
> On Sat, May 28, 2016, 2:05 AM Mohamed Naufal <naufal11 at gmail.com <mailto:naufal11 at gmail.com>> wrote:
> Hi,
> 
> I see that this is applicable to EmptyMap and EmptySet as well, here's an
> updated patch with clear() overridden for all 3 classes.
> 
> Thanks,
> Naufal
> 
> On 23 May 2016 at 16:13, Paul Sandoz <paul.sandoz at oracle.com <mailto:paul.sandoz at oracle.com>> wrote:
> 
> > Hi Naufal,
> >
> > Thanks for looking at this.
> >
> > For us to accept your patch (no matter how small) you need to become a
> > contributor, which requires that you agree to the Oracle Contributor
> > Agreement (OCA), see:
> >
> >   http://openjdk.java.net/contribute/ <http://openjdk.java.net/contribute/>
> >
> > Thanks,
> > Paul.
> >
> > > On 22 May 2016, at 12:10, Mohamed Naufal <naufal11 at gmail.com <mailto:naufal11 at gmail.com>> wrote:
> > >
> > > Hi,
> > >
> > > A call to clear() on Collections#EMPTY_LIST is currently redirected to
> > > AbstractList#clear(), which performs a bunch of checks and creates a
> > > ListItr object, all of which is unnecessary for an EmptyList.
> > >
> > > PFA a patch that implements a noop clear() for EmptyList.
> > >
> > > Thanks,
> > > Naufal
> > > <EmptyList_clear.diff>
> >
> >
>