Puts JsScope on a diet

Puts JsScope on a diet

Lazy initialization of the names field and interning the description field
reduces memory consumption from > 128M to < 64 M in a unit test that was
suffering from OOM problems.

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

[zundel, 2011-11-29 15:03:05]

http://codereview.chromium.org/8659022/diff/3/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/backend/js/AbstractJsBackend.java
(right):

http://codereview.chromium.org/8659022/diff/3/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/AbstractJsBackend.java:378: //
on the heap when running some unit tests.
In the case of this unit test:


 dartc --noincremental
./tests/co19/src/LangSpecTest/13_Types/13_4_Interface_Types/13_4_Interface_Types_A11_t02.dart

The loop is allocating ~2000 JsScope instances (2 per class declared in the
file) in each loop.  Since there are ~1000 classes, we get 2 million live 
JsScope instances (2000 per JsProgram).

I looked at changing GenerateScopesAndNames, but there is a strong tie between a
JsProgram instance (of which there is a new one per class) and JsScope.  We
could try to save JsScope instances by building one root scope, then re-setting
the scopes to a new JsProgram each time through the loop.

[zundel, 2011-11-29 16:51:59]

This change reduces the number of JsScope instances in the aforementioned test
from 2,000,000 to about 17,000 by modifying GenerateNamesAndScopes to skip over
classes not being generated in this pass. 

This change incorporates default heap settings for dartc on Mac (256M) due to
problems reported by David Chandler.  Additionally I constrained the heap to
128M on all architectures for the unit tests to catch memory problems like this
in the future.

Overall, I didn't notice a significant change in the speed of running the unit
tests, other than my workstation feels more responsive when they are running
(presumably due to using less memory than before.)

[zundel, 2011-11-29 19:21:55]

Ready for review - I think I am done changing my mind on this patch.

As an experiment, I cranked the heap down to 32M and only 2 tests failed, both
in the parser/scanner (the files being parsed are about 400K long.)


=== debugia32 dartc co19/LibTest/core/List/sort/List/sort/A01/t06 ===          
Path: co19/LibTest/core/List/sort/List/sort/A01/t06
java.lang.OutOfMemoryError: Java heap space
	at com.google.dart.compiler.parser.DartScanner.<init>(DartScanner.java:492)
	at com.google.dart.compiler.parser.DartScanner
...
Command: out/Debug_ia32/compiler/bin/dartc_test --ignore-unrecognized-flags
/home/zundel/dart/tests/co19/src/LibTest/core/List/sort/List_sort_A01_t06.dart
--- CRASHED ---
=== debugia32 dartc co19/LibTest/core/List/sort/List/sort/A01/t05 ===          
Path: co19/LibTest/core/List/sort/List/sort/A01/t05
java.lang.OutOfMemoryError: Java heap space
	at
com.google.dart.compiler.ast.DartIntegerLiteral.get(DartIntegerLiteral.java:15)	
...
Command: out/Debug_ia32/compiler/bin/dartc_test --ignore-unrecognized-flags
/home/zundel/dart/tests/co19/src/LibTest/core/List/sort/List_sort_A01_t05.dart

[fabiomfv, 2011-11-29 19:34:53]

IIRC we had removed interner due to compilation time. did you have the chance to
measure the impact of putting it back? it would be nice before we commit this
change.

[zundel, 2011-11-29 19:45:48]

On 2011/11/29 19:34:53, fabiomfv wrote:
> IIRC we had removed interner due to compilation time. did you have the chance
to
> measure the impact of putting it back? it would be nice before we commit this
> change.

Since we are creating far fewer scopes, it isn't as much of an issue, but seeing
as nearly all of these scope descriptions are the same (most are named 'temp' or
'function $Constructor', its a flagrant waste of memory.

[zundel, 2011-11-29 20:01:55]

Rietveld ate my last comment.

codefu and I briefly looked at the DartScanner memory usage and I filed this
bug.

http://code.google.com/p/dart/issues/detail?id=634

[mmendez, 2011-12-01 14:53:48]

LGTM

Part of the reason why we use multiple JsProgram's is to ensure proper ordering
between globals and other top level elements.  Using JsPrograms was just an
expedient solution at the time.

Overall, we spend a lot of time generating a JsAST to either produce a string,
which we write to a file, or to create a closure AST.  We need to ask ourselves
whether we really expect to implement optimizations like inlining, etc at the JS
level.  If we don't it may be simpler to just rip that stuff out and produce a
string.  That said, ripping it out would be a big change in and of itself so we
should see how the native support debate evolves.