Here is the PR https://github.com/apache/ignite/pull/409 - not ready to merge, but cache full api suite passes, so I can run distributed benchmarks tomorrow.
Thanks! -- Yakov Zhdanov, Director R&D *GridGain Systems* www.gridgain.com 2016-01-16 19:04 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > Yakov, > > Any chance you can rerun the benchmarks to confirm your findings? > > Would also be nice if someone else could run them as well. Is there a > branch that we can check out? > > D. > > On Fri, Jan 15, 2016 at 12:17 PM, Vladimir Ozerov <voze...@gridgain.com> > wrote: > > > I also hardly believe that such change can give us such big immediate > > performance benefit. Profiling shows that in our standard benchmarks > > futures doesn't generate so much garbage to get ~5% from field removal. I > > would rather re-check if benchmark is valid. > > > > On the other hand, lots of our utility classes like GridFunc, GridUtils, > > futures, etc., are overly complex and inefficient. As they are used > almost > > everywhere, if we see something what can be easily optimized, we should > do > > that right away, even if the benefit cannot be observed in benchmarks. > This > > will give us confidence that our fundamental abstractions are good > enough, > > so we can focus on business logic and algorithms, rather than on > supporting > > code. > > > > So I think that regardless of the benchmark numbers, we should invest > > efforts in changes like this. > > > > On Fri, Jan 15, 2016 at 6:15 PM, Artem Shutak <ashu...@gridgain.com> > > wrote: > > > > > If it really can give as 5% of performance we have to do it. > > > > > > If anyone uses this methods we can support it by user request (if user > > > explicitly asked about) and it will work slower in that case. For > > example, > > > we can add *IgniteCache.withAsync(boolean useFutureWithTimer)* that > will > > > give to user an async cache with futures that support startTime(), > > > duration() and endTime methods as well. > > > > > > -- Artem -- > > > > > > On Fri, Jan 15, 2016 at 3:10 PM, Yakov Zhdanov <yzhda...@apache.org> > > > wrote: > > > > > > > All of optimizations applied to futures so far were extremely > > effective. > > > > Ignite can create different number of futures per operation depending > > on > > > > context. Multiple this by ops/sec.. This is probably one of the most > > > > intensively instantiated (and therefore GCed) object. Internal > futures > > is > > > > very sensitive part of the system and should be 100% effective. > > > > > > > > As far as public API, anyone > > > > uses org.apache.ignite.lang.IgniteFuture#startTime > > > > or org.apache.ignite.lang.IgniteFuture#duration? > > > > > > > > --Yakov > > > > > > > > 2016-01-15 1:20 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > > > > > > > Really hard to believe that removing 16 bytes in futures gives 5% > of > > > > > performance. Yakov, are you certain about this? > > > > > > > > > > If this turns out to be true, let’s see if we can slim down the > > futures > > > > > used internally, without breaking the public API. > > > > > > > > > > D. > > > > > > > > > > On Thu, Jan 14, 2016 at 8:36 AM, Vladimir Ozerov < > > voze...@gridgain.com > > > > > > > > > wrote: > > > > > > > > > > > +1 > > > > > > > > > > > > BTW, corresponding ticket already exists. You can find it under > > > > umbrella > > > > > > ticket IGNITE-2232 > > > > > > 14 янв. 2016 г. 18:40 пользователь "Yakov Zhdanov" < > > > > yzhda...@apache.org> > > > > > > написал: > > > > > > > > > > > > > Guys, > > > > > > > > > > > > > > We have startTime(), duration() and endTime() methods which > have > > > > > several > > > > > > > usages each along internals of the project, but to support > these > > > > > methods > > > > > > we > > > > > > > have 2 long fields in GridFutureAdapter which gives us 16 > bytes. > > > > > > > > > > > > > > Other fields - res (reference 8 bytes at max), ignoreInterrupts > > > > > (boolean > > > > > > 1 > > > > > > > byte) and resFlag (byte 1 byte) = 10 bytes > > > > > > > > > > > > > > I did quick tests and I see that removing these fields (i.e. > > making > > > > > each > > > > > > > future 16 bytes thinner) can give us 5-6% in performance > results. > > > > > > > > > > > > > > I want to deprecate startTime(), duration() and endTime() and > > > > > therefore > > > > > > > deprecate corresponding methods in IgniteFuture. > > > > > > > > > > > > > > Thoughts? > > > > > > > > > > > > > > --Yakov > > > > > > > > > > > > > > > > > > > > > > > > > > > >
