Bug 239044 – [JSC] Move StructureCache from VM to JSGlobalObject

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Move StructureCache from VM to JSGlobalObject

[Yusuke Suzuki]

Created attachment 457195 [details]
Patch

[Yusuke Suzuki]

Created attachment 457197 [details]
Patch

[Saam Barati]

Comment on attachment 457197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457197&action=review

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3144
> +                structure = globalObject->structureCache().emptyObjectStructureConcurrently(base.getObject(), JSFinalObject::defaultInlineCapacity());

This looks wrong. The runtime code is using the prototype's global object. But we're using the lexical location's global object. Is that mismatch ok?

[Yusuke Suzuki]

Comment on attachment 457197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457197&action=review

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3144
>> +                structure = globalObject->structureCache().emptyObjectStructureConcurrently(base.getObject(), JSFinalObject::defaultInlineCapacity());
> 
> This looks wrong. The runtime code is using the prototype's global object. But we're using the lexical location's global object. Is that mismatch ok?

I think Object.create is using its callee's JSGlobalObject (in this case, it is encoded as lexical global object), right?

```
   628 JSC_DEFINE_HOST_FUNCTION(objectConstructorCreate, (JSGlobalObject* globalObject, CallFrame* callFrame))
   629 {
   630     VM& vm = globalObject->vm();
   631     auto scope = DECLARE_THROW_SCOPE(vm);
   632
   633     JSValue proto = callFrame->argument(0);
   634     if (!proto.isObject() && !proto.isNull())
   635         return throwVMTypeError(globalObject, scope, "Object prototype may only be an Object or null."_s);
   636     JSObject* newObject = proto.isObject()
   637         ? constructEmptyObject(globalObject, asObject(proto))
   638         : constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure());
   639     if (callFrame->argument(1).isUndefined())
   640         return JSValue::encode(newObject);
   641     JSObject* properties = callFrame->uncheckedArgument(1).toObject(globalObject);
   642     RETURN_IF_EXCEPTION(scope, { });
   643                                                                                                                                                                                  644     RELEASE_AND_RETURN(scope, JSValue::encode(defineProperties(globalObject, newObject, properties)));
   645 }
```

[Justin Michaud]

Comment on attachment 457197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457197&action=review

> Source/JavaScriptCore/runtime/StructureCache.cpp:-45
> -    PrototypeKey key { makePolyProtoStructure ? nullptr : prototype, executable, inlineCapacity, classInfo, globalObject };

Does this deserve an assert?

[Yusuke Suzuki]

Comment on attachment 457197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457197&action=review

>> Source/JavaScriptCore/runtime/StructureCache.cpp:-45
>> -    PrototypeKey key { makePolyProtoStructure ? nullptr : prototype, executable, inlineCapacity, classInfo, globalObject };
> 
> Does this deserve an assert?

Added.

[Yusuke Suzuki]

Created attachment 457277 [details]
Patch

[Saam Barati]

Comment on attachment 457277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457277&action=review

r=me

> Source/JavaScriptCore/ChangeLog:10
> +        JSGlobalObject, and (3) simplifies JSGlobalObject::haveABadTime handling.

for (3), you should say in the compiler.

[Yusuke Suzuki]

Comment on attachment 457277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457277&action=review

>> Source/JavaScriptCore/ChangeLog:10
>> +        JSGlobalObject, and (3) simplifies JSGlobalObject::haveABadTime handling.
> 
> for (3), you should say in the compiler.

Done.

[Yusuke Suzuki]

Committed r292795 (?): <https://commits.webkit.org/r292795>

[Radar WebKit Bug Importer]

<rdar://problem/91667038>