Bug 233878

Summary: Split IsoSubspace into a GCClient allocator used by VM and a backend managed by Heap.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, annulen, beidson, cdumez, ews-watchlist, gyuyoung.kim, hi, joepeck, jsbell, keith_miller, msaboff, pangle, ryuan.choi, saam, sergio, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=236847
Bug Depends on:    
Bug Blocks: 232849    
Attachments:
Description Flags
work in progress.
ews-feeder: commit-queue-
work in progress.
ews-feeder: commit-queue-
work in progress.
ews-feeder: commit-queue-
work in progress.
none
work in progress.
none
proposed patch.
ysuzuki: review+
[fast-cq] patch for landing. none

Mark Lam
Reported 2021-12-06 09:35:58 PST
This is another step building towards a global GC.
Attachments
work in progress. (236.44 KB, patch)
2021-12-06 09:38 PST, Mark Lam
ews-feeder: commit-queue-
work in progress. (239.91 KB, patch)
2021-12-06 10:33 PST, Mark Lam
ews-feeder: commit-queue-
work in progress. (240.81 KB, patch)
2021-12-06 10:57 PST, Mark Lam
ews-feeder: commit-queue-
work in progress. (236.14 KB, patch)
2021-12-06 16:17 PST, Mark Lam
no flags
work in progress. (292.52 KB, patch)
2022-02-15 18:01 PST, Mark Lam
no flags
proposed patch. (801.66 KB, patch)
2022-02-17 18:07 PST, Mark Lam
ysuzuki: review+
[fast-cq] patch for landing. (825.21 KB, patch)
2022-02-18 02:00 PST, Mark Lam
no flags
Radar WebKit Bug Importer
Comment 1 2021-12-06 09:36:43 PST
Mark Lam
Comment 2 2021-12-06 09:38:59 PST
Created attachment 446046 [details] work in progress.
Mark Lam
Comment 3 2021-12-06 10:33:09 PST
Created attachment 446051 [details] work in progress.
Mark Lam
Comment 4 2021-12-06 10:57:25 PST
Created attachment 446054 [details] work in progress.
Mark Lam
Comment 5 2021-12-06 16:17:19 PST
Created attachment 446092 [details] work in progress.
Mark Lam
Comment 6 2022-02-15 18:01:02 PST
Created attachment 452120 [details] work in progress.
Mark Lam
Comment 7 2022-02-17 18:07:06 PST
Created attachment 452461 [details] proposed patch.
Yusuke Suzuki
Comment 8 2022-02-17 20:38:49 PST
Comment on attachment 452461 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=452461&action=review r=me > Source/JavaScriptCore/heap/Heap.cpp:3484 > +#undef DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER_SLOW Can we avoid repeating this list? I think we have the same kind of thing in VM, is it correct? > Source/JavaScriptCore/heap/Heap.h:1244 > +class Heap { > + WTF_MAKE_NONCOPYABLE(Heap); > +public: > + Heap(JSC::Heap&); > + ~Heap(); > + > + VM& vm() const; > + JSC::Heap& server() { return m_server; } > + > + // FIXME GlobalGC: need a GCClient::Heap::lastChanceToFinalize() and in there, > + // relinquish memory from the IsoSubspace LocalAllocators back to the server. > + // Currently, this is being handled by BlockDirectory::stopAllocatingForGood(). > + > +private: > + JSC::Heap& m_server; > + > + IsoSubspace arraySpace; > + IsoSubspace bigIntSpace; > + IsoSubspace calleeSpace; > + IsoSubspace clonedArgumentsSpace; > + IsoSubspace customGetterSetterSpace; > + IsoSubspace dateInstanceSpace; > + IsoSubspace domAttributeGetterSetterSpace; > + IsoSubspace exceptionSpace; > + IsoSubspace executableToCodeBlockEdgeSpace; > + IsoSubspace functionSpace; > + IsoSubspace getterSetterSpace; > + IsoSubspace globalLexicalEnvironmentSpace; > + IsoSubspace internalFunctionSpace; > + IsoSubspace jsProxySpace; > + IsoSubspace nativeExecutableSpace; > + IsoSubspace numberObjectSpace; > + IsoSubspace plainObjectSpace; > + IsoSubspace promiseSpace; > + IsoSubspace propertyNameEnumeratorSpace; > + IsoSubspace propertyTableSpace; > + IsoSubspace regExpSpace; > + IsoSubspace regExpObjectSpace; > + IsoSubspace ropeStringSpace; > + IsoSubspace scopedArgumentsSpace; > + IsoSubspace sparseArrayValueMapSpace; > + IsoSubspace stringSpace; > + IsoSubspace stringObjectSpace; > + IsoSubspace structureChainSpace; > + IsoSubspace structureRareDataSpace; > + IsoSubspace structureSpace; > + IsoSubspace brandedStructureSpace; > + IsoSubspace symbolTableSpace; > + > +#define DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(name) \ > + template<SubspaceAccess mode> \ > + IsoSubspace* name() \ > + { \ > + if (m_##name || mode == SubspaceAccess::Concurrently) \ > + return m_##name.get(); \ > + return name##Slow(); \ > + } \ > + JS_EXPORT_PRIVATE IsoSubspace* name##Slow(); \ > + std::unique_ptr<IsoSubspace> m_##name; > + > +#if JSC_OBJC_API_ENABLED > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(apiWrapperObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(objCCallbackFunctionSpace) > +#endif > +#ifdef JSC_GLIB_API_ENABLED > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(apiWrapperObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(jscCallbackFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(callbackAPIWrapperGlobalObjectSpace) > +#endif > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(apiGlobalObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(apiValueWrapperSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(arrayBufferSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(arrayIteratorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(asyncGeneratorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(bigInt64ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(bigIntObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(bigUint64ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(booleanObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(boundFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(callbackConstructorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(callbackGlobalObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(callbackFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(callbackObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(customGetterFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(customSetterFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(dataViewSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(debuggerScopeSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(errorInstanceSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(float32ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(float64ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(functionRareDataSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(generatorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(globalObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(injectedScriptHostSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(int8ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(int16ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(int32ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(javaScriptCallFrameSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(jsModuleRecordSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(mapBucketSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(mapIteratorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(mapSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(moduleNamespaceObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(nativeStdFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(proxyObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(proxyRevokeSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(remoteFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(scopedArgumentsTableSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(scriptFetchParametersSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(scriptFetcherSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(setBucketSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(setIteratorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(setSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(shadowRealmSpace); > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(strictEvalActivationSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(stringIteratorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(sourceCodeSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(symbolSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(symbolObjectSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(templateObjectDescriptorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(temporalCalendarSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(temporalDurationSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(temporalInstantSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(temporalPlainTimeSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(temporalTimeZoneSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(uint8ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(uint8ClampedArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(uint16ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(uint32ArraySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(unlinkedEvalCodeBlockSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(unlinkedFunctionCodeBlockSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(unlinkedModuleProgramCodeBlockSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(unlinkedProgramCodeBlockSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(finalizationRegistrySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(weakObjectRefSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(weakSetSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(weakMapSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(withScopeSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlCollatorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlDateTimeFormatSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlDisplayNamesSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlListFormatSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlLocaleSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlNumberFormatSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlPluralRulesSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlRelativeTimeFormatSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlSegmentIteratorSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlSegmenterSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(intlSegmentsSpace) > +#if ENABLE(WEBASSEMBLY) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(jsToWasmICCalleeSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyExceptionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyFunctionSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyGlobalSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyInstanceSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyMemorySpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyModuleSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyModuleRecordSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyTableSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyTagSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(webAssemblyWrapperFunctionSpace) > +#endif > + > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(evalExecutableSpace) > + DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER(moduleProgramExecutableSpace) > + > +#undef DYNAMIC_ISO_SUBSPACE_DEFINE_MEMBER > + > + IsoSubspace codeBlockSpace; > + IsoSubspace functionExecutableSpace; > + IsoSubspace programExecutableSpace; > + IsoSubspace unlinkedFunctionExecutableSpace; > + > + Vector<IsoSubspacePerVM*> perVMIsoSubspaces; > + > + friend class JSC::VM; > + friend class JSC::IsoSubspacePerVM; Can we avoid repeating this list? I think we have the same kind of thing in VM, is it correct?
Mark Lam
Comment 9 2022-02-18 02:00:38 PST
Created attachment 452500 [details] [fast-cq] patch for landing.
Mark Lam
Comment 10 2022-02-18 08:23:12 PST
Comment on attachment 452500 [details] [fast-cq] patch for landing. Thanks for the review. I've introduced macro lists of subspaces to remove all there redundancy the was previously needed. Now, to add a new subspace, we just add it to one of the macro lists. Landing now.
Mark Lam
Comment 11 2022-02-18 08:47:04 PST
Note You need to log in before you can comment on or make changes to this bug.