558263 - Make the value in dslots[-1] consistent

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

For dense arrays, dslots[-1] holds the dslots capacity.  For all other objects, it holds the dslots capacity + JS_INITIAL_SLOTS.  This is really confusing and error-prone (js_GetObjectTotalSize() gets it wrong, for example).

I propose to change it so that dslots[-1] always holds the dslots capacity.  This will require some more adds/subs in some places and fewer in others, so I expect it'll make no difference, performance-wise.

This bug depends on bug 558262, which will encapsulate all dslots accesses.  That will make this change much easier.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

+1 to making this more consistent, I experimented with encapsulating dslots in bug 553681 and must have gotten the capacity wrong in every way possible.

Comment 2

[Jeff Walden [:Waldo]]

Another possibility: do something vaguely (or so I hear) like what JSC does, and keep fslots/dslots in a union, storing the capacity of dslots adjacent to the dslots pointer.  The nice thing about that is that the dslots storage wouldn't hit reallocation edge cases as often -- if you have an array mapping, say, ASCII character codes to something, you aren't going to be allocating the next larger power-of-two just to hold dslots[-1].  See the comment in jsarray.cpp!EnsureCapacity; currently that 256-element array would require dslots with space for 512 elements.

Comment 3

[Jeff Walden [:Waldo]]

Er, make that dslots with space for 511 elements plus 1 for dslots[-1].

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #2)
> Another possibility: do something vaguely (or so I hear) like what JSC does,
> and keep fslots/dslots in a union, storing the capacity of dslots adjacent to
> the dslots pointer.

That's not this bug. It's bug 555128. Let's move this bug forward first.

Even for big objects, tracing can constant-fold the first few properties' slot address computation and load from obj, avoiding a dependent load of dslots or its future equivalent. This needs measurement before jumping on the JSC bandwagon. It could be ok if L1 hit rate is high enough but that's a big if, esp. with our GC!

/be

Comment 5

[Jeff Walden [:Waldo]]

Fine, whatever, pretend I didn't even mention JSC, just that I suggested storing dslots length in objects directly -- I only made the comment here because people in the office said, when I mentioned the bad off-by-one-at-power-of-two behavior with arrays, that this was where the comment should go.

Comment 6

[Brendan Eich [:brendan]]

Ok, pretend I didn't say "dependent load" :-/.

This is a righteous bug to fix. Gregor and I were confused by what the heck dslots[-1], and when it changes meaning.

/be

Comment 7

[Nicholas Nethercote [inactive]]

One of the aims of all this encapsulation I'm doing is to make it easier to do experiments with the layout -- such as storing dslots capacity directly.  But the encapsulation has to come first!  (Admittedly, this bug isn't so much about encapsulation although it does help slightly, because we won't have to change the capacity when a dense array becomes slow.)

Comment 8

[Nicholas Nethercote [inactive]]

Arrays no longer use dslots[-1];  the capacity is now stored in an fslot.  

So the question is now whether it's worth leaving dslots[-1] as holding the total number of slots (fslots + dslots) for non-arrays, or whether dslots[-1] should just hold the number of dslots.  I lean towards the latter, since we always know how many fslots there are, so including it in dslots[-1] is redundant.  Then again, bhackett is talking about variable-length objects...

Comment 9

[Brendan Eich [:brendan]]

To predict optimal fslots and avoid a branch, bhackett is making dslots point to fslots. This moves private out to a data member wasted on instances of classes that lack privates; more winningly, moves capacity from dslots[-1] to another data member of JSObject. The tracer can still constant fold its way to direct fslot addressing for the first N properties.

That doesn't leave much value in this bug unless the approach Brian is pursuing does not pan out.

/be

Comment 10

[Nicholas Nethercote [inactive]]

Bug 584917 fixed this by getting rid of dslots[-1] altogether.  Hooray!