Hello!
I managed to push the change by myself:
http://hg.openjdk.java.net/jdk/jdk/rev/cfceb4df2499
This is my first push in a committer role. Hopefully I did not mess anything up.
With best regards,
Tagir Valeev.
On Thu, Dec 13, 2018 at 12:33 PM Tagir Valeev wrote:
>
> Hello, Zheka!
>
> I'm not
Hello, Zheka!
I'm not sure whether it's possible to commit a patch which is
partially contributed by another person. Probably you should submit it
separately? Also for complete patch a testcase would be necessary.
With best regards,
Tagir Valeev.
On Thu, Dec 13, 2018 at 11:48 AM Zheka Kozlov
OK, this is a fixed version:
@Override
public void forEach(Consumer action) {
Objects.requireNonNull(action);
final int n = this.n;
final E e = this.element;
for (int i = 0; i < n; i++) {
action.accept(e);
}
}
Tagir, can you add this to your patch? I signed the OCA.
I used to believe that, but apparently I was wrong.
https://openjdk.markmail.org/thread/rfqfultw35i2az45
On Tue, Dec 11, 2018 at 8:14 PM Zheka Kozlov wrote:
> Would be better to add @Stable to the fields instead? (`n` and `element`
> are final, so @Stable is OK here)
>
> ср, 12 дек. 2018 г. в
Would be better to add @Stable to the fields instead? (`n` and `element`
are final, so @Stable is OK here)
ср, 12 дек. 2018 г. в 11:02, Martin Buchholz :
> In performance critical code, we don't trust hotspot to not reload final
>> fields. Other forEach methods do this, e.g.
>
>
> final
>
> In performance critical code, we don't trust hotspot to not reload final
> fields. Other forEach methods do this, e.g.
final Object[] es = queue;
for (int i = 0, n = size; i < n; i++)
action.accept((E) es[i]);
Hi Sergey.
`n` and `element` are final fields. Extracting it into local vars will not
make any difference.
ср, 12 дек. 2018 г. в 02:25, Сергей Цыпанов :
> Hi Zheka and Tagir,
>
> @Override
> public void forEach(final Consumer action) {
> Objects.requireNonNull(action);
> for (int i = 0; i <
Hi Zheka and Tagir,
@Override
public void forEach(final Consumer action) {
Objects.requireNonNull(action);
for (int i = 0; i < this.n; i++) {
action.accept(this.element);
}
}
I think here we should extract this.element and this.n into a local vars, as
Consumer may have interaction
What about forEach()?
@Override
public void forEach(final Consumer action) {
Objects.requireNonNull(action);
for (int i = 0; i < this.n; i++) {
action.accept(this.element);
}
}
I think this version has better chances to be faster than the default
implementation (did not
Hello!
> In CopiesList.equals() please use eq() instead of Objects.equal() (see a
> comment at the line 5345).
Ok
> I think you should use iterator() instead of listIterator(). See the
> explanation here:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-April/052472.html
Ok. I
I think you should use iterator() instead of listIterator(). See the
explanation here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-April/052472.html
вт, 4 дек. 2018 г. в 12:23, Tagir Valeev :
> Hello!
>
> Thank you for your comments!
>
> Yes, deserialization will be broken if we
Hi Tagir!
I think what you have in the last webrev looks good!
In CopiesList.equals() please use eq() instead of Objects.equal() (see a
comment at the line 5345).
With kind regards,
Ivan
On 12/3/18 9:22 PM, Tagir Valeev wrote:
Hello!
Thank you for your comments!
Yes, deserialization will
Hello!
Thank you for your comments!
Yes, deserialization will be broken if we assume that size is never 0.
Also we'll introduce referential identity Collections.nCopies(0, x) ==
Collections.nCopies(0, y) which might introduce slight semantics
change in existing programs. Once I suggested to wire
I believe it makes sense to override CopiesList.equals()
Also: contains(), iterator(), listIterator()
equals(): sure
contains() is already overridden. Not sure there's much benefit to overriding
the iterators.
s'marks
time...
Thanks,
-Andrew
-Original Message-
From: core-libs-dev On Behalf Of Zheka
Kozlov
Sent: Monday, December 3, 2018 12:43 AM
To: Ivan Gerasimov
Cc: core-libs-dev
Subject: Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()
Would it make sense to use CopiesList
To: Ivan Gerasimov
Cc: core-libs-dev
Subject: Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()
> Would it make sense to use CopiesList only for n > 0
This can break serialization. When CopiesList(0, e) is serialized and
deserialized back, it will be in an invalid state
> Would it make sense to use CopiesList only for n > 0
This can break serialization. When CopiesList(0, e) is serialized and
deserialized back, it will be in an invalid state.
> I believe it makes sense to override CopiesList.equals()
Also: contains(), iterator(), listIterator()
пн, 3 дек.
Also, I believe it makes sense to override CopiesList.equals() to at
least optimize the case when the other list is a CopiesList too.
With kind regards,
Ivan
On 12/2/18 11:23 PM, Ivan Gerasimov wrote:
Thank you Tagir!
I think your solution is quite clever, and the fix looks good.
While we
Thank you Tagir!
I think your solution is quite clever, and the fix looks good.
While we are here: Would it make sense to use CopiesList only for n >
0, and make nCopies() and nCopies().subList() return
Collection.emptyList() otherwise?
This would allow to remove the check for n == 0 in a
19 matches
Mail list logo
