[PATCH] Implement a noop clear() for Collections#EMPTY_LIST
Louis Wasserman
lowasser at google.com
Sat May 28 18:27:50 UTC 2016
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> 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> 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> 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> 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/
>>> >
>>> > Thanks,
>>> > Paul.
>>> >
>>> > > On 22 May 2016, at 12:10, Mohamed Naufal <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>
>>> >
>>> >
>>>
>>
>
More information about the core-libs-dev
mailing list