ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Setrakyan <dsetrak...@apache.org>
Subject Re: GridFutureAdapter: deprecate startTime(), duration() and endTime()
Date Sat, 16 Jan 2016 16:04:45 GMT
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 <vozerov@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 <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