Summary: | [JSC] Move StructureCache from VM to JSGlobalObject | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ews-watchlist, justin_michaud, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2022-04-10 03:31:36 PDT
Created attachment 457195 [details]
Patch
Created attachment 457197 [details]
Patch
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? 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 } ``` 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? 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. Created attachment 457277 [details]
Patch
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. 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. Committed r292795 (?): <https://commits.webkit.org/r292795> |