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 <[email protected]> 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 <[email protected] 
> <mailto:[email protected]>> 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 <[email protected] 
> <mailto:[email protected]>> 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 <[email protected] 
> <mailto:[email protected]>> 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 <[email protected] 
> <mailto:[email protected]>> 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 <[email protected] 
> > > <mailto:[email protected]>> 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>
> >
> >
> 

Reply via email to