233878 – Split IsoSubspace into a GCClient allocator used by VM and a backend managed by Heap.

Bug 233878 - Split IsoSubspace into a GCClient allocator used by VM and a backend managed by Heap.

Summary: Split IsoSubspace into a GCClient allocator used by VM and a backend managed ...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Mark Lam

URL:
Keywords:	InRadar

Depends on:
Blocks:	232849
	Show dependency tree / graph

Reported:	2021-12-06 09:35 PST by Mark Lam
Modified:	2022-02-18 10:51 PST (History)
CC List:	18 users (show)

See Also:	236847

Attachments
work in progress. (236.44 KB, patch) 2021-12-06 09:38 PST, Mark Lam	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
work in progress. (239.91 KB, patch) 2021-12-06 10:33 PST, Mark Lam	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
work in progress. (240.81 KB, patch) 2021-12-06 10:57 PST, Mark Lam	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
work in progress. (236.14 KB, patch) 2021-12-06 16:17 PST, Mark Lam	no flags	Details \| Formatted Diff \| Diff
work in progress. (292.52 KB, patch) 2022-02-15 18:01 PST, Mark Lam	no flags	Details \| Formatted Diff \| Diff
proposed patch. (801.66 KB, patch) 2022-02-17 18:07 PST, Mark Lam	ysuzuki: review+	Details \| Formatted Diff \| Diff
[fast-cq] patch for landing. (825.21 KB, patch) 2022-02-18 02:00 PST, Mark Lam	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

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>.