ArrayClass should imply @@isConcatSpreadable
On Mon, Oct 28, 2013 at 8:20 PM, Boris Zbarsky <bzbarsky at mit.edu> wrote:
In terms of existing ArrayClass objects that are shipping on the web right now, Gecko is shipping (though perhaps not in final releases yet) the .ports of a MessageEvent and the return value of getClientRects(). I think changing the concat() behavior of these should be OK. Certainly for .ports, which we haven't been shipping for very long at all.
Thoughts?
Could we still change those to actual arrays? I guess for .ports that might be trickier as it implies a readonly view. ArrayClass feels like a hack.
On 10/28/13 7:13 PM, Anne van Kesteren wrote:
Could we still change those to actual arrays? I guess for .ports that might be trickier as it implies a readonly view. ArrayClass feels like a hack.
getClientRects might be changeable.
.ports has exactly the problem you describe.
This came up today in a discussion of how we want to implement dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html in Gecko, and specifically how to implement the "axes" and "buttons" attributes on dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html#gamepad-interface. Our current implementation returns vanilla JS arrays, but returns a new one every get, which is pretty suboptimal. So we were considering changing them to some ArrayClass interface and thinking about what issues that might cause for callers...
If there is a better proposal for the "return an array that can be read but not written but I can still write to it" use case, I'm all ears. But as far as I can tell the way to do such a thing in ES is to return a filtering proxy to an actual array. Such a proxy would of course fail Array.isArray() checks, but presumably at some point proxies will be able to control their @@isConcatSpreadable, right? Or so I would hope... And then the proxy solution would quack a lot like ArrayClass with my proposal, I think.
On 10/28/13 7:43 PM, Boris Zbarsky wrote:
Our current implementation returns vanilla JS arrays, but returns a new one every get, which is pretty suboptimal. So we were considering changing them to some ArrayClass interface and thinking about what issues that might cause for callers...
To be clear, we could just change this particular API (Gamepad) to returning a plain-vanilla JS array and the same one every time and allow the caller to write to it; it's not like the API implementation ever reads this array, so there aren't even safety issues here. That still seems like it makes it really easy for a caller to accidentally modify the array and cause another caller to see incorrect data here, which smells quite fishy to me.
On Mon, Oct 28, 2013 at 4:13 PM, Anne van Kesteren <annevk at annevk.nl> wrote:
Could we still change those to actual arrays? I guess for .ports that might be trickier as it implies a readonly view.
Lets just return a frozen Array. I know that people on TC39 has said that it's ugly, but I still think it's far less ugly than creating a whole pile of host classes just because we lack immutable arrays.
ArrayClass feels like a hack.
Agreed.
On Mon, Oct 28, 2013 at 4:43 PM, Boris Zbarsky <bzbarsky at mit.edu> wrote:
If there is a better proposal for the "return an array that can be read but not written but I can still write to it" use case, I'm all ears.
My suggestion has been:
Return a frozen (and thus immutable) Array. When you need to change the contents of the array, create a new frozen Array which contains the new Array contents.
In the case of gamepads, the second rule would kick in when a gamepad was connected or disconnected.
The "i don't want others to change the array, but I want to be able to do so myself" is basically a "live array", which a lot of people has expressed dislike of. So maybe it's something we should avoid.
My proposal above has two nice properties:
x.arrayProp === x.arrayPropreturnstrue.x.arrayPropbehaves entirely like a normal array (includingArray.isArrayandArray.concat), as long as you don't try to mutate it.
On Oct 28, 2013, at 4:43 PM, Boris Zbarsky wrote:
... This came up today in a discussion of how we want to implement dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html in Gecko, and specifically how to implement the "axes" and "buttons" attributes on dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html#gamepad-interface. Our current implementation returns vanilla JS arrays, but returns a new one every get, which is pretty suboptimal. So we were considering changing them to some ArrayClass interface and thinking about what issues that might cause for callers...
On Oct 28, 2013, at 4:54 PM, Jonas Sicking wrote:
Lets just return a frozen Array. I know that people on TC39 has said that it's ugly, but I still think it's far less ugly than creating a whole pile of host classes just because we lack immutable arrays.
Those really are the two options if you don't want to create a communications path between clients, either return a fresh object to each time or have a single immutable object that is returned.
I don't think there is any thing wrong with using Object.freeze if you really need to return an immutable object. But what's wrong with returning a fresh object each time? Are these operations highly likely to be used in performance critical loops. If not, trying to share a result object sounds like premature optimization.
From: es-discuss [mailto:es-discuss-bounces at mozilla.org] On Behalf Of Allen
I don't think there is any thing wrong with using Object.freeze if you really need to return an immutable object. But what's wrong with returning a fresh object each time? Are these operations highly likely to be used in performance critical loops. If not, trying to share a result object sounds like premature optimization.
I think the issue is that these things are properties, either because of web legacy (as in some specifications) or because the spec writers conceptualize them as such and are reluctant to change them (for the newer specifications). And returning a fresh array from the getter each time is unpleasant. If they could be changed to methods, returning a new object each time makes a lot of sense.
That said I feel like this is a common enough need that it might be worth the DOM speccing a read-only proxy-onto-an-array view type that they could reuse. As Boris says it would probably be very similar to what ArrayClass does, so maybe ArrayClass is OK as-is, but I feel like fleshing it out in terms of real ES proxies would make it feel less hackish, perhaps?
(Such a type would be generally useful for not just DOM specs, IMO. Maybe I should work on a library prototyping this and if it's awesome and everyone loves it and uses it, someone can spec it officially.)
So what's so onerous about returning a fresh array from the getter each time it was called. If it was implemented in Es6, it would just be:
return Array.from(internal_compy);
Proxies aren't cheap. Unless these arrays tend to be quite large, I wouldn't be surprised if the overhead of using a Proxy for this useless ended up being higher than the cost of making fresh arrays.
From: Allen Wirfs-Brock [mailto:allen at wirfs-brock.com]
So what's so onerous about returning a fresh array from the getter each time it was called.
The fact that api.property !== api.property.
You mean people want to do identity checks of the value of the property? Why?
Sorry, for pushing too much on this, but issues like when to produce fresh objets, when its safe to share mutable object, and when there are object identity expectations are pretty fundamental issues when designing object based APIs.
On 10/28/13 7:54 PM, Jonas Sicking wrote:
Lets just return a frozen Array.
That works for .ports, I think, but not for the gamepad API case: in that case the array can change as gamepads are connected or disconnected...
On 10/28/13 8:08 PM, Jonas Sicking wrote:
Return a frozen (and thus immutable) Array. When you need to change the contents of the array, create a new frozen Array which contains the new Array contents.
Ah, hmm. I could probably live with that.
On 10/28/13 8:50 PM, Allen Wirfs-Brock wrote:
So what's so onerous about returning a fresh array from the getter each time it was called. If it was implemented in Es6, it would just be:
return Array.from(internal_compy);
Implementing this is pretty easy, for sure.
The questions are:
- What the object-identity expectations of callers are.
- Whether this can cause nasty O(N^2) behavior when people loop over the property's value in a naive way. Mostly an issue if the list can be long, so not a problem for the gamepad API but could be a problem for other use cases.
Proxies aren't cheap. Unless these arrays tend to be quite large, I wouldn't be surprised if the overhead of using a Proxy for this useless ended up being higher than the cost of making fresh arrays.
Maybe. Measuring in Firefox [1] a get on a proxy seems to be in the ~100ns range for me, while duplicating a small array (3 entries) via slice.call() seems to be about 300ns (mostly the call overhead). Duplicating an array with ~100 entries takes closer to 2000ns, just to see the scaling here.
[1] Testcase:
<!DOCTYPE html>
<body>
<pre><script>
var count = 4000000;
var arr = [1, 2, 3];
var slice = Array.prototype.slice;
var proxy = new Proxy(arr, {});
var sideEffect;
var i;
var start = new Date;
for (i = 0; i < count; ++i) {
}
var stop = new Date;
document.writeln("Empty loop: " + (stop - start) / count * 1e6 + "ns");
var start = new Date;
for (var i = 0; i < count; ++i) {
sideEffect = arr[0];
}
var stop = new Date;
document.writeln("Normal array get: " + (stop - start) / count * 1e6 + "ns");
var start = new Date;
for (var i = 0; i < count; ++i) {
sideEffect = proxy[0];
}
var stop = new Date;
document.writeln("Proxy get: " + (stop - start) / count * 1e6 + "ns");
var start = new Date;
for (var i = 0; i < count; ++i) {
sideEffect = slice.call(arr);
}
var stop = new Date;
document.writeln("Array creation, length " + arr.length +": " + (stop - start) / count * 1e6 + "ns");
arr = arr.concat(arr);
arr = arr.concat(arr);
arr = arr.concat(arr);
arr = arr.concat(arr);
arr = arr.concat(arr);
var start = new Date;
for (var i = 0; i < count; ++i) {
sideEffect = slice.call(arr);
}
var stop = new Date;
document.writeln("Array creation, length " + arr.length +": " + (stop - start) / count * 1e6 + "ns");
</script>
On 28 October 2013 21:20, Boris Zbarsky <bzbarsky at mit.edu> wrote:
As far as I can tell, the two places in ES5 that test [[Class]] being equal to "Array" are Array.isArray() and Array.prototype.concat.
In ES6, the former does some sort of brand check, but the latter calls IsConcatSpreadable, which checks for a @@isConcatSpreadable symbol.
It seems to me like we should probably have ArrayClass objects return true from the @@isConcatSpreadable symbol.
Honestly, I think we should rather get rid of @@isConcatSpreadable. It's far too special-cased (extending the object protocol for a single random method?). Why can't we simply use @@iterator instead?
Allen Wirfs-Brock wrote:
You mean people want to do identity checks of the value of the property? Why?
It might be helpful to look at it the other way around. If you spend a day chasing down a bug that boils down to the problem above triggered in some subtle way, would you say that the issue should have been caught in code review and there should have been tests for issues like this? Would you say it's safe to change a popular API from returning the same object to returning a different object every time?
It is not diffcult to end up with code that does something like this:
var temp = a.example;
...
if (...) {
...
temp = b.example;
}
...
if (temp === a.example) {
...
} else /* temp === b.example */ {
...
}
which works fine so long as a.example returns the same object; if it
does not, this would always take the else branch, but that is hard to
spot and counter-intuitive considering such odd behavior is very rare.
As far as I can tell, the two places in ES5 that test [[Class]] being equal to "Array" are Array.isArray() and Array.prototype.concat.
In ES6, the former does some sort of brand check, but the latter calls IsConcatSpreadable, which checks for a @@isConcatSpreadable symbol.
It seems to me like we should probably have ArrayClass objects return true from the @@isConcatSpreadable symbol.
The benefit is that this makes ArrayClass objects quack as much like an array as we can, I think.
The drawback is that this perhaps makes it harder to add ArrayClass to existing objects...
In terms of existing ArrayClass objects that are shipping on the web right now, Gecko is shipping (though perhaps not in final releases yet) the .ports of a MessageEvent and the return value of getClientRects(). I think changing the concat() behavior of these should be OK. Certainly for .ports, which we haven't been shipping for very long at all.
Thoughts?