Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Valentin Kulichenko
Pavel, Quite frankly, I think we used to lean into performance too much. We generally preferred it over data consistency, project modularity and code readability. Performance, of course, plays a very important rule in Ignite, but it's possible to overdo anything. There are certainly parts of the

Re: Sync vs async APIs in Ignite 3

2021-09-08 Thread Pavel Tupitsyn
To put it another way: - true sync operation completes by itself - sync-over-async operation requires another thread to complete On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn wrote: > Val, > > That's exactly what I have in mind. > Yes, we block the user thread, but then we should unblock it by

Re: Sync vs async APIs in Ignite 3

2021-09-08 Thread Pavel Tupitsyn
Val, That's exactly what I have in mind. Yes, we block the user thread, but then we should unblock it by completing the future. We can't complete the future from a Netty thread [1], so we'll need some other thread from some executor. If there are no threads available (because they are blocked by t

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Pavel Tupitsyn
Alexander, Ivan, > not very productive to assume that 100% of your code is > on the hot path That is exactly what we should be doing. When I joined Ignite community 6 years ago, this was the prevalent mindset. I'm not sure which part of Ignite can be considered "not on a hot path". Create/alter

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Valentin Kulichenko
I don't think we should ban anything. Streams API is just a tool in the toolbox - it should be used appropriately. It's up to the contributor and reviewer(s) to identify whether a particular usage might cause performance issues. -Val On Wed, Sep 8, 2021 at 8:01 AM Alexander Polovtcev wrote: > -

Re: Sync vs async APIs in Ignite 3

2021-09-08 Thread Valentin Kulichenko
Pavel, I might be missing something - could you please elaborate a little more? When I say "sync on top of async", I basically mean that (for example) 'insert(..)' is equivalent to 'insertAsync(..).join()'. In my understanding, it only blocks the user's thread. Am I wrong? Or you have a differen

Re: Ignite Website UI and UX improvements

2021-09-08 Thread Denis Magda
Folks, an update for you. We keep prototyping new pages and you can find a reference to the mockups in the ticket. Need to hear your opinion on one of the mockups. Kseniya and I worked with the design studio on a Community page, and this is what we have prepared so far: https://app.moqups.com/4WR0

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Alexander Polovtcev
-1 I think that it is not very productive to assume that 100% of your code is on the hot path, it would be impossible to write and maintain. Humans are not very good at guessing where the performance bottlenecks are, so the performance of the possible hot paths should be measured first and only the

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Ivan Pavlukhin
Does not this trivial strategy work for us? https://wiki.c2.com/?OptimizeLater 2021-09-08 13:52 GMT+03:00, Andrey Gura : > Agree that any additional object creation on a hot path could be a > problem. So hot path should not contain stream API and any other > potentially problem code (e.g. iterator

Re: Static hierarchy in jmx tree

2021-09-08 Thread Igor Akkuratov
Alexei, >It's not ok to have the property IGNITE_MBEAN_STATIC_HIERARCHY - actually >it's not always static, because node id changes on each restart. I am fine to remove this property. > Instead I would prefer to make IGNITE_MBEAN_APPEND_CLASS_LOADER_ID=false by > default. Already done in my PR

Re: [VOTE] Allow or prohibit usages of the Guava library methods

2021-09-08 Thread Вячеслав Коптилин
-1 I am leaning toward -1 because of vulnerability issues (that is a possible case in general). Thanks, S. ср, 8 сент. 2021 г. в 12:13, Andrey Mashenkov : > -1 > Supporting few copy-pasted methods is much easier than support dependencies > compatibility. > > On Tue, Sep 7, 2021 at 7:42 PM Zhenya

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Andrey Gura
Agree that any additional object creation on a hot path could be a problem. So hot path should not contain stream API and any other potentially problem code (e.g. iterator instead of for by index). On Wed, Sep 8, 2021 at 1:45 PM Pavel Tupitsyn wrote: > > Ok, maybe a total ban is overkill, but rig

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Pavel Tupitsyn
Ok, maybe a total ban is overkill, but right now streams are used even on some hot paths like getAllAsync [1]. Another issue is that Collectors.toList() and other variants don't accept capacity, and we end up with unnecessary reallocations of underlying arrays. [1] https://github.com/apache/ignit

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Konstantin Orlov
Hi! Agree with Ivan that it’s an overkill. Code readability and maintainability should have the same priority as the performance (with some exceptions of course). BTW the result of your benchmark looks quite strange. The performance penalty on my laptop (Core i7 9750H, 6 cores up to 4.50 GHz) is

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Ivan Pavlukhin
-1 for "banning" streams. Fully agree with Ivan B. 2021-09-08 12:23 GMT+03:00, Ivan Bessonov : > Hello Igniters, > > I object, banning streams is an overkill. I would argue that most of the > code > is not on hot paths and that allocations in TLAB don't create much pressure > on GC. > > Streams m

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Ivan Bessonov
Hello Igniters, I object, banning streams is an overkill. I would argue that most of the code is not on hot paths and that allocations in TLAB don't create much pressure on GC. Streams must be used cautiously, developers should know whether they write hot methods or not. And if methods are not ho

Re: [VOTE] Allow or prohibit usages of the Guava library methods

2021-09-08 Thread Andrey Mashenkov
-1 Supporting few copy-pasted methods is much easier than support dependencies compatibility. On Tue, Sep 7, 2021 at 7:42 PM Zhenya Stanilovsky wrote: > > Aleksandr, thanks for this activity. > -1 from my side, all my decisions are in linked discussion. > > >Dear Igniters, > > > >In this thread

Re: Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Zhenya Stanilovsky
Pavel, thanks ! And what about stream usage not in a hot path ? I.e. create\drop table operation, may be such a cases will leave for committer\contributor  consideration ?   >Igniters, > >Java streams are known to be slower and cause more GC pressure than an >equivalent loop. >Below is a simple f

Ban Java Streams usage in Ignite 3 code

2021-09-08 Thread Pavel Tupitsyn
Igniters, Java streams are known to be slower and cause more GC pressure than an equivalent loop. Below is a simple filter/map/reduce scenario (code [1]): * Benchmark Mode Cnt Score Error Units * StreamVsLoopBenchmark.loopSum

Re: Sync vs async APIs in Ignite 3

2021-09-08 Thread Pavel Tupitsyn
Val, Agree with your points. > async API should be primary It should be noted that all our APIs are inherently async, because thin client is implemented asynchronously. > with the sync version build on top We should document somehow that sync APIs are based on async ones, because this may be

Re: Sync vs async APIs in Ignite 3

2021-09-08 Thread Courtney Robinson
Hi Val, I'd highly support an async first API based on CompletionStage or its subtypes like CompletableFuture. In Ignite 2 we've written a wrapper library around IgniteFuture to provide CompletionStage instead be