9:34 a.m.
On Mon, Oct 22, 2012 at 3:47 PM, Manik Surtani <manik(a)jboss.org> wrote:
On 22 Oct 2012, at 09:13, Dan Berindei <dan.berindei(a)gmail.com> wrote:
On Mon, Oct 22, 2012 at 9:25 AM, Galder Zamarreño <galder(a)redhat.com>wrote:
>
> On Oct 19, 2012, at 6:28 PM, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>
> > Galder, what JDK are you using?
>
> java version "1.6.0_35"
> Java(TM) SE Runtime Environment (build 1.6.0_35-b10-428-11M3811)
> Java HotSpot(TM) 64-Bit Server VM (build 20.10-b01-428, mixed mode)
>
> > OpenJDK 1.7 uses EmptyIterator.EMPTY_ITERATOR since 2007:
> >
> >
>
http://hg.openjdk.java.net/jdk7/jdk7/jdk/annotate/37a05a11f281/src/share/...
>
> Good that the figured out this is non-sense.
>
> > I don't have JDK 1.6 sources on hand to check, but I don't think
it's
> worth optimizing for such an old version anyway.
>
> Why not? We support JDK 1.6, so nothing stops them from running it.
>
> Until we baseline on JDK7 (or 8), I'd leave it in.
>
>
My point was that it is a really big change,
Why? The patch was trivial. Implementing it is easy, and then doing a
search and replace on the codebase again resulted in a pretty small patch.
and further it imposes a burden on anyone writing code with collections in
Infinispan.
Is it that hard for a reviewer to spot? :) Maybe even write a script to
check and warn for this.
I'm not saying it's hard, but he'll have to write a comment, the
author
will have to notice the comment and update the PR, then the reviewer will
have to notice the PR update and look at it again, and so on.
All these things take time, and I would rather not invest that time if we
can't see an improvement in performance.
I'm sure we will spend non-trivial amounts of time during reviews pointing
out places there the author should have used InfinispanCollections
instead
of Collections...
I would be willing to go for it if a benchmark showed a significant
increase in performance after the change, but you didn't mention any
benchmarks, so I guessed that you didn't see a big difference...
>
> > Cheers
> > Dan
> >
> >
> > On Fri, Oct 19, 2012 at 5:50 PM, Vladimir Blagojevic <
> vblagoje(a)redhat.com> wrote:
> > Cool! Did not know about this! Is this original idea or others are
> > already doing this?
> > On 12-10-19 8:31 AM, Galder Zamarreño wrote:
> > > Hi all,
> > >
> > > Re:
>
https://github.com/galderz/infinispan/commit/0609207d13216de81d77ff51dc20...
> > >
> > > Please avoid using these JDK methods where possible. I've created
> alternative versions in InfinispanCollections util class that provide a
> better implementation.
> > >
> > > The problem with the JDK versions is that even if these collections
> are immutable, if you wanna iterate them (i.e. for(X : emptySet) ), they'll
> create a brand new Iterator every time they're looped, and that generates
> useless garbage.
> > >
> > > The InfinispanCollections versions of emptyX will return a constant
> singleton iterator which avoids this problem, and avoids the need for
> client code to check if the collection is empty before looping.
> > >
> > > Cheers,
> > > --
> > > Galder Zamarreño
> > > galder(a)redhat.com
> > > twitter.com/galderz
> > >
> > > Project Lead, Escalante
> > > http://escalante.io
> > >
> > > Engineer, Infinispan
> > > http://infinispan.org
> > >
>
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Manik Surtani
manik(a)jboss.org
twitter.com/maniksurtani
Platform Architect, JBoss Data Grid
http://red.ht/data-grid
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev