On 2013/06/03 13:47:32, ben2 wrote:
Replying to things that are unclear. No comment means "I agree" or "will
do."
On 2013/06/03 13:09:04, Dmitry Lomov (chromium) wrote:
> Ben,
> I appreciate the work you put into this patch.
>
> However, I think this implementation is a little premature: 1. DataView
is
not
> yet very well specified in ES6 draft
I don't exactly disagree but it's already out there. SM implements it and
people
use it. Node.js too, for that matter.
Sure, but I am unconvinced as yet that this is the right thing to put into
V8
_yet_.
> 2. much of the implementation of typed arrays/array buffers etc.,
especially
on
> API side, might change significantly while we are integrating this in
Blink.
On
> implementation side, one thing that is not finished at all yet is
ArrayBuffer
> neutering, and that will affect many implementation details here.
What implementation details are we talking here? Neutering is a fairly
straightforward operation, isn't it, both conceptually and
implementation-wise?
Well, you need to maintain weak lists of views of array buffers (because
once a
buffer is neutered all its views need to be neutered too).
Here is some ongoing work: https://codereview.chromium.org/15562008/.
Frankly I am not too thrilled at the perspective of merging that CL with
your
stuff :)
> If you noticed, I have been implementing ArrayBuffers and friends in
V8, and
I
> postponed tackling DataView for precisely these reasons.
>
> If you feel like continuing working on this patch, here are a few bigger
things
> that would be needed:
>
> 1. Please split this patch into two parts: one concerning JavaScript
side of
> things (implementation of constructor and methods), and another
concerning
V8
> API (include/v8.h, api.h etc).
> 2. Generally there should be a base class for JSTypedArray an
JSDataView,
such
> as JSArrayBufferView, that shares byte_offset, byte_length and buffer.
> 2. Please add way more tests. I put some suggestions below, also you
might
want
> to take a look at WebKit layout tests.
> Also, my understanding is that your tests will not work on big-endian
machine.
Why is that?
Ah, ok I see. I though you have that test, but you do not ;)
Well, you need to test that values that you set through DataView functions
end
up correctly in the underlying ArrayBuffer. Something like:
var dv = new DataView(ab)
var u = new Uint8Array(ab)
dv.setUint16(0xAAFF);
if (%IsLittleEndian()) {
assert(0xFF, u[0]);
assert(0xAA, u[1]);
} else { /* vice versa */ }
https://codereview.chromium.org/15943002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
