ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pavel Tupitsyn <ptupit...@gridgain.com>
Subject Re: .Net: separate methods for async operations.
Date Fri, 09 Oct 2015 20:46:49 GMT
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 <dsetrakyan@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 <ptupitsyn@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 <vozerov@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message