[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-core-libs-dev
Subject:    Re: [PATCH] Implement a noop clear() for Collections#EMPTY_LIST
From:       Mohamed Naufal <naufal11 () gmail ! com>
Date:       2016-05-28 18:50:35
Message-ID: CAGJzMZho+A7d4St1N+y9PVBk67d_FMzQ6KJfd3BvQB3mr3iB_w () mail ! gmail ! com
[Download RAW message or body]

Hi,

Compared to avoiding an allocation, not really. In addition to avoiding the
calls, this was more for the sake of consistency with the change for
EmptyList.

Thanks,
Naufal

On 28 May 2016 at 23:57, Louis Wasserman <lowasser@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@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@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@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@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@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>
>>>> >
>>>> >
>>>>
>>>
>>
[prev in list] [next in list] [prev in thread] [next in thread]