ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vladimir Ozerov <voze...@gridgain.com>
Subject Re: GridFutureAdapter: deprecate startTime(), duration() and endTime()
Date Fri, 15 Jan 2016 19:17:24 GMT
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 <ashutak@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 <yzhdanov@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 <dsetrakyan@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 <vozerov@gridgain.com
> >
> > > wrote:
> > >
> > > > +1
> > > >
> > > > BTW, corresponding ticket already exists. You can find it under
> > umbrella
> > > > ticket IGNITE-2232
> > > > 14 янв. 2016 г. 18:40 пользователь "Yakov Zhdanov" <
> > yzhdanov@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
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message