Add a new interceptor class JsArray, and support intercepting some list methods with the new interc…

Add a new interceptor class JsArray, and support intercepting some list methods with the new interceptor scheme.
Committed: https://code.google.com/p/dart/source/detail?r=15009

[sra1, 2012-11-14 19:37:34]

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/interceptors.dart (right):

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/interceptors.dart:25:
getInterceptor(object) {
We will have very many different types flowing to this function so it is
unlikely to be specialized, and the sequence of tests to select the interceptor
is longer than many of the test sequences in the older code.

For example, $.clear() now does an unnecessary string test.

What is the plan for optimizing through this point of polymorphism?

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/js_array.dart (right):

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:18: JS('Object',
r'#.push(#)', this, value);
Use 'void'.
Otherwise the native behavior is to assume all native classes can be
instantiated.

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:27: return JS("E",
r'#.splice(#, 1)[0]', this, index);
Single quotes for consistency.

Naming a type parameters is going to have some implications for the native
behavior, but I'm not sure yet.  I might have change this.

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:48: JS('Object',
r'#.push(#)', this, value);
'void'

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:52: length = 0;
checkGrowable?

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:80: return
JS('List', r'#.slice(#, #)', this, start, end);
So this method returns a parameterized empty list or an unparameterized list. 
Can we make that consistent?

[ngeoffray, 2012-11-14 21:32:40]

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/interceptors.dart (right):

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/interceptors.dart:25:
getInterceptor(object) {
On 2012/11/14 19:37:34, sra1 wrote:
> We will have very many different types flowing to this function so it is
> unlikely to be specialized,

I assume you're talking about being specialized at runtime by the JS engine?

>  and the sequence of tests to select the interceptor
> is longer than many of the test sequences in the older code.
> 
> For example, $.clear() now does an unnecessary string test.
> 
> What is the plan for optimizing through this point of polymorphism?

Once we move all interceptors to use the new interceptor classes, there are many
things we can sort doing for optimizations. Among them and I'm sure we can come
up with others:

1) GVN the getInterceptor call. You're right that this method does many more
type checks than before, but note that if there are two intercepted calls in a
method with the same receiver, the getInterceptor call can be done only once.
Before we were doing the type checks twice.

2) Have multiple versions of getInterceptor. We know which interceptor classes
define a member. For example on a call to clear, we would call
getInterceptorArray, and on a call to indexOf, we would call
getInterceptorArrayOrString.

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/js_array.dart (right):

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:18: JS('Object',
r'#.push(#)', this, value);
On 2012/11/14 19:37:34, sra1 wrote:
> Use 'void'.
> Otherwise the native behavior is to assume all native classes can be
> instantiated.

Done.

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:27: return JS("E",
r'#.splice(#, 1)[0]', this, index);
On 2012/11/14 19:37:34, sra1 wrote:
> Single quotes for consistency.

Done.

> 
> Naming a type parameters is going to have some implications for the native
> behavior, but I'm not sure yet.  I might have change this.

Let me know if it becomes a problem for you. I used it because I found it cute
for now, but we can definitely postpone using it until we have a cleaner
implementation of parameterized lists.

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:48: JS('Object',
r'#.push(#)', this, value);
On 2012/11/14 19:37:34, sra1 wrote:
> 'void'

Done.

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:52: length = 0;
Will be done by the length setter.

On 2012/11/14 19:37:34, sra1 wrote:
> checkGrowable?

https://codereview.chromium.org/11275316/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_array.dart:80: return
JS('List', r'#.slice(#, #)', this, start, end);
On 2012/11/14 19:37:34, sra1 wrote:
> So this method returns a parameterized empty list or an unparameterized list. 
> Can we make that consistent?

Good catch. I made it consistent by not returning a parameterized list and added
a TODO.

[karlklose, 2012-11-16 11:42:43]

LGTM.

[ahe, 2012-11-23 09:10:01]

LGTM!