ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yakov Zhdanov <yzhda...@apache.org>
Subject Re: Review request
Date Wed, 03 Feb 2016 17:06:07 GMT
I think we should not allocate list at all! We can just add listener to
added futures!:) and the semantics will be preserved!

However this will not work if we want to iterate over the added ones.
Number of unfinished futures will be still available through listener calls
count. Let me review.

If this doesn't work, we can maintain an array on our own.

And the last point.. This GC impact should be measured. What happen if you
add a reference to future and reference to collection?

Yakov
On Feb 3, 2016 7:44 PM, "Dmitriy Setrakyan" <dsetrakyan@apache.org> wrote:

> On Wed, Feb 3, 2016 at 7:02 AM, Vladimir Ozerov <vozerov@gridgain.com>
> wrote:
>
> > As per cache - I hardly understand affected logic, so my review wouldn't
> > help much here.
> >
> > As per the rest changes - looks good for me. I also see garbage from NIO
> > and "force keys" as huge memory hotspots. The only problem is
> > GridCompoundFuture:
> >
> > if (futs == null)
> >     futs = new ArrayList<>();
> >
> > futs.add(fut);
> >
> >
> > Things like this are proven to be anti-pattern in terms of memory
> > allocations, because on the last line you effectively allocate
> Object[10],
> > while usually you will have much less child futures. We could delay
> > allocation of array if we store the very first child future as direct
> > reference. And only second added future should lead to ArrayList
> > allocation. This should positively affect lots operations with "compound
> > semantics" and single cache key involved (e.g. single puts/gets).
> >
>
> Vova,
> I couldn’t agree more. Can we try it out and see what kind of GC or
> performance improvement we get?
>
>
> > On Mon, Feb 1, 2016 at 9:08 PM, Yakov Zhdanov <yzhdanov@apache.org>
> wrote:
> >
> > > No visible changes to throughput and latency on our common
> configuration,
> > > but allocation pressure reduced up to 20% in put-get benchmarks.
> > >
> > > --Yakov
> > >
> > > 2016-02-01 20:02 GMT+03:00 Dmitriy Setrakyan <dsetrakyan@apache.org>:
> > >
> > > > Any preliminary performance numbers?
> > > >
> > > > On Mon, Feb 1, 2016 at 8:52 AM, Yakov Zhdanov <yzhdanov@apache.org>
> > > wrote:
> > > >
> > > > > Vladimir Ozerov and Alex Goncharuk, can you please take a look at
> PR
> > > and
> > > > > provide comments? Other reviewers are welcome, too! =)
> > > > >
> > > > > https://github.com/apache/ignite/pull/422
> > > > >
> > > > > I did some changes to decrease allocation pressure and fixed force
> > keys
> > > > > request not to be sent if rebalancing has already been successfully
> > > > > completed on operation's topology version.
> > > > >
> > > > > --Yakov
> > > > >
> > > >
> > >
> >
>

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