640503 - Implement strings' length property with a plain old data property

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch, length already pretty well-tested elsewhere so doesn't need any

This is atop the patch for bug 640072 (with obvious changes to DefineConstructorAndPrototype to actually use the specified key, not JSProto_RegExp).  It's a pretty straightforward transliteration of bug 640072's patch to apply to String.

I see scary comments by some of the New* methods that suggest it really is necessary to use different ones to create the prototype object and to create subsequent String objects.  That's a shame if true, because it's easy to create but forget to init, and it'd be easy to not notice the mistake.  I'd love to see one JSObject::createStringObject (maybe for RegExp too, although I might be forgetting a complicating factor there), but I don't think I understand empty shapes well enough now to figure out how to make one, assuming that's possible -- a shame, since there's a handful of random scattered places where we create String objects for strings .

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Updated for tree churn

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 522161 [details] [diff] [review]
Updated for tree churn

Good stuff.

Comment 3

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/474e167e344a

Comment 4

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/474e167e344a

Comment 5

[Brian Hackett [Laid off!]]

Why are the initialStringShape and initialRegExpShape stored per-compartment?

There is a reasonably important invariant used by JITs that two objects with the same shape have the same prototype (this is also critical for plans to move obj->prototype into the shape), but there can be multiple String.prototype or RegExp.prototype objects in the same compartment and presumably different string or regexp objects associated with different prototypes could end up having the same shape.

Comment 6

[Brendan Eich [:brendan]]

(In reply to comment #5)
> Why are the initialStringShape and initialRegExpShape stored per-compartment?
> 
> There is a reasonably important invariant used by JITs that two objects with
> the same shape have the same prototype (this is also critical for plans to
> move obj->prototype into the shape), but there can be multiple
> String.prototype or RegExp.prototype objects in the same compartment and
> presumably different string or regexp objects associated with different
> prototypes could end up having the same shape.

New bug on file?

Compartment per global may fix this.

/be

Comment 7

[Luke Wagner [:luke]]

If I read comment 5 correctly then it makes sense that the shapes would need to be per-global so CPG would fix this.  Its not close, so sounds like these suckers need to be stashed on the global object.  When CPG does arrive, we can undo all this global-reserved-slots hackery and just stick things on the compartment.