Hi Sergey. `n` and `element` are final fields. Extracting it into local vars will not make any difference.
ср, 12 дек. 2018 г. в 02:25, Сергей Цыпанов <[email protected]>: > Hi Zheka and Tagir, > > @Override > public void forEach(final Consumer<? super E> 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 with volatile field inside (e.g. > AtomicInteger::addAndGet) which would cause a load of consumed field at > each iteration. See original post by Nitsan Wakart > https://psy-lob-saw.blogspot.com/2014/08/the-volatile-read-suprise.html > > Regards, > Sergey Tsypanov > > > 07.12.2018, 15:22, "Zheka Kozlov" <[email protected]>: > > What about forEach()? > > > > @Override > > public void forEach(final Consumer<? super E> 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 measure though). > > > > вт, 4 дек. 2018 г. в 14:40, Tagir Valeev <[email protected]>: > > > >> 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 wonder why this message received no attention. > >> > >> Here's updated webrev: > >> http://cr.openjdk.java.net/~tvaleev/webrev/8214687/r3/ > >> > >> With best regards, > >> Tagir Valeev > >> On Tue, Dec 4, 2018 at 1:10 PM Zheka Kozlov <[email protected]> > wrote: > >> > > >> > 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 <[email protected]>: > >> >> > >> >> 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 > Arrays.asList() > >> >> (with no args) to Collections.emptyList(), but it was rejected for > the > >> >> same reason: no need to introduce a risk of possible semantics > change. > >> >> > >> >> I updated webrev with equals implementation and test: > >> >> http://cr.openjdk.java.net/~tvaleev/webrev/8214687/r2/ > >> >> Comparing two CopiesList is much faster now indeed. Also we can > spare > >> >> an iterator in the common case and hoist the null-check out of the > >> >> loop. Not sure whether we can rely that JIT will always do this for > >> >> us, but if you think that it's unnecessary, I can merge the loops > >> >> back. Note that now copiesList.equals(arrayList) could be faster > than > >> >> arrayList.equals(copiesList). I don't think it's an issue. On the > >> >> other hand we could keep simpler and delegate to > super-implementation > >> >> if the other object is not a CopiesList (like it's implemented in > >> >> java.util.RegularEnumSet::equals for example). What do you think? > >> >> > >> >> With best regards, > >> >> Tagir Valeev. > >> >> On Tue, Dec 4, 2018 at 10:56 AM Stuart Marks < > [email protected]> > >> wrote: > >> >> > > >> >> > > >> >> > >> 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 >
