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.goncha...@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.goncha...@gmail.com > >: > >> пт, 28 авг. 2020 г. в 11:16, Alex Plehanov <plehanov.a...@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? >> >
