Issue 1166343002: Don't load barback for "pub run" unless necessary.

Issue 1166343002: Don't load barback for "pub run" unless necessary. (Closed)

Created:
5 years, 6 months ago by nweiz

Modified:
5 years, 6 months ago

Reviewers:
Bob Nystrom

CC:
reviews_dartlang.org

Base URL:
git@github.com:dart-lang/pub.git@master

Target Ref:
refs/heads/master

Visibility:
Public.

More Reviews

Description

Don't load barback for "pub run" unless necessary. If all packages depended on by an executable don't use transformers, this will avoid loading barback entirely and instead run the VM against the Dart files on the filesystem. This shaves about 600ms off the load time for "pub run test" when running from source. R=rnystrom@google.com Committed: https://github.com/dart-lang/pub/commit/e1c16aa2790d3d13b53f25665040f99e2b07fe5e

Patch Set 1 #

Total comments: 4

Patch Set 2 : Code review changes #

Created: 5 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+95 lines, -46 lines)			Patch
M	lib/src/command/run.dart	View		1 chunk	+1 line, -3 lines	0 comments	Download
M	lib/src/executable.dart	View	1	3 chunks	+66 lines, -43 lines	0 comments	Download
M	lib/src/package_graph.dart	View	1	1 chunk	+28 lines, -0 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Bob Nystrom

LGTM. Super exciting! https://codereview.chromium.org/1166343002/diff/1/lib/src/executable.dart File lib/src/executable.dart (right): https://codereview.chromium.org/1166343002/diff/1/lib/src/executable.dart#newcode141 lib/src/executable.dart:141: // // TODO(nweiz): Once sdk#23369 is ...

5 years, 6 months ago (2015-06-09 00:15:33 UTC) #2

nweiz

Committed patchset #2 (id:20001) manually as e1c16aa2790d3d13b53f25665040f99e2b07fe5e (presubmit successful).

5 years, 6 months ago (2015-06-09 00:37:06 UTC) #4

nweiz

5 years, 6 months ago (2015-06-09 00:37:34 UTC) #5

Message was sent while issue was closed.

https://codereview.chromium.org/1166343002/diff/1/lib/src/executable.dart
File lib/src/executable.dart (right):

https://codereview.chromium.org/1166343002/diff/1/lib/src/executable.dart#new...
lib/src/executable.dart:141: // // TODO(nweiz): Once sdk#23369 is fixed, allow
global executables to be run
On 2015/06/09 00:15:33, Bob Nystrom wrote:
> Double "//"

Done.

https://codereview.chromium.org/1166343002/diff/1/lib/src/package_graph.dart
File lib/src/package_graph.dart (right):

https://codereview.chromium.org/1166343002/diff/1/lib/src/package_graph.dart#...
lib/src/package_graph.dart:90: if (package == null) return true;
On 2015/06/09 00:15:33, Bob Nystrom wrote:
> Document when this case comes into play.

Done.

Expand Messages | Collapse Messages