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
> > >
> >
>
