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

Description Mark Lam 2021-12-06 09:35:58 PST
This is another step building towards a global GC.
Comment 1 Radar WebKit Bug Importer 2021-12-06 09:36:43 PST
<rdar://problem/86108394>
Comment 2 Mark Lam 2021-12-06 09:38:59 PST
Created attachment 446046 [details]
work in progress.
Comment 3 Mark Lam 2021-12-06 10:33:09 PST
Created attachment 446051 [details]
work in progress.
Comment 4 Mark Lam 2021-12-06 10:57:25 PST
Created attachment 446054 [details]
work in progress.
Comment 5 Mark Lam 2021-12-06 16:17:19 PST
Created attachment 446092 [details]
work in progress.
Comment 6 Mark Lam 2022-02-15 18:01:02 PST
Created attachment 452120 [details]
work in progress.
Comment 7 Mark Lam 2022-02-17 18:07:06 PST
Created attachment 452461 [details]
proposed patch.
Comment 8 Yusuke Suzuki 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?
Comment 9 Mark Lam 2022-02-18 02:00:38 PST
Created attachment 452500 [details]
[fast-cq] patch for landing.
Comment 10 Mark Lam 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.
Comment 11 Mark Lam 2022-02-18 08:47:04 PST
Landed in r290129: <http://trac.webkit.org/r290129>.