Dart2js, move deferred global vars to the right output unit.

Dart2js, move deferred global variables to the right output unit.

BUG=dartbug.com/21840
R=floitsch@google.com

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

[sigurdm, 2014-12-11 09:19:59]

Patchset #1 (id:1) has been deleted

[sigurdm, 2014-12-11 09:20:17]

sigurdm@google.com changed reviewers:
+ floitsch@google.com

[karlklose, 2014-12-11 09:25:20]

karlklose@google.com changed reviewers:
+ karlklose@google.com

[karlklose, 2014-12-11 09:25:20]

DBC:

'vars' -> 'variables' in the CL description?

https://codereview.chromium.org/795923003/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://codereview.chromium.org/795923003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1008: if (ou !=
outputUnit) continue;
Inline ou?

[floitsch, 2014-12-11 09:46:28]

LGTM with one comment.
Please add more tests.

https://codereview.chromium.org/795923003/diff/40001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://codereview.chromium.org/795923003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1007: OutputUnit ou =
compiler.deferredLoadTask.outputUnitForElement(element);
Don't use "ou".

https://codereview.chromium.org/795923003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1008: if (ou !=
outputUnit) continue;
Don't always continue.
If it's the main output unit add a `$isolateProperties.# = 0;`
Otherwise we will have a map transition for all statics when we load a new
library.

[sigurdm, 2014-12-11 13:26:45]

Patchset #4 (id:80001) has been deleted

[sigurdm, 2014-12-11 13:27:39]

Patchset #4 (id:100001) has been deleted

[sigurdm, 2014-12-11 13:28:50]

PTAL

https://codereview.chromium.org/795923003/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://codereview.chromium.org/795923003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1008: if (ou !=
outputUnit) continue;
On 2014/12/11 09:25:20, karlklose wrote:
> Inline ou?

Done.

https://codereview.chromium.org/795923003/diff/40001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://codereview.chromium.org/795923003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1007: OutputUnit ou =
compiler.deferredLoadTask.outputUnitForElement(element);
On 2014/12/11 09:46:28, floitsch wrote:
> Don't use "ou".

Done.

https://codereview.chromium.org/795923003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1008: if (ou !=
outputUnit) continue;
On 2014/12/11 09:46:28, floitsch wrote:
> Don't always continue.
> If it's the main output unit add a `$isolateProperties.# = 0;`
> Otherwise we will have a map transition for all statics when we load a new
> library.

Done.

[floitsch, 2014-12-11 13:37:30]

LGTM.

https://codereview.chromium.org/795923003/diff/120001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://codereview.chromium.org/795923003/diff/120001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:1014: initialValue =
jsAst.number(0);
Comment, on why we do this.

https://codereview.chromium.org/795923003/diff/120001/tests/language/deferred...
File tests/language/deferred_global_test.dart (right):

https://codereview.chromium.org/795923003/diff/120001/tests/language/deferred...
tests/language/deferred_global_test.dart:60: asyncEnd();
This asyncEnd is before the lib.loadLibrary() above can finish.

Easiest is you move lines 59 to line 50, and then move the asyncEnd into the
closure at line 55.

[sigurdm, 2014-12-11 13:56:40]

Committed patchset #5 (id:140001) manually as 42287 (presubmit successful).