BREAKING CHANGE: dart:io APIs changing.

[Mads Ager]

Hi all,

we promised that the dart:io APIs would change over time. They
just did: https://chromiumcodereview.appspot.com/9500002/

Short summary of changes:

One-shot methods now take their callback as an argument:

Instead of

var f = new File('myfile.txt');
f.exists();
f.existsHandler = (result) {
// do stuff.
};

write

var f = new File('myfile.txt');
f.exists((result) {
// do stuff.
});

For active objects such as streams that emit events, the event
handlers have changed names from eventHandler to onEvent:

Instead of

stdin.dataHandler = () {
// do stuff.
};

write

stdin.onData = () {
// do stuff.
};

Cheers, -- Mads

[Chris Buckett]

+1 to that - much better.

Thanks

Chris.

[Christian Grobmeier]

+1 agreed

--
http://www.grobmeier.de
https://www.timeandbill.de

[Ladislav Thon]

Agree that it's much better, but I think it still isn't enough. I have some proposal for these IO interfaces in the work, I hope to publish them this weekend (not implementation, only interfaces). Basically, is looks like this:

1. All objects are immutable. This is an axiom. You really can't pass a File or Directory object to some other code now, which sucks.

2. Separate sync and async interfaces completely, i.e. instead of File having both sync and async operations, have an async File and sync FileSync.

3. In addition to File and Directory, add a Path for working with paths. File and Directory are created from the Path.

4. All async operations take a callback. Those that can repeat and therefore are subject to flow control are split into two cases: the basic one, in which you have no control of the operation, it basically fires until it is done, and the advanced one, in which you get a "control" object in the callback, which can be used to start/pause/resume/stop. (I haven't fully thought this through, but this is my current thinking.)

Hope to start a discussion over the code soon.

LT

[A Matías Quezada]

+1 I love this change!

[Matt B]

Love it. This is what I was rather expecting in most cases so when I
saw the existsHandler syntax in the past I was a little confused by
why it was implemented in that way. I much more prefer this. Changing
to onEvent as opposed to eventHandler is going to take me a little
adjustment but it's all good.

Though it would be nice if there was a... prettier name for the *Sync
methods. existsSync() is a little obtuse to me. Regretfully though I
have no suggestions for an alternative.

Thanks,
Matt

[Mads Ager]

On Thu, Mar 1, 2012 at 2:18 PM, Matt B <butler....@gmail.com> wrote:
> Love it. This is what I was rather expecting in most cases so when I
> saw the existsHandler syntax in the past I was a little confused by
> why it was implemented in that way. I much more prefer this. Changing
> to onEvent as opposed to eventHandler is going to take me a little
> adjustment but it's all good.
>
> Though it would be nice if there was a... prettier name for the *Sync
> methods. existsSync() is a little obtuse to me. Regretfully though I
> have no suggestions for an alternative.

The *Sync names are actually intentionally obtuse (and are used by
node.js as well). :-)

Basically, the Sync part of the name is a warning that you probably do
not want to use them. Whenever you call a method that is called
something ending in Sync you know that you are blocking the main
thread which is a Bad Thing for scalability. It is fine for simple
scripts, it should be avoided for servers that should scale to deal
with many requests.

Cheers, -- Mads

[Rémi Forax]

On 03/01/2012 02:30 PM, Mads Ager wrote:
> On Thu, Mar 1, 2012 at 2:18 PM, Matt B<butler....@gmail.com> wrote:
>> Love it. This is what I was rather expecting in most cases so when I
>> saw the existsHandler syntax in the past I was a little confused by
>> why it was implemented in that way. I much more prefer this. Changing
>> to onEvent as opposed to eventHandler is going to take me a little
>> adjustment but it's all good.
>>
>> Though it would be nice if there was a... prettier name for the *Sync
>> methods. existsSync() is a little obtuse to me. Regretfully though I
>> have no suggestions for an alternative.
> The *Sync names are actually intentionally obtuse (and are used by
> node.js as well). :-)
>
> Basically, the Sync part of the name is a warning that you probably do
> not want to use them. Whenever you call a method that is called
> something ending in Sync you know that you are blocking the main
> thread which is a Bad Thing for scalability. It is fine for simple
> scripts, it should be avoided for servers that should scale to deal
> with many requests.
>
> Cheers, -- Mads

why fooSync doesn't register a callback, block the current lightweight
isolate
(not the thread) and let the current thread serve another lightweight
isolate.

I think Dart IO should be designed to not let users to be able to block
a thread
(and not be able to even control threads).

A server should first create a pool of threads with as many thread as
core (+/- 1).
Each time a new connection is accepted, the server should create a
lightweight isolate
and push it into a waiting queue.
Each thread from the pool, should dequeue the lightweight isolate and
run it,
if this isolate do a blocking operation, it should register the
operation asynchronously
and when the event arrive, the lightweight isolate should be enqueue again.
so when a thread should try to run it, the lightweight isolate will be
resumed.

Users should only control isolates not threads.

R�mi

[Mads Ager]

I'm not sure I understand. It is already the case that users do not
have control over threads. Asynchronous operations lets you do a lot
with only one thread. When that one thread is not enough you can turn
to isolates.

Cheers, -- Mads

[Rémi Forax]

On 03/01/2012 02:59 PM, Mads Ager wrote:

As a user I can create lightweight isolate or heavyweight isolate.
I may be wrong but for me an heavyweight isolate is a thread.

What I'm trying to say is that the user should just do IOs
and that if the IO call is done in a server, it should be an async call
that stop the isolate and resume it when the IO is done and
that do classical blocking IO in a script (if there is only one isolate).

R�mi

[Sean Eagan]

This is great!  How about optional error callbacks as well for the single shot methods ?  That is the only thing still preventing wrapping these APIs with higher level ones which return Futures (which allow registering error callbacks).  Also, I would prefer the onXXX properties to be full-fledged multicast events (see http://dartbug.com/1873), similar to those in dart:html, which would allow passing the objects around without worrying about anyone removing (or even knowing about) your handler.  For example:

stdin.onData.then(doStuff);

stdin.onData.cancel(doStuff);

This could of course be provided by a wrapper, but it doesn't seem like too much overhead.

Also, regardless of whether full-fledged events were used, maybe consider reusing dart:html's naming convention:

// without events

stdin.on.data = doStuff

stdin.on.data = null;

// with events

stdin.on.data.then(doStuff);

stdin.on.data.cancel(doStuff);

Cheers,

Sean

On Thu, Mar 1, 2012 at 8:14 AM, Rémi Forax <fo...@univ-mlv.fr> wrote:

On 03/01/2012 02:59 PM, Mads Ager wrote:

Rémi

[Mads Ager]

Hi Sean,

we did consider going with object.on.event.add(handler). However,
having a collection of handlers for dart:io events usually doesn't
make sense. Then you are down to object.on.event = stuff. At that
point I think it is better to just have object.onEvent = stuff.

Your point about error callbacks is a good one. For now we decided
that having it on the object is good enough. Once you get to having
multiple callbacks for one method things get messy with passing in
callbacks. Let's see where this takes us. I'm not against changing all
of this again if it turns out that something else would be better. :)

One of the advantages of explicitly passing in the callback to
single-shot methods is that you get a very tight coupling between the
call and its continuation. You *have to* specify what happens next and
therefore it is hard to use the API incorrectly. Futures doesn't give
you that because you can forget to do the ".then(continuation)" part.

Cheers, -- Mads

[Ivan Posva]

On Thu, Mar 1, 2012 at 06:14, Rémi Forax <fo...@univ-mlv.fr> wrote:
> On 03/01/2012 02:59 PM, Mads Ager wrote:
>>

> Rémi

Remi,

Regardless of how threads are mapped to isolates calling a *Sync
operation will suspend your isolate from handling any other requests.
That is what Mads referred to as a "Bad Thing for scalability".

In any case how threads are assigned to isolates is an implementation
detail and is orthogonal to this discussion.

-Ivan

[Sean Eagan]

On Thu, Mar 1, 2012 at 10:34 AM, Mads Ager <ag...@google.com> wrote:

Hi Sean,

we did consider going with object.on.event.add(handler). However,
having a collection of handlers for dart:io events usually doesn't
make sense.

I think it could be pretty common for stdin, stdout, and stderr at least.

Then you are down to object.on.event = stuff. At that
point I think it is better to just have object.onEvent = stuff.

I don't have much of a preference between `object.on.event` and `object.onEvent`, I just think it ought to be consistent between dart:html, dart:io, etc.

Your point about error callbacks is a good one. For now we decided
that having it on the object is good enough. Once you get to having
multiple callbacks for one method things get messy with passing in
callbacks.

The only time I see it being tricky is when the method accepts other optional arguments as well, as with File#open:

open(void success(bool exists), [FileMode mode, void error(String error)/* = this.onError*/]);

// or should it be

open(void success(bool exists), [void error(String error)/* = this.onError*/, FileMode mode]);

which is not an issue with Futures.

Another option would be to combine the success and error callbacks by adding an optional `String error` parameter to the success callback signatures:

open(void success(bool exists, [String error]), [FileMode mode]);

You wouldn't want to be required to specify this extra parameter in your callback, but with http://dartbug.com/1827 you wouldn't be.

Let's see where this takes us. I'm not against changing all
of this again if it turns out that something else would be better. :)

One of the advantages of explicitly passing in the callback to
single-shot methods is that you get a very tight coupling between the
call and its continuation. You *have to* specify what happens next and
therefore it is hard to use the API incorrectly. Futures doesn't give
you that because you can forget to do the ".then(continuation)" part.

That's a good point, but completers could choose to throw exceptions when no callbacks are registered for futures by the time the operation is complete.  Regarding Future being too much overhead, the single-shot methods could let you choose between the two styles by allowing you to pass callbacks, but if you don't, then returning a Future:

// returns null iff success is null

Future<void> exists([void success(), void failure()]);

// then one can choose between the following

file.exists(success);

file.exists().then(success);

Cheers,
Sean

[Sean Eagan]

On Thu, Mar 1, 2012 at 11:35 AM, Sean Eagan <seane...@gmail.com> wrote:

oops, a couple corrections...

Another option would be to combine the success and error callbacks by adding an optional `String error` parameter to the success callback signatures:

open(void success(bool exists, [String error]), [FileMode mode]);

void exists(void success(bool exists, [String error]), [FileMode mode]);

 

// returns null iff success is null

// returns a Future if success is null, otherwise null

[Ladislav Thon]

Then you are down to object.on.event = stuff. At that
point I think it is better to just have object.onEvent = stuff.

I don't have much of a preference between `object.on.event` and `object.onEvent`, I just think it ought to be consistent between dart:html, dart:io, etc.

I'm probably hijacking the thread now, but I will say it anyway: I don't. I'm not a fan of unifying Dart's event model, at least not right now. All these different event consumers and producers have different needs and they should find their own way of handling asynchronicity. For example, Node.js uses callbacks for async IO successfully, and AFAIK they tried to use futures in the past and decided not to. This is really a place where we shouldn't rush on some "unification", "consolidation" or whatever, the consensus just isn't there yet and we should only try to find it by writing lots of code. And I guess that it were exactly such practical inconveniences of the previous model in dart:io that lead to this change.

Sorry for spam :-)

LT