Load parallel transformers in the same isolate.

Load parallel transformers in the same isolate.

Transformers that are used in the package that defines them will be loaded in
the same isolate only if they're in the same phase. Otherwise, all transformers
within a given package will be loaded in the same isolate.

A future CL will take more advantage of this by more aggressively determining
when transformers can be safely loaded in parallel.

R=rnystrom@google.com

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

[Bob Nystrom, 2014-06-11 20:33:36]

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart (right):

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart:265: Future
load(Iterable<TransformerId> ids) {
Doc: "Skip any transformers that are already loaded."

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart:270: // same
isolate rather than spinning up a separate isolate for each one.
Is this TODO still meaningful?

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart (right):

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:21: /// [ids].
Document the return value.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:24: return
listToMapAsync(ids, (id) => id, (id) {
I think this would be easier to read if these were named parameters. That would
also let you avoid passing an explicit identity function.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:25: return
id.getAssetId(environment.barback);
=>

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:73:
firstErrorLine.startsWith("Failure getting ${idsToUrls[id]}:"));
Will this ever match more than one transformer?

[nweiz, 2014-06-12 20:11:18]

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart (right):

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart:265: Future
load(Iterable<TransformerId> ids) {
On 2014/06/11 20:33:35, Bob Nystrom wrote:
> Doc: "Skip any transformers that are already loaded."

Done.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart:270: // same
isolate rather than spinning up a separate isolate for each one.
On 2014/06/11 20:33:35, Bob Nystrom wrote:
> Is this TODO still meaningful?

Yes. It's about loading multiple instances of the same transformer that appear
in various places in the package graph (potentially with different
configurations) from the same isolate, which this change doesn't do.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart (right):

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:21: /// [ids].
On 2014/06/11 20:33:36, Bob Nystrom wrote:
> Document the return value.

Done.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:24: return
listToMapAsync(ids, (id) => id, (id) {
On 2014/06/11 20:33:35, Bob Nystrom wrote:
> I think this would be easier to read if these were named parameters. That
would
> also let you avoid passing an explicit identity function.

Will do in a separate CL.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:25: return
id.getAssetId(environment.barback);
On 2014/06/11 20:33:35, Bob Nystrom wrote:
> =>

I had it as that originally, but that followed by ".then" looked pretty awkward.

https://codereview.chromium.org/323263002/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart:73:
firstErrorLine.startsWith("Failure getting ${idsToUrls[id]}:"));
On 2014/06/11 20:33:36, Bob Nystrom wrote:
> Will this ever match more than one transformer?

I guess not. Changed.

[Bob Nystrom, 2014-06-12 21:52:14]

lgtm

[nweiz, 2014-06-12 23:12:56]

Committed patchset #3 manually as r37296 (presubmit successful).