Small changes to get the compilation size of code importing dart:html shorter.

Small changes to get the compilation size of code importing dart:html shorter.
Committed: https://code.google.com/p/dart/source/detail?r=11292

[ngeoffray, 2012-08-21 13:18:15]

It should be better to have our optimizers do (most of) the job, but we're not
there yet. By doing these changes I cut the output of a console.log program from
5000 lines to 300 lines.

So this code is critical enough that it requires this style of using 'JS'
whenever needed.

[kasperl, 2012-08-21 13:24:51]

LGTM, but I'd like to start using inlining to get more readable,
self-documenting uses of the JS helper.

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
lib/compiler/implementation/enqueue.dart:102: if (!compiler.hasIsolateSupport()
Add a comment that explains that this enables isolate support if we ever use
anything from withing the isolate library. Maybe cache element.getLibrary in a
local variable?

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/native_helper.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/native_helper.dart:124: return 'Instance of
'.concat(JS('String', '#', getTypeNameOf(obj)));
Shouldn't interpolation be okay as long as we know that the type of the
getTypeNameOf is a string? It is much more readable using interpolation. If you
need to keep it like this, it deserves a comment.

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/native_helper.dart:157: for (int i = 0; i <
JS('int', '#.length', _dynamicMetadata); i++) {
Florian landed support for very basic inlining. Would it make sense to start
adding top-level inlinable methods for some of these JS uses to give them a more
saying name?

[ngeoffray, 2012-08-23 14:06:01]

Thanks Kasper, PTAL

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
lib/compiler/implementation/enqueue.dart:102: if (!compiler.hasIsolateSupport()
On 2012/08/21 13:24:51, kasperl wrote:
> Add a comment that explains that this enables isolate support if we ever use
> anything from withing the isolate library. Maybe cache element.getLibrary in a
> local variable?

Done.

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/native_helper.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/native_helper.dart:124: return 'Instance of
'.concat(JS('String', '#', getTypeNameOf(obj)));
On 2012/08/21 13:24:51, kasperl wrote:
> Shouldn't interpolation be okay as long as we know that the type of the
> getTypeNameOf is a string? It is much more readable using interpolation. If
you
> need to keep it like this, it deserves a comment.

Changed to use back interpolation, and forcing the type of getTypeNameOf(obj) to
be of String.

https://chromiumcodereview.appspot.com/10828416/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/native_helper.dart:157: for (int i = 0; i <
JS('int', '#.length', _dynamicMetadata); i++) {
On 2012/08/21 13:24:51, kasperl wrote:
> Florian landed support for very basic inlining. Would it make sense to start
> adding top-level inlinable methods for some of these JS uses to give them a
more
> saying name?

After landing Florian's changes, I can confirm that the top-level JS methods are
inlined. I will refactor our code in js_helper to have these helpers in another
CL.

[kasperl, 2012-08-24 05:03:14]

LGTM.

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:102: if (!compiler.hasIsolateSupport()
You didn't add the comment? ... and you didn't cache element.getLibrary in a
local?

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
File lib/compiler/implementation/lib/native_helper.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/lib/native_helper.dart:184: method =
arrayGet(methods, 'Object');
Hmm. This isn't really getting from an array -- arrayGet take an int index.
You'll need something like propertyGet that takes a string. For now I would keep
this using JS unless you want to spend the extra time on introducing propertyGet
and using it (also in line 177).

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/lib/native_helper.dart:239: if (dartMethod !== null)
arraySet(methods, 'Object', dartMethod);
This should not use arraySet. It should use propertySet.

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/lib/native_helper.dart:308: var set = newJSObject();
newJsObject would be easier to read

[ngeoffray, 2012-08-24 07:32:51]

Thanks Kasper.

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:102: if (!compiler.hasIsolateSupport()
On 2012/08/24 05:03:14, kasperl wrote:
> You didn't add the comment? ... and you didn't cache element.getLibrary in a
> local?

That's odd. I was pretty sure I did it and uploaded it. Anyway, comment and
local variable added.

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
File lib/compiler/implementation/lib/native_helper.dart (right):

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/lib/native_helper.dart:184: method =
arrayGet(methods, 'Object');
On 2012/08/24 05:03:14, kasperl wrote:
> Hmm. This isn't really getting from an array -- arrayGet take an int index.
> You'll need something like propertyGet that takes a string. For now I would
keep
> this using JS unless you want to spend the extra time on introducing
propertyGet
> and using it (also in line 177).

Good catch. I added the property get helper.

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/lib/native_helper.dart:239: if (dartMethod !== null)
arraySet(methods, 'Object', dartMethod);
On 2012/08/24 05:03:14, kasperl wrote:
> This should not use arraySet. It should use propertySet.

Done.

https://chromiumcodereview.appspot.com/10828416/diff/3001/lib/compiler/implem...
lib/compiler/implementation/lib/native_helper.dart:308: var set = newJSObject();
On 2012/08/24 05:03:14, kasperl wrote:
> newJsObject would be easier to read

Done.