I was one of few people who reviewed this design originally and didn't see much problem with it (it did seem contrived a bit). I now actually agree with "below" sentiment that in retrospect it was a very bad decision and APIs is frankly unusable IMHO.
The minute you pass instance of cache elsewhere on the call chain - this API cannot be used sanely. Java vs. .NET, API parity vs. feature parity are all good discussions to have - but at least Java side needs to be fixed. -- Nikita Ivanov On Sat, Oct 10, 2015 at 2:39 AM, Sergi Vladykin <sergi.vlady...@gmail.com> wrote: > I think IgniteAsyncSupport is a big piece of crap even in java, sync/async > API separation definitely must be done in another way. > > Sergi > > 2015-10-10 0:40 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > Pavel, can you explain how .NET async semantics are different from Java? > > > > On Fri, Oct 9, 2015 at 1:46 PM, Pavel Tupitsyn <ptupit...@gridgain.com> > > wrote: > > > > > Hi Dmitry, > > > > > > > First of all, from my experience, the async APIs are used a lot less > > > than sync > > > ones > > > > > > This may be true, especially if the API is clunky. > > > But .NET has async/await functionality which makes async code a lot > > cleaner > > > and easier. > > > Good async/await support is very important, because it does not block > > > current thread, which in turn is important for high load applications. > > > All modern .NET APIs are async. > > > > > > > > > > Secondly, the scope of this change would be huge. > > > > > > I don't think so. There are around 40 methods with async support in > > current > > > Ignite.NET. > > > Adding their async counterparts will take a couple of hours at most. > > > And it will simplify interop code somewhat because GetFuture goes away. > > > > > > > > > > And lastly, I am against having .NET APIs different from Java APIs. > > > > > > Functionally they will be the same. But we should not try to bring Java > > > semantics to .NET. > > > Async methods in .NET are "T DoSomething()" + "Task<T> > > DoSomethingAsync()" > > > and we should follow this pattern so our API looks familiar to .NET > > > community. > > > > > > > > > > // Line #2 > > > > cache.Future().Get(); > > > > > > It is cache.GetFuture<X>(). Pay attention to "X". This is very > important: > > > user has to specify correct return type according to return type of > > > operation on "Line 1". > > > Very annoying and error prone. There is even a style-checker warning > > about > > > such things. > > > > > > > > > > 2 lines of code instead of one is not a big deal in my view. > > > > > > It is 2 times too much, sometimes even more. Imagine a situation where > > you > > > need to perform multiple async operations: > > > > > > cache.Get(1); > > > var res = await cache.GetFuture<int>().ToTask(); > > > > > > compute.Call(new MyCallable(res)) > > > var res2 = await compute.GetFuture<string>().ToTask() > > > > > > > > > And with proper async API it can even be a one-liner. > > > var res2 = await compute.CallAsync(new MyCallable(await > > > cache.GetAsync(1))); > > > > > > > > > API is the first thing a programmer sees in the new product. Let's do > it > > > right. > > > > > > Thanks, > > > > > > On Fri, Oct 9, 2015 at 7:17 PM, Dmitriy Setrakyan < > dsetrak...@apache.org > > > > > > wrote: > > > > > > > I don't think I like the proposed change. > > > > > > > > First of all, from my experience, the async APIs are used a lot less > > than > > > > sync ones, so having 2 lines of code for async calls is not a big > deal > > in > > > > my view. > > > > > > > > Secondly, the scope of this change would be huge. We would have to > > double > > > > our Compute, Cache, Services, and many other APIs. Seems to me like a > > > huge > > > > amount of effort for a very questionable benefit, if any at all. One > > can > > > > argue, for example, that so many duplicate sync/async methods on all > > the > > > > APIs is more confusing, not less. > > > > > > > > And lastly, I am against having .NET APIs different from Java APIs. > We > > > > should have API parity as much as possible between all the platforms, > > and > > > > especially the .NET one, where we provide so many features. > > > > > > > > On Fri, Oct 9, 2015 at 7:05 AM, Pavel Tupitsyn < > ptupit...@gridgain.com > > > > > > > wrote: > > > > > > > > > As a .Net dev, I support this change very much. > > > > > > > > > > Current design with 2 method calls is not easy to use, is error > > prone, > > > > and > > > > > is not familiar to .Net crowd: > > > > > > > > > > var cache = ignite.GetCache<int, int>().WithAsync(); > > > > > var value = cache.Get(1); // Is it sync or async? Can't tell from > > > code. > > > > > > > > In async mode this always returns 0. > > > > > > > > > > > > > Yes, you are right. But then again, async documentation clearly says > > that > > > > you should get a future to get the actual asynchronous result. > > > > > > > > The proper code here is: > > > > ----------- > > > > var cache = ignite.GetCache<int, int>().WithAsync(); > > > > > > > > // Line #1 > > > > cache.Get(1); > > > > > > > > // Line #2 > > > > cache.Future().Get(); > > > > ----------- > > > > > > > > 2 lines of code instead of one is not a big deal in my view. > > > > > > > > > > > > > var future = cache.GetFuture<int>(); // User has to specify right > > > > generic > > > > > argument here. Not convenient, error prone, violates design > > guidelines > > > > > var actualValue = await future.ToTask(); > > > > > > > > > > > > > > > As opposed to: > > > > > var value = await cache.GetAsync(1).ToTask(); > > > > > > > > > > > > > > > > > > > > > > Which is one line, obviously async, with proper generic inference. > > > > > > > > > > > > > > > > > > > > On Fri, Oct 9, 2015 at 4:47 PM, Vladimir Ozerov < > > voze...@gridgain.com> > > > > > wrote: > > > > > > > > > > > Igniters, > > > > > > > > > > > > Some time ago we decided to merge sync and async methods. E.g. > > > instead > > > > of > > > > > > ... > > > > > > > > > > > > interface Cache<K, V> { > > > > > > V get(K key); > > > > > > Future<V> getAsync(K key); > > > > > > } > > > > > > > > > > > > ... we now have: > > > > > > > > > > > > interface Cache<K, V> extends AsyncSupport { > > > > > > V get(K key); > > > > > > Cache withAsync(); > > > > > > > > > > > > Future lastFuture(); // From AsyncSupport, returns future for > > the > > > > > last > > > > > > operation. > > > > > > } > > > > > > > > > > > > This approach is questionable. Less methods is good, and all > > methods > > > go > > > > > > through JCache API. But async mode became more complex and less > > > usable. > > > > > > This is especially obvious in Java 8 with its lambdas and > > > > > > CompletableFuture. > > > > > > > > > > > > In .Net we blindly applied this approach as well. But in this > world > > > > > > AsyncSupport gives even less advantages than in Java: > > > > > > 1) There is no JCache spec here; > > > > > > 2) Core .Net API very often use paired sync and async operations > in > > > the > > > > > > same class near each other - DoMethod(), DoMethodAsync() - and > this > > > is > > > > > what > > > > > > users normally expect from async-enabled classes. > > > > > > 3) [AsyncSupported] attribute is not highlighted in Visual > Studio. > > > The > > > > > only > > > > > > way to understand that method supports async mode is to install > > > > ReSharper > > > > > > or to look into source code. > > > > > > 4) .Net has native continuations support with async/await > keywords. > > > Our > > > > > API > > > > > > doesn't support it well. > > > > > > > > > > > > Having said that I want to return paired "async" operations to > .Net > > > > API: > > > > > > interface ICache<K, V> { > > > > > > V Get(K key); > > > > > > IFuture<V> GetAsync(K key); > > > > > > } > > > > > > > > > > > > It will add 25 new methods to ICache interface and remove 2. But > > API > > > > will > > > > > > become much more friendly and natural for .Net users. > > > > > > > > > > > > Any thoughts/objections? > > > > > > > > > > > > Vladimir. > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > -- > > > > > Pavel Tupitsyn > > > > > GridGain Systems, Inc. > > > > > www.gridgain.com > > > > > > > > > > > > > > > > > > > > > -- > > > -- > > > Pavel Tupitsyn > > > GridGain Systems, Inc. > > > www.gridgain.com > > > > > >