Bug 40970

Created attachment 59341 [details] Patch v1 This is my take on the issue. Of course I would be glad to hear suggestions if you think there's a better way to handle this :)

I see two requests in this bug. 1. "I want a way to create an array with an initial length". I don't think it's a good idea to match the quirk of the JavaScript Array constructor here. It's a very confusing behavior to the uninitiated, and one we intentionally avoided in the API design. The simple way to get this behavior in the current API is to make an array and then set its length property. That seems sufficient to me. Plus, an author who really really wants the quirks of the Array constructor can just call the Array constructor. 2. "I want a way to preallocate the Array's internal storage, for performance reasons". Can you give me an example of a case where the Array's built-in behavior is insufficient? If there is such a case, I think the proper solution is to fix the built-in behavior, instead of adding to the C API. We want all array clients to go fast, not just C API clients.

Comment on attachment 59341 [details] Patch v1 r- because I don't think this change is warranted, but maybe you can convince me.

Thanks for the comments, Geoffrey. While I agree this is a quirk in the JavaScript Array API, JSC actually does something different instead of simply creating an empty Array and setting the length. It allocates storage in some situations and fill it with empty values. So, JSC code already optimizes this case for Array constructor (even calling it a quirk). The patch would just allow C API users to access this case just like JS Array constructor provides us. > We want all array clients to go fast, not just C API clients. I might misunderstood your comment, but the patch seems to work in the opposite direction. Enable C API clients to have same access as JavaScript clients have. The context of this patch is supporting the QScriptEngine::newArray(uint length), which follows the quirk on the builtin Array constructor. We can, of course, live with the empty followed by setLength(). But just feels a little bit "odd" since we do have special tuned code for this exact case, i.e. JSC internally optimizes this case instead of simply treating as a quirk.

> We can, of course, live with the empty followed by setLength(). But just feels a little bit "odd" since we do have special tuned code for this exact case, i.e. JSC internally optimizes this case instead of simply treating as a quirk. Don't read too much into the current optimization. It will probably change in the future. Though the current optimization is written for the JSArray constructor, I think a better solution going forward would be for JSArray to take any assignment to .length -- either an implicit assignment by the constructor quirk, or an explicit assignment by API or the .length property -- as a storage size hint. It would also be nice if JSArray could allocate spare storage larger than MIN_SPARSE_ARRAY_INDEX, at least for a few arrays, reclaiming wasted / fragmented space as needed. It's best not to expose these kinds of details in complex and quirky APIs. It might seem like the right thing to do looking at JavaScriptCore from the inside out, but API should be designed for understanding from the outside in.

Ok. I agree now that extending C API in this case isn't desirable. > Though the current optimization is written for the JSArray constructor, I think a better solution going forward would be for JSArray to take any assignment to .length -- either an implicit assignment by the constructor quirk, or an explicit assignment by API or the .length property -- as a storage size hint. This seems to be a better solution and keeps the C API clean. > It would also be nice if JSArray could allocate spare storage larger than MIN_SPARSE_ARRAY_INDEX, at least for a few arrays, reclaiming wasted / fragmented space as needed. You mean allowing a larger vector (when e.g. it was explicitly asked) even if it doesn't satisfy the minimum threshold that today's code use? If so, when the reclaim could take place? > It's best not to expose these kinds of details in complex and quirky APIs. It might seem like the right thing to do looking at JavaScriptCore from the inside out, but API should be designed for understanding from the outside in. I'll keep that advice in mind. Thanks. "Operational" question: should this bug be marked as WONTFIX or RESOLVED and another one created for the "use 'length' as storage size hint"?

(In reply to comment #6) > "Operational" question: should this bug be marked as WONTFIX or RESOLVED and another one created for the "use 'length' as storage size hint"? I've changed summary.

> > It would also be nice if JSArray could allocate spare storage larger than MIN_SPARSE_ARRAY_INDEX, at least for a few arrays, reclaiming wasted / fragmented space as needed. > > You mean allowing a larger vector (when e.g. it was explicitly asked) even if it doesn't satisfy the minimum threshold that today's code use? ...even if it exceeds the maximum threshold. > If so, when the reclaim could take place? Probably at GC time. Maybe on a round-robin basis as new Array buffers are allocated. > "Operational" question: should this bug be marked as WONTFIX or RESOLVED and another one created for the "use 'length' as storage size hint"? Probably WONTFIX, with a new bug for using "length" as a storage hint, and another bug for using storage hints more aggressively.