567259 - change "dense arrays" to "fast arrays"

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

We have "dense" arrays and "slow" arrays.  I'd like to change "dense" to "fast" for two reasons:

- It pairs more sensibly with "slow" because they are opposites.

- It's the same length as "slow", which leads to code lining up nicely.

This change would touch a lot of code, so it would be best folded in with (or done after) a patch that splits off JSFastArray and JSSlowArray from JSObject (in the style of bug 566789).  That would be a good way to do it because then JSObject::getDenseArrayLength() will become JSFastObject::getLength(), ie. a lot of the "DenseArray" sub-names will disappear anyway.

So the point of this bug is to solicit any objections to the idea.  Any objections?

I also don't like the fact that we have js_SlowArrayClass and js_ArrayClass.  It would be better if the latter is js_DenseArrayClass.  But this seems difficult as I think it gets tied to the JS-level "Array" via JS_PROTO.

Comment 1

[Andreas Gal :gal]

In my path I use JSValueArray and 
                 JSInt32Array and
                 JSArray (currently JSSlowArray).

Doesn't all line up but I think it makes more sense than fast imo. But I don't have a strong opinion either way. Whatever you prefer is fine with me.

Comment 2

[Nicholas Nethercote [inactive]]

(In reply to comment #1)
> In my path I use JSValueArray and 
>                  JSInt32Array and
>                  JSArray (currently JSSlowArray).

I was thinking of then subclassing FastArray with FastIntArray, FastJsvalArray (or FastValueArray), etc.

I don't like JSArray for slow arrays because js_ArrayClass is used for fast arrays.

Comment 3

[Jeff Walden [:Waldo]]

js::FastArrayObject and js::SlowArrayObject (note Object at end for parity with everything else; I kind of wish we could get rid of the Object suffix, but "String", "Number", and "Boolean" are just no good), but yeah.

Another reason: "dense" arrays aren't actually so due to holes, they're more dense-ish arrays.  r++++++ would review again on "dense-ish" (DenseIsh?) if you want it :-D

Comment 4

[Nicholas Nethercote [inactive]]

(In reply to comment #0)
> 
> I also don't like the fact that we have js_SlowArrayClass and js_ArrayClass. 
> It would be better if the latter is js_DenseArrayClass.  But this seems
> difficult as I think it gets tied to the JS-level "Array" via JS_PROTO.

This turns out easier than I thought.  In jsapi.cpp we just need to do something like this:

-    {js_InitArrayClass,                 EAGER_ATOM_AND_CLASP(Array)},
+    {js_InitArrayClass,                 EAGER_CLASS_ATOM(Array), &js_FastArray},

Comment 5

[Nicholas Nethercote [inactive]]

(In reply to comment #3)
> js::FastArrayObject and js::SlowArrayObject (note Object at end for parity with
> everything else; I kind of wish we could get rid of the Object suffix, but
> "String", "Number", and "Boolean" are just no good), but yeah.

IMHO losing the "Object" suffix would be a win.

The downside is that it's possible that newcomers to the code might think that "String" (js::String, to be precise) is some kind of generic string type, but a few seconds of code inspection will dispel that notion.  The corresponding danger is less for "Number" and "Boolean" because there are similar/identical native C++ types.

The upside is that it'll be a lot less typing.  And we already eschew the Object suffix in some cases, eg. isDenseArray(), isBoolean(), etc.

Comment 6

[Nicholas Nethercote [inactive]]

Can't be bothered.