ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yakov Zhdanov <yzhda...@gridgain.com>
Subject Re: GridFutureAdapter: deprecate startTime(), duration() and endTime()
Date Mon, 18 Jan 2016 16:18:24 GMT
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 <dsetrakyan@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 <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