Call dart2js directly from sdk/bin.

Call dart2js directly from sdk/bin.

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

[ricow1, 2012-11-27 18:28:35]

LGTM, although the vm options seems rather strange to me.

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js
File dart/sdk/bin/dart2js (right):

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js#newco...
dart/sdk/bin/dart2js:52:
EXTRA_VM_OPTIONS[${#EXTRA_VM_OPTIONS[@]}]='--heap_growth_rate=512'
This seems almost insane - I think the default max heap size is 512 MB so how
can we even grow this much on an existing heap?

Did you try experimenting with other flags like --new_gen_heap_size? I would
think that a bigger newspace may actually help here (in the long term hopefully
not, but at the moment probably so)

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js#newco...
dart/sdk/bin/dart2js:54: # Tell the VM to don't bother inlining methods. So far
it isn't
The comment could be misunderstood as saying that it does not pay of the disable
inlining, how about:
So far it isn't -> So far inlining isn't

https://codereview.chromium.org/11299206/diff/2001/dart/tools/testing/dart/te...
File dart/tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11299206/diff/2001/dart/tools/testing/dart/te...
dart/tools/testing/dart/test_suite.dart:127: // The script dart2js_developer is
not in the SDK.
this comment is slightly confusing, above we set prefix to sdk/bin but here we
state that the script is not in the sdk

[kustermann, 2012-11-28 08:53:50]

LGTM

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js
File dart/sdk/bin/dart2js (right):

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js#newco...
dart/sdk/bin/dart2js:20: done
Since you do this procedure two times (following symlinks) you could make it a
function.

[ahe, 2012-11-28 11:53:25]

Rico, Martin: thank you for your suggestions.

I have addressed your comments, but I realized that I can't submit this before
our benchmark runner is changed to not use dart2js from the build dir.

Cheers,
Peter

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js
File dart/sdk/bin/dart2js (right):

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js#newco...
dart/sdk/bin/dart2js:20: done
On 2012/11/28 08:53:50, kustermann wrote:
> Since you do this procedure two times (following symlinks) you could make it a
> function.

Done.

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js#newco...
dart/sdk/bin/dart2js:52:
EXTRA_VM_OPTIONS[${#EXTRA_VM_OPTIONS[@]}]='--heap_growth_rate=512'
On 2012/11/27 18:28:35, ricow1 wrote:
> This seems almost insane - I think the default max heap size is 512 MB so how
> can we even grow this much on an existing heap?
> 
> Did you try experimenting with other flags like --new_gen_heap_size? I would
> think that a bigger newspace may actually help here (in the long term
hopefully
> not, but at the moment probably so)

The flag is documented as:

heap_growth_rate: 4 (The size the heap is grown, in heap pages)

As far as I can tell, the heap page size is:

  static const intptr_t kPageSize = 256 * KB;

256*1024*512 = 128M

Regardless, this option is already added to dart2js and have shown improved
compile-time on our internal performance tracker.  This change is simply to make
sure we use the same value everywhere.  See the deleted file build_helper.dart.

https://codereview.chromium.org/11299206/diff/2001/dart/sdk/bin/dart2js#newco...
dart/sdk/bin/dart2js:54: # Tell the VM to don't bother inlining methods. So far
it isn't
On 2012/11/27 18:28:35, ricow1 wrote:
> The comment could be misunderstood as saying that it does not pay of the
disable
> inlining, how about:
> So far it isn't -> So far inlining isn't

Done.

https://codereview.chromium.org/11299206/diff/2001/dart/tools/testing/dart/te...
File dart/tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11299206/diff/2001/dart/tools/testing/dart/te...
dart/tools/testing/dart/test_suite.dart:127: // The script dart2js_developer is
not in the SDK.
On 2012/11/27 18:28:35, ricow1 wrote:
> this comment is slightly confusing, above we set prefix to sdk/bin but here we
> state that the script is not in the sdk

Done.

[ahe, 2012-11-28 11:54:04]

Kasper has "volunteered" to change how we run dart2js benchmarks :-)

[ahe, 2012-11-28 12:46:54]

Patch set 3 removes the changes to the build system.  I'll land those when our
benchmark runners are ready.

[ricow1, 2012-11-28 13:22:50]

LGTM