592534 - InitScopeForObject should never make an empty shape (it needs a new name, too)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Brendan Eich [:brendan]]

This non-native (null proto) or class-differs-between-proto-and-new-instance hazard makes for shape explosion (see bug 592001, bug 570435, bug 554626, and others I'm forgetting).

We want either (a) proto and instance to have same class; (b) singleton per runtime, thread-data, or compartment (runtime now, thread-data or compartment soon) shared among all instances.

I just noticed that the righteous fix for bug 438633, to give JSScript structs created for API callers of JS_Compile*Script* GC protection by giving each script an object wrapping it, walked into this InitScopeForObject hazard, because of course the class differs. So we end up creating an empty shape per script compiled!

I will try to fix this. I may need help with XBL.

/be

Comment 1

[Brendan Eich [:brendan]]

(b) is a well-known singleton, that is, a dedicated C++ member of the appropriate VM data structure (JSRuntime, currently).

I noticed that JSC has memoized and well-typed structure pointers in its global object data structure. Lazily initialized, IIRC.

/be

Comment 2

[Igor Bukanov]

(In reply to comment #0)
> We want either (a) proto and instance to have same class; (b) singleton per
> runtime, thread-data, or compartment (runtime now, thread-data or compartment
> soon) shared among all instances.

singleton needs some form of garbage collection as FF creates classes on the fly. But I guess this is not hard as GC would just need to enumerate singleton hash and remove unreachable shape instances.

Also the runtime version needs some locking. So why not start from a thread-based hash?

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #2)
> (In reply to comment #0)
> > We want either (a) proto and instance to have same class; (b) singleton per
> > runtime, thread-data, or compartment (runtime now, thread-data or compartment
> > soon) shared among all instances.
> 
> singleton needs some form of garbage collection as FF creates classes on the
> fly. But I guess this is not hard as GC would just need to enumerate singleton
> hash and remove unreachable shape instances.

It's even easier if the classes that XBL and XPConnect synthesizes are "empty shape compatible", which I believe they are. More in a bit.

> Also the runtime version needs some locking. So why not start from a
> thread-based hash?

Sure, I was starting from where we are today (rt->emptyArgumentsShape, etc.). Probably gonna move to compartments for isolation (avoid risking "JS capability leaks").

/be

Comment 4

[Brendan Eich [:brendan]]

I believe this is totally fixed by bhackett's work. Brian, let me know if not. Thanks,

/be