Modify global compile tool to compile by package

Modify global compile tool to compile by package

This produces output closer to what we get in bazel - you may want to use this to analyzer load perf.  It will also let me drill down on individual compile steps.

Using process & async adds complexity, but a lot of speedup over doing it linearly.

R=jmesserly@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/b705b2e5b18c9092289ef9447fdba81a1d46fca6

[vsm, 2016-05-10 20:21:43]

Description was changed from

==========
Modify global compile tool to compile by package
==========

to

==========
Modify global compile tool to compile by package

This produces output closer to what we get in bazel - you may want to use this
to analyzer load perf.  It will also let me drill down on individual compile
steps.

Using process & async adds complexity, but a lot of speedup over doing it
linearly.
==========

[vsm, 2016-05-10 20:21:43]

vsm@google.com changed reviewers:
+ jmesserly@google.com, leafp@google.com

[Jennifer Messerly, 2016-05-10 20:43:00]

Some comments but this LGTM-ish. I do wonder if we can start using a normal
build graph tool though, that would save us implementation work and also be
closer to the real-world Bazel usage model.

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dart
File tool/global_compile.dart (right):

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:17: const ENTRY = "main";
style nit: this should not be upper camel case :)
https://www.dartlang.org/effective-dart/style/#identifiers

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:26: ..addFlag('unsafe-force-compile',
I may have asked this before but, shouldn't this just populate with our existing
args from src/compiler/command.dart and src/analyzer/context.dart?

they both have static methods to make it easy to populate any ArgParser for just
this reason :)

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:33: ..addOption('tmp',
any reason to not always use system temp? I'm trying to think what Unix
conventions are for these kinds of commands. I think usually temp is usually
controlled by an environment variable right?
https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html

there is also a flag in GCC/Clang to save temp files to the output dir:
http://clang.llvm.org/docs/CommandGuide/clang.html#cmdoption-save-temps

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:35: 'Directory for temporary artifacts (defaults to a
system tmp directory)');
long line

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:68: // this?
yes indeed there is! See

https://github.com/dart-lang/dev_compiler/blob/335c43428ee67b93cda49b9dc9279b...

=)

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:96: // Build modules asynchronously
Would it be easier to generate a Makefile and just call "make"?

The makefile might even be a useful artifact, then when you're iterating, you
can just call "make" and get incrementalism "for free"

That's also in the Unix philosophy of keeping commands small and modular :)

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:121: return ready.then((result) {
this could be an `await`
https://www.dartlang.org/effective-dart/usage/#prefer-asyncawait-over-using-r...

(I know the outer one can't because you're trying on purpose to spawn things in
parallel, but this one looks like it can be)

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:143: last.then((_) {
same this could be async/await

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:156: final dependenceMap = new Map<String,
Set<String>>();
all of these maps do make me wonder if we should have a graph abstraction?

Past a certain point, is it really that we have a graph node here and we're just
splitting out its fields into different Map values?

https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
tool/global_compile.dart:167: void orderModules(
fyi ... I didn't look at all this build graph stuff too closely, let me know if
you want me to, I feel like eventually we'll replace with a real build too
(Make/Bazel) so it doesn't seem like something we need to worry much about.

[vsm, 2016-05-11 21:56:28]

Description was changed from

==========
Modify global compile tool to compile by package

This produces output closer to what we get in bazel - you may want to use this
to analyzer load perf.  It will also let me drill down on individual compile
steps.

Using process & async adds complexity, but a lot of speedup over doing it
linearly.
==========

to

==========
Modify global compile tool to compile by package

This produces output closer to what we get in bazel - you may want to use this
to analyzer load perf.  It will also let me drill down on individual compile
steps.

Using process & async adds complexity, but a lot of speedup over doing it
linearly.

R=jmesserly@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/b705b2e5b18c9092289ef9447fdb...
==========

[vsm, 2016-05-11 21:56:29]

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

[vsm, 2016-05-11 21:57:25]

On 2016/05/10 20:43:00, John Messerly wrote:
> Some comments but this LGTM-ish. I do wonder if we can start using a normal
> build graph tool though, that would save us implementation work and also be
> closer to the real-world Bazel usage model.
> 
> https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dart
> File tool/global_compile.dart (right):
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:17: const ENTRY = "main";
> style nit: this should not be upper camel case :)
> https://www.dartlang.org/effective-dart/style/#identifiers
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:26: ..addFlag('unsafe-force-compile',
> I may have asked this before but, shouldn't this just populate with our
existing
> args from src/compiler/command.dart and src/analyzer/context.dart?
> 
> they both have static methods to make it easy to populate any ArgParser for
just
> this reason :)
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:33: ..addOption('tmp',
> any reason to not always use system temp? I'm trying to think what Unix
> conventions are for these kinds of commands. I think usually temp is usually
> controlled by an environment variable right?
> https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html
> 
> there is also a flag in GCC/Clang to save temp files to the output dir:
> http://clang.llvm.org/docs/CommandGuide/clang.html#cmdoption-save-temps
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:35: 'Directory for temporary artifacts (defaults to a
> system tmp directory)');
> long line
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:68: // this?
> yes indeed there is! See
> 
>
https://github.com/dart-lang/dev_compiler/blob/335c43428ee67b93cda49b9dc9279b...
> 
> =)
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:96: // Build modules asynchronously
> Would it be easier to generate a Makefile and just call "make"?
> 
> The makefile might even be a useful artifact, then when you're iterating, you
> can just call "make" and get incrementalism "for free"
> 
> That's also in the Unix philosophy of keeping commands small and modular :)
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:121: return ready.then((result) {
> this could be an `await`
>
https://www.dartlang.org/effective-dart/usage/#prefer-asyncawait-over-using-r...
> 
> (I know the outer one can't because you're trying on purpose to spawn things
in
> parallel, but this one looks like it can be)
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:143: last.then((_) {
> same this could be async/await
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:156: final dependenceMap = new Map<String,
> Set<String>>();
> all of these maps do make me wonder if we should have a graph abstraction?
> 
> Past a certain point, is it really that we have a graph node here and we're
just
> splitting out its fields into different Map values?
> 
>
https://codereview.chromium.org/1965013003/diff/20001/tool/global_compile.dar...
> tool/global_compile.dart:167: void orderModules(
> fyi ... I didn't look at all this build graph stuff too closely, let me know
if
> you want me to, I feel like eventually we'll replace with a real build too
> (Make/Bazel) so it doesn't seem like something we need to worry much about.

Thanks for the comment!  Pushing so Leaf can use this.  With iterate on a
followup CL.