Handle TypedDataView objects in Dart_TypedDataAcquireData and

Handle TypedDataView objects in Dart_TypedDataAcquireData and
Dart_TypedDataReleaseData
Committed: https://code.google.com/p/dart/source/detail?r=20482

[Ivan Posva, 2013-03-22 04:50:01]

https://codereview.chromium.org/12937010/diff/11/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:2630: // the fields using offsets for a more
efficient implementation.
How about making class _TypedListView known to the VM in a similar manner to the
way class _TypedList is partially implemented in the VM.

https://codereview.chromium.org/12937010/diff/11/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:1087: REGISTER_SCALARLIST_CLASS(Float32x4, float32x4);
Can we make it a goal to remove dart:scalarlist on Monday?

https://codereview.chromium.org/12937010/diff/11/runtime/vm/raw_object.cc
File runtime/vm/raw_object.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/raw_object.cc#new...
runtime/vm/raw_object.cc:297: !RawObject::IsTypedDataViewClassId(class_id)) {
We should not add an extra test into this time critical code. Either we need to
have these values in such a way that it is a simple test or we define this typed
data view within the VM.

https://codereview.chromium.org/12937010/diff/11/runtime/vm/snapshot.cc
File runtime/vm/snapshot.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/snapshot.cc#newco...
runtime/vm/snapshot.cc:1203: RawObject::IsTypedDataViewClassId(class_id)) {
ditto

[Mads Ager (google), 2013-03-22 08:45:13]

LGTM for the API part.

[siva, 2013-03-25 17:20:34]

https://codereview.chromium.org/12937010/diff/11/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:2630: // the fields using offsets for a more
efficient implementation.
I thought about this but for now choose against adding two more classes in the
VM (in this case we would need to setup the offsets in the VM code and have some
verification to ensure that the Dart fields and the VM assumptions match).

Maybe if this turns out to be super hot entrypoint we could consider that.

On 2013/03/22 04:50:01, Ivan Posva wrote:
> How about making class _TypedListView known to the VM in a similar manner to
the
> way class _TypedList is partially implemented in the VM.

https://codereview.chromium.org/12937010/diff/11/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:1087: REGISTER_SCALARLIST_CLASS(Float32x4, float32x4);
Yes, once the simd stuff has landed we could get rid of it.

On 2013/03/22 04:50:01, Ivan Posva wrote:
> Can we make it a goal to remove dart:scalarlist on Monday?

https://codereview.chromium.org/12937010/diff/11/runtime/vm/raw_object.cc
File runtime/vm/raw_object.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/raw_object.cc#new...
runtime/vm/raw_object.cc:297: !RawObject::IsTypedDataViewClassId(class_id)) {
I have removed this extra check and instead moved the code for typed data view
classes into the switch statement.


On 2013/03/22 04:50:01, Ivan Posva wrote:
> We should not add an extra test into this time critical code. Either we need
to
> have these values in such a way that it is a simple test or we define this
typed
> data view within the VM.

https://codereview.chromium.org/12937010/diff/11/runtime/vm/snapshot.cc
File runtime/vm/snapshot.cc (right):

https://codereview.chromium.org/12937010/diff/11/runtime/vm/snapshot.cc#newco...
runtime/vm/snapshot.cc:1203: RawObject::IsTypedDataViewClassId(class_id)) {
Made the same code changes.

On 2013/03/22 04:50:01, Ivan Posva wrote:
> ditto

[Ivan Posva, 2013-03-25 17:55:14]

LGTM -ip

[siva, 2013-03-25 20:18:52]

Committed patchset #4 manually as r20482 (presubmit successful).