FfiCallbackMetadata::kPageSize should be target::kOSPageSize or similar? about sdk

[dcharkes]

Right, FfiCallbackMetadata::kPageSize

sdk/runtime/vm/ffi_callback_metadata.h

Lines 205 to 216 in bfe68f9

	#if defined(DART_TARGET_OS_LINUX) && defined(TARGET_ARCH_ARM64)
	static constexpr intptr_t kPageSize = 64 * KB;
	#elif defined(DART_TARGET_OS_ANDROID) && defined(TARGET_ARCH_IS_64_BIT)
	static constexpr intptr_t kPageSize = 64 * KB;
	#elif defined(DART_TARGET_OS_MACOS) && defined(TARGET_ARCH_ARM64)
	static constexpr intptr_t kPageSize = 16 * KB;
	#elif defined(DART_TARGET_OS_FUCHSIA)
	// Fuchsia only gets one page, so make it big.
	// TODO(https://dartbug.com/52579): Remove.
	static constexpr intptr_t kPageSize = 64 * KB;
	#else
	static constexpr intptr_t kPageSize = 4 * KB;

Could be reused for other things that require a memory block which is at least as large as a page on both host and target.
Moving it to a place where it is reuseable and renaming it sgtm if there's a use for that.

target::kPageSize also should be renamed to target::kHeapPageSize

@rmacnak-google is the one to say something about that.

Maybe @rmacnak-google can also suggest a good name if we move FfiCallbackMetadata::kPageSize to be reusable (as a page-size that is at least as large as host and target).

fyi @liamappelbe

[eseidel]

For now I've also just made all our classes just depend on FfiCallbackMetadata::kPageSize, which is fine. You're welcome to close this if there is no use upstream, thanks.

[rmacnak-google]

target::kPageSize also should be renamed to target::kHeapPageSize

No, kHeapPageSize refers to the size of a normal Dart heap chunk. Parts of the GC assume these chunks are also aligned to this value. This value is bigger than the OS page size. The overloading of the term "page" is unfortunate, but common.

[eseidel]

I think the real footgun here is that there is a "global" dart::kPageSize (everything is in namespace dart) and then various classes intentionally shadow that kPageSize and end up with confusing (or sometimes wrong) code if they get the wrong size not understanding the shadow.

In any case, we've worked around this by just using FfiCallbackMetadata::kPageSize with our own alias that is named uniquely from kPageSize so there is no risk of getting the wrong one.

We've solved this for us for now, so please feel free to close if not useful to the wider project.

[a-siva]

@rmacnak-google is there any actionable item here in terms of moving FfiCallbackMetadata::kPageSize and making a host and target version of it ?