lgtm once the comments are addressed.
Non-blocking observation: initializing Worker with a function without the
closure looks confusing. I am wondering whether we should not create workers
with a string. That at least removes the element of surprise. If one really
wants to do without the string, I guess you can still do something like
function f() { ... }
w = new Worker(f.toString() + "; f();");
https://codereview.chromium.org/1216023003/diff/20001/test/mjsunit/d8-worker-sharedarraybuffer.js
File test/mjsunit/d8-worker-sharedarraybuffer.js (right):
https://codereview.chromium.org/1216023003/diff/20001/test/mjsunit/d8-worker-sharedarraybuffer.js#newcode31
test/mjsunit/d8-worker-sharedarraybuffer.js:31: function f() {
I do not think this is doing what you think it's doing. E.g.
var x = false;
if (x) function f() { print("f"); }
f();
still happily prints f.
You really have to say
var f = function () { ... }
Even then I am not entirely sure why this has to be conditioned on
this.Worker being defined?
https://codereview.chromium.org/1216023003/diff/20001/test/mjsunit/d8-worker-sharedarraybuffer.js#newcode36
test/mjsunit/d8-worker-sharedarraybuffer.js:36: throw new
Error("SharedArrayBuffer transfer byteLength");
Nit: Could we get the braces here and below? (In general, we like braces
if the body is not on the same line.)
https://codereview.chromium.org/1216023003/diff/20001/test/mjsunit/d8-worker-sharedarraybuffer.js#newcode55
test/mjsunit/d8-worker-sharedarraybuffer.js:55: assertEquals(16,
sab.byteLength); // ArrayBuffer should not neutered.
In the comment: ... should not *be* neutered.
https://codereview.chromium.org/1216023003/diff/20001/test/mjsunit/d8-worker-sharedarraybuffer.js#newcode59
test/mjsunit/d8-worker-sharedarraybuffer.js:59: while ((ta0 =
Atomics.load(ta, 0)) == 0);
"while (<condition>);" is not recommended by the Chromium C++ style
guide (and we are trying to stick to the C++ style guide even for JS).
Either do "while (<condition>) continue;" or "while (<condition>) {}".
https://codereview.chromium.org/1216023003/diff/20001/test/mjsunit/d8-worker-sharedarraybuffer.js#newcode63
test/mjsunit/d8-worker-sharedarraybuffer.js:63: w.terminate();
Could we also check that the array has not been neutered after
termination?
https://codereview.chromium.org/1216023003/
--
--
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/d/optout.
