ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Muzafarov <mmu...@apache.org>
Subject Re: Apache Ignite 2.9.0 RELEASE [Time, Scope, Manager]
Date Mon, 07 Sep 2020 11:14:00 GMT
Alexey,

I propose to include [1] issue to the 2.9 release. Since this issue is
related to the new master key change functionality which haven't been
released yet I think it will be safe to cherry-pick commit to the
release branch.

[1] https://issues.apache.org/jira/browse/IGNITE-13390

On Tue, 1 Sep 2020 at 12:13, Nikolay Izhikov <nizhikov@apache.org> wrote:
>
> Hello, Igniters.
>
> Alexey, please, include one more Python thin client fix [1] into the 2.9 release
> It fixes kinda major issue - "Python client returns fields in wrong order since the 2
row when fields_count>10"
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12809
> [2] https://github.com/apache/ignite/commit/38025ee4167f05eaa2d6a2c5c2ab70c83a462cfc
>
> > 31 авг. 2020 г., в 19:23, Alexey Goncharuk <alexey.goncharuk@gmail.com>
написал(а):
> >
> > Alexey, thanks, got it. I am not sure we can optimize anything out of the
> > message factory with suppliers (at least I have no ideas right now), so
> > most likely the only move here is to switch back to the switch approach
> > somehow preserving the metrics part. Probably, inlining the Ignite messages
> > to the IgniteMessageFactoryImpl should do the trick. Let me explore the
> > code a bit.
> >
> > P.S. I am surprised by the impact this part makes for the performance.
> > Message creation is indeed on the hot path, but a single virtual call
> > should not make that much of a difference given the amount of other work
> > happening during the message processing.
> >
> > пн, 31 авг. 2020 г. в 18:33, Alex Plehanov <plehanov.alex@gmail.com>:
> >
> >> Alexey, sorry, I wrongly interpreted our benchmark results. Actually, we
> >> were looking for a drop using bi-sect in the range between e6a7f93 (first
> >> commit in the 2.9 branch after 2.8 branch cut) and 6592dfa5 (last commit in
> >> the 2.9 branch). And we found these two problematic commits.
> >>
> >> Perhaps only IGNITE-13060 (Tracing) is responsible for a drop between
> >> 2.8.1 and 2.9 (we have benchmarked 2.8.1 vs 2.9 with reverted IGNITE-13060
> >> now and performance looks the same)
> >>
> >> Ticket IGNITE-12568 (MessageFactory refactoring) is not related to drop
> >> between 2.8.1 and 2.9, but still has some performance problem, and we can
> >> win back IGNITE-13060 drop by this ticket.
> >>
> >> Do we need more investigation on IGNITE-13060 or we leave it as is?
> >>
> >> What should we do with IGNITE-12568 (MessageFactory refactoring)?
> >>
> >> пн, 31 авг. 2020 г. в 13:25, Alexey Goncharuk <alexey.goncharuk@gmail.com
> >>> :
> >>
> >>> Alexey,
> >>>
> >>> While investigating, I found that IGNITE-12568 has an incorrect fix
> >>> version and is actually present in ignite-2.8.1 branch [1], so it cannot
be
> >>> the source of the drop against 2.8.1.
> >>>
> >>> P.S. Looks like we need to enforce a more accurate work with fix versions
> >>> or develop some sort of tooling to verify the fix versions.
> >>>
> >>> --AG
> >>>
> >>> [1]
> >>> https://github.com/apache/ignite/commit/3e492bd23851856bbd0385c6a419892d0bba2a34
> >>>
> >>> пн, 31 авг. 2020 г. в 12:42, Alexey Goncharuk <alexey.goncharuk@gmail.com
> >>>> :
> >>>
> >>>> пт, 28 авг. 2020 г. в 11:16, Alex Plehanov <plehanov.alex@gmail.com>:
> >>>>
> >>>>> Guys,
> >>>>>
> >>>>> We have benchmarked 2.9 without IGNITE-13060 and IGNITE-12568 (reverted
> >>>>> it
> >>>>> locally) and got the same performance as on 2.8.1
> >>>>>
> >>>>> IGNITE-13060 (Tracing) - some code was added to hot paths, to trace
> >>>>> these
> >>>>> hot paths, it's clear why we have performance drop here.
> >>>>>
> >>>>> IGNITE-12568 (MessageFactory refactoring) - switch/case block was
> >>>>> refactored to an array of message suppliers. The message factory
is on
> >>>>> the
> >>>>> hot path, which explains why this commit has an impact on total
> >>>>> performance.
> >>>>> I've checked JIT assembly output, done some JMH microbenchmarks,
and
> >>>>> found
> >>>>> that old implementation of MessageFactory.create() about 30-35%
faster
> >>>>> than
> >>>>> the new one. The reason - approach with switch/case can effectively
> >>>>> inline
> >>>>> message creation code, but with an array of suppliers relatively
heavy
> >>>>> "invokeinterface" cannot be skipped. I've tried to rewrite the code
> >>>>> using
> >>>>> an abstract class for suppliers instead of an interface (to
> >>>>> replace "invokeinterface" with the "invokevirtual"), but it gives
back
> >>>>> only
> >>>>> 10% of method performance and in this case, code looks ugly (lambdas
> >>>>> can't
> >>>>> be used). Currently, I can't find any more ways to optimize the
current
> >>>>> approach (except return to the switch/case block). Andrey Gura,
as the
> >>>>> author of IGNITE-12568, maybe you have some ideas about optimization?
> >>>>>
> >>>>> Perhaps we should revert IGNITE-12568, but there are some metrics
> >>>>> already
> >>>>> created, which can't be rewritten using old message factory
> >>>>> implementation
> >>>>> (IGNITE-12756). Guys, WDYT?
> >>>>>
> >>>>
> >>>> Alexey,
> >>>>
> >>>> I see that IGNITE-12756 (metrics improvements) is already released in
> >>>> Ignite 2.8.1 while IGNITE-12568 (message factory) is only present in
Ignite
> >>>> 2.9. Let's revert both IGNITE-12568 and whichever new metrics created
for
> >>>> 2.9 that depend on the new message factory to unblock the release and
deal
> >>>> with the optimizations in 2.10?
> >>>>
> >>>
>

Mime
View raw message