dart2js: Merge intercepted names map.

dart2js: Merge intercepted names map.

R=ahe@google.com

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

[floitsch, 2015-01-07 14:51:42]

floitsch@google.com changed reviewers:
+ ahe@google.com, zarah@google.com

[ahe, 2015-01-07 15:00:12]

Can you test this?

I'm not sure what you're merging. Could you elaborate a bit?

[floitsch, 2015-01-07 15:04:21]

When we do a mirror-based invocation we need to know if an instance method is
intercepted or not (i.e. if it needs the extra first argument).
We had two maps for that:
- one statically emitted.
- one dynamically filled for stubs (from intercepted methods).

With this change we only have one map (set) anymore.

No additional test, since there isn't any behavior change.

[ahe, 2015-01-07 15:31:32]

lgtm

I agree that no new tests are needed.

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

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/interceptor_emitter.dart:70: //
TODO(ahe): We should not generate the list of intercepted names at
You may also want to put this on you to-do list, or just remove this to-do.

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/interceptor_emitter.dart:73: if
(!compiler.enabledInvokeOn) return null;
Or mirrors enabled?

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/interceptor_emitter.dart:81:
properties[i] = new jsAst.Property(js.string(name), js.number(1));
Random thought about a potential optimization:

For is-tests, we found that initializing boolean properties to a dummy function
would generate faster accesses.

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

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/reflection_data_parser.dart:255: if
(#enabledInvokeOn)
Curly braces?

[floitsch, 2015-01-07 15:48:12]

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

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/interceptor_emitter.dart:70: //
TODO(ahe): We should not generate the list of intercepted names at
On 2015/01/07 15:31:32, ahe wrote:
> You may also want to put this on you to-do list, or just remove this to-do.

Changed to a comment.

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/interceptor_emitter.dart:73: if
(!compiler.enabledInvokeOn) return null;
On 2015/01/07 15:31:32, ahe wrote:
> Or mirrors enabled?

invokeOn is the only one using the data structure.
invokeOn implies reflection.

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/interceptor_emitter.dart:81:
properties[i] = new jsAst.Property(js.string(name), js.number(1));
On 2015/01/07 15:31:32, ahe wrote:
> Random thought about a potential optimization:
> 
> For is-tests, we found that initializing boolean properties to a dummy
function
> would generate faster accesses.

I thought about it. Here I went for simple and size. (Yes: one could have a
temporary one character variable with a function, but that's more complicated).

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

https://codereview.chromium.org/833623003/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/reflection_data_parser.dart:255: if
(#enabledInvokeOn)
On 2015/01/07 15:31:32, ahe wrote:
> Curly braces?

They would stay, when the if(#hole) is resolved.

[floitsch, 2015-01-08 14:16:51]

Committed patchset #4 (id:60001) manually as 42694 (presubmit successful).