Skeleton API for lazy library loading.

Skeleton API for lazy library loading.

Committed: https://code.google.com/p/dart/source/detail?r=18637

[ahe, 2013-01-07 12:12:11]

Sigh. No first-class libraries. I fought bravely.

[kasperl, 2013-01-07 12:18:07]

LGTM as a starting point.

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
File dart/sdk/lib/core/load_async.dart (right):

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
dart/sdk/lib/core/load_async.dart:21: * @OnLibraryLoaded('com.example.foo')
Maybe this would be better as WhenLibraryLoaded because it really isn't an event
handler itself. Not sure.

[gbracha, 2013-01-07 17:40:15]

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
File dart/sdk/lib/core/load_async.dart (right):

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
dart/sdk/lib/core/load_async.dart:26: */
1. The behavior of this annotation should be properly specified. Examples do not
constitute specification.
2. This is a language feature slipping in as an annotation, as it changes what
warnings are given.  If this is what we want, we should make it part of the
language spec.

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
dart/sdk/lib/core/load_async.dart:38: */
1. The comment says nothing about the semantics of this annotation.
2. From the examples, I conclude that this suppresses warnings generated by
DeferLoad. This should be stated here.
3. More substantively, this is a language change, as it effects what warnings
are generated. This is a slippery slope with magical annotations having language
effects.

[ahe, 2013-01-08 07:19:48]

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
File dart/sdk/lib/core/load_async.dart (right):

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
dart/sdk/lib/core/load_async.dart:38: */
On 2013/01/07 17:40:15, gbracha wrote:
> 1. The comment says nothing about the semantics of this annotation.
> 2. From the examples, I conclude that this suppresses warnings generated by
> DeferLoad. This should be stated here.
> 3. More substantively, this is a language change, as it effects what warnings
> are generated. This is a slippery slope with magical annotations having
language
> effects.

I agree. What do you suggest we do instead?

[Sean Eagan, 2013-01-14 15:57:52]

https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
File dart/sdk/lib/core/load_async.dart (right):

https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
dart/sdk/lib/core/load_async.dart:21: * @OnLibraryLoaded('com.example.foo')
Could the libraryName possibly default to the name used in the "as" clause of
the import statement if there is one, e.g. "foo" in this case?

https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
dart/sdk/lib/core/load_async.dart:51: external Future<bool> load(String
libraryName, {String uri});
Does this Future throw some sort of error, e.g. LibraryLoadException, if the
library could not be loaded?

Does the libraryName have to correspond to an @DeferLoad annotation directly in
this library, or could it also be from an @DeferLoad in a library which this
library imports or exports?

[ahe, 2013-01-14 16:17:36]

https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
File dart/sdk/lib/core/load_async.dart (right):

https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
dart/sdk/lib/core/load_async.dart:21: * @OnLibraryLoaded('com.example.foo')
On 2013/01/14 15:57:52, seaneagan1 wrote:
> Could the libraryName possibly default to the name used in the "as" clause of
> the import statement if there is one, e.g. "foo" in this case?

It is the name of the library, not the prefix. This allows the VM to detect
multiple libraries with the same name, without loading the sources eagerly
(which we want to avoid).

https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
dart/sdk/lib/core/load_async.dart:51: external Future<bool> load(String
libraryName, {String uri});
On 2013/01/14 15:57:52, seaneagan1 wrote:
> Does this Future throw some sort of error, e.g. LibraryLoadException, if the
> library could not be loaded?

Yes.

> 
> Does the libraryName have to correspond to an @DeferLoad annotation directly
in
> this library, or could it also be from an @DeferLoad in a library which this
> library imports or exports?

It can be in a different library, which is another reason to use library names,
not prefixes above.

[Sean Eagan, 2013-01-14 17:17:19]

On 2013/01/14 16:17:36, ahe wrote:
>
https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
> File dart/sdk/lib/core/load_async.dart (right):
> 
>
https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
> dart/sdk/lib/core/load_async.dart:21: * @OnLibraryLoaded('com.example.foo')
> On 2013/01/14 15:57:52, seaneagan1 wrote:
> > Could the libraryName possibly default to the name used in the "as" clause
of
> > the import statement if there is one, e.g. "foo" in this case?
> 
> It is the name of the library, not the prefix. This allows the VM to detect
> multiple libraries with the same name, without loading the sources eagerly
> (which we want to avoid).

In that case, maybe the name can just be detected from the library declaration
itself, removing the need to specify it in the @DeferLoad annotation.

> 
>
https://chromiumcodereview.appspot.com/11784018/diff/1/dart/sdk/lib/core/load...
> dart/sdk/lib/core/load_async.dart:51: external Future<bool> load(String
> libraryName, {String uri});
> On 2013/01/14 15:57:52, seaneagan1 wrote:
> > Does the libraryName have to correspond to an @DeferLoad annotation directly
> in
> > this library, or could it also be from an @DeferLoad in a library which this
> > library imports or exports?
> 
> It can be in a different library, which is another reason to use library
names,
> not prefixes above.

So then I guess if the dependency graph includes both a regular import and an
@DeferLoad import of the same library, that library will be loaded at startup.

[ahe, 2013-01-14 17:22:02]

On 2013/01/14 17:17:19, seaneagan1 wrote:
> In that case, maybe the name can just be detected from the library declaration
> itself, removing the need to specify it in the @DeferLoad annotation.

Reading the library declaration eagerly kinda defeats the purpose of lazy
loading (not for dart2js, but for the VM).

> So then I guess if the dependency graph includes both a regular import and an
> @DeferLoad import of the same library, that library will be loaded at startup.

Yes. That is something we would warn about.

[Sean Eagan, 2013-01-14 17:38:03]

On 2013/01/14 17:22:02, ahe wrote:
> On 2013/01/14 17:17:19, seaneagan1 wrote:
> > In that case, maybe the name can just be detected from the library
declaration
> > itself, removing the need to specify it in the @DeferLoad annotation.
> 
> Reading the library declaration eagerly kinda defeats the purpose of lazy
> loading (not for dart2js, but for the VM).

True.  I guess if we later get an import syntax which uses the library name
like:

import com.example.foo;

then it could be defaulted at least for those type of imports.

[ahe, 2013-02-18 12:55:30]

I'm thinking this API is a better fit for dart:async.

[floitsch, 2013-02-18 13:22:35]

DBC.

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
File dart/sdk/lib/core/load_async.dart (right):

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
dart/sdk/lib/core/load_async.dart:48: * The returned future if this invocation
of [load] caused the library
Comment seems incomplete.

https://codereview.chromium.org/11784018/diff/1/dart/sdk/lib/core/load_async....
dart/sdk/lib/core/load_async.dart:51: external Future<bool> load(String
libraryName, {String uri});
a top-level "load" sounds dangerous to me.

[ahe, 2013-02-18 13:39:13]

PTAL.

I figured that I could let the class serve a dual role as metadata and having
the "load" method. Dropped OnLibraryLoaded and any discussion of warnings for
now.

[kasperl, 2013-02-18 13:53:08]

I actually think this works pretty well.

https://codereview.chromium.org/11784018/diff/12001/dart/sdk/lib/async/deferr...
File dart/sdk/lib/async/deferred_load.dart (right):

https://codereview.chromium.org/11784018/diff/12001/dart/sdk/lib/async/deferr...
dart/sdk/lib/async/deferred_load.dart:13: * @lazy
So lazy is a user-chosen name now. Neat. Maybe we should come naming guidelines?
You import with the prefix 'foo' so you call the deferred library instance
deferred$foo? deferredFoo?

https://codereview.chromium.org/11784018/diff/12001/dart/sdk/lib/async/deferr...
dart/sdk/lib/async/deferred_load.dart:32: const DeferLoad(this.libraryName,
{this.uri});
DeferredLibrary

[floitsch, 2013-02-18 14:08:37]

I like that much more.

[ahe, 2013-02-18 14:32:11]

PTAL, I have added a dummy implementation and test to ensure that the basic API
works as expected. However, this does not actually defer loading. I'm working on
implementing that in dart2js in different CLs.

I'd like to submit this and use it as a basis for further work.

https://codereview.chromium.org/11784018/diff/12001/dart/sdk/lib/async/deferr...
File dart/sdk/lib/async/deferred_load.dart (right):

https://codereview.chromium.org/11784018/diff/12001/dart/sdk/lib/async/deferr...
dart/sdk/lib/async/deferred_load.dart:13: * @lazy
On 2013/02/18 13:53:08, kasperl wrote:
> So lazy is a user-chosen name now. Neat. Maybe we should come naming
guidelines?
> You import with the prefix 'foo' so you call the deferred library instance
> deferred$foo? deferredFoo?

I would use "lazyFoo", but I'll let other people have fun with guidelines. I
don't expect we'll see any in the Dart SDK.

https://codereview.chromium.org/11784018/diff/12001/dart/sdk/lib/async/deferr...
dart/sdk/lib/async/deferred_load.dart:32: const DeferLoad(this.libraryName,
{this.uri});
On 2013/02/18 13:53:08, kasperl wrote:
> DeferredLibrary

Done.

[floitsch, 2013-02-18 14:51:02]

LGTM.

https://codereview.chromium.org/11784018/diff/10002/dart/sdk/lib/async/deferr...
File dart/sdk/lib/async/deferred_load.dart (right):

https://codereview.chromium.org/11784018/diff/10002/dart/sdk/lib/async/deferr...
dart/sdk/lib/async/deferred_load.dart:13: * @lazy
I would follow some convention (as Kasper suggested).
for example:
@lazyFooLibrary

[kasperl, 2013-02-18 14:54:56]

LGTM.

[ngeoffray, 2013-02-18 15:01:09]

LGTM

[ahe, 2013-02-18 15:20:28]

https://codereview.chromium.org/11784018/diff/10002/dart/sdk/lib/async/deferr...
File dart/sdk/lib/async/deferred_load.dart (right):

https://codereview.chromium.org/11784018/diff/10002/dart/sdk/lib/async/deferr...
dart/sdk/lib/async/deferred_load.dart:13: * @lazy
On 2013/02/18 14:51:02, floitsch wrote:
> I would follow some convention (as Kasper suggested).
> for example:
> @lazyFooLibrary

As I implied to Kasper, I'm not keen on setting precedent. Calling this "lazy"
doesn't work if you have multiple libraries, so people need to come up with
different conventions. Once the debate has settled down, I'll be happy to
approve a CL to change this comment.