Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions Source/WTF/wtf/PageBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,14 @@ namespace WTF {
// On Linux, Power systems normally use 64 KiB pages.
//
// aarch64 systems seem to be all over the place. Most Linux distros use 4 KiB, but RHEL uses
// 64 KiB. Linux on Apple Silicon uses 16KiB for best performance, so use that for Linux on
// aarch64 by default. USE(64KB_PAGE_BLOCK) allows overriding this.
// 64 KiB. Since we cannot know the page size at compile time on Linux, use 64 KiB as the ceiling
// for Linux ARM64 to support all possible page size configurations (4 KiB, 16 KiB, 64 KiB).
// USE(64KB_PAGE_BLOCK) allows overriding this on other architectures too.
//
// Use 64 KiB for any unknown CPUs to be conservative.
#if OS(DARWIN) || PLATFORM(PLAYSTATION) || CPU(MIPS) || CPU(MIPS64) || CPU(LOONGARCH64) || (OS(LINUX) && CPU(ARM64) && !USE(64KB_PAGE_BLOCK))
#if OS(DARWIN) || PLATFORM(PLAYSTATION) || CPU(MIPS) || CPU(MIPS64) || CPU(LOONGARCH64)
constexpr size_t CeilingOnPageSize = 16 * KB;
#elif USE(64KB_PAGE_BLOCK) || CPU(PPC) || CPU(PPC64) || CPU(PPC64LE) || CPU(UNKNOWN)
#elif USE(64KB_PAGE_BLOCK) || (OS(LINUX) && CPU(ARM64)) || CPU(PPC) || CPU(PPC64) || CPU(PPC64LE) || CPU(UNKNOWN)
constexpr size_t CeilingOnPageSize = 64 * KB;
Comment on lines 49 to 58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

LGTM — this ceiling change belongs here.

It makes the compile-time upper bound safe for 64 KiB Linux ARM64 kernels without perturbing the other ARM64 ports. If CI does not already cover it, a Linux ARM64 / 64 KiB-page builder would be worth keeping on this path so future regressions fail before landing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Source/WTF/wtf/PageBlock.h` around lines 49 - 58, The change to set
CeilingOnPageSize to 64 * KB for the (USE(64KB_PAGE_BLOCK) || (OS(LINUX) &&
CPU(ARM64)) || CPU(PPC/ PPC64/ PPC64LE) || CPU(UNKNOWN)) branch is correct;
approve and merge as-is, and optionally add/ensure a Linux ARM64 build with 64
KiB pages in CI to catch regressions; look for the CeilingOnPageSize constexpr
in PageBlock.h to confirm the conditional and value are unchanged before
merging.

#elif OS(WINDOWS) || CPU(X86) || CPU(X86_64) || CPU(ARM) || CPU(ARM64) || CPU(RISCV64)
constexpr size_t CeilingOnPageSize = 4 * KB;
Expand Down
4 changes: 4 additions & 0 deletions Source/bmalloc/bmalloc/BPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,10 @@
#define BUSE_PRECOMPUTED_CONSTANTS_VMPAGE16K 1
#endif

#if !defined(BUSE_PRECOMPUTED_CONSTANTS_VMPAGE64K)
#define BUSE_PRECOMPUTED_CONSTANTS_VMPAGE64K 1
#endif

/* We only export the mallocSize and mallocGoodSize APIs if they're supported by the SystemHeap allocator (currently only Darwin) and the current bmalloc allocator (currently only libpas). */
#if BUSE(LIBPAS) && BOS(DARWIN)
#define BENABLE_MALLOC_SIZE 1
Expand Down
10 changes: 10 additions & 0 deletions Source/bmalloc/libpas/src/libpas/jit_heap_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,26 @@ PAS_BEGIN_EXTERN_C;
#define JIT_SMALL_SEGREGATED_MIN_ALIGN (1u << JIT_SMALL_SEGREGATED_MIN_ALIGN_SHIFT)
#define JIT_SMALL_BITFIT_MIN_ALIGN_SHIFT 2u
#define JIT_SMALL_BITFIT_MIN_ALIGN (1u << JIT_SMALL_BITFIT_MIN_ALIGN_SHIFT)
#if PAS_ARM64 && PAS_OS(LINUX)
#define JIT_SMALL_PAGE_SIZE 65536u
#define JIT_SMALL_GRANULE_SIZE 65536u
#else
#define JIT_SMALL_PAGE_SIZE 16384u
#define JIT_SMALL_GRANULE_SIZE 16384u
#endif
#define JIT_MEDIUM_BITFIT_MIN_ALIGN_SHIFT 8u
#define JIT_MEDIUM_BITFIT_MIN_ALIGN (1u << JIT_MEDIUM_BITFIT_MIN_ALIGN_SHIFT)
#if PAS_ARM64 && PAS_OS(LINUX)
#define JIT_MEDIUM_PAGE_SIZE 262144u
#define JIT_MEDIUM_GRANULE_SIZE 65536u
#else
#define JIT_MEDIUM_PAGE_SIZE 131072u
#if PAS_ARM64
#define JIT_MEDIUM_GRANULE_SIZE 16384u
#else
#define JIT_MEDIUM_GRANULE_SIZE 4096u
#endif
#endif

PAS_API void jit_heap_config_activate(void);

Expand Down
4 changes: 4 additions & 0 deletions Source/bmalloc/libpas/src/libpas/pas_expendable_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ struct pas_expendable_memory {
pas_expendable_memory_state states[1];
};

#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_EXPENDABLE_MEMORY_PAGE_SIZE 65536llu
#else
#define PAS_EXPENDABLE_MEMORY_PAGE_SIZE 16384llu
#endif

PAS_API extern pas_expendable_memory_state_version pas_expendable_memory_version_counter;

Expand Down
27 changes: 25 additions & 2 deletions Source/bmalloc/libpas/src/libpas/pas_internal_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,43 @@
#define PAS_USE_MARGE_BITFIT_OVERRIDE true

/* The OS may have a smaller page size. That's OK. */
/* On Linux ARM64, kernels may be configured with 64 KiB pages (e.g. RHEL, Oracle Linux).
All page and granule sizes must be at least as large as the system page size, so use
64 KiB minimums on Linux ARM64 to support all possible kernel page configurations. */
#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_SMALL_PAGE_DEFAULT_SHIFT 16
#else
#define PAS_SMALL_PAGE_DEFAULT_SHIFT 14
#endif
#define PAS_SMALL_PAGE_DEFAULT_SIZE ((size_t)1 << PAS_SMALL_PAGE_DEFAULT_SHIFT)

#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_SMALL_BITFIT_PAGE_DEFAULT_SHIFT 16
#else
#define PAS_SMALL_BITFIT_PAGE_DEFAULT_SHIFT 14
#endif
#define PAS_SMALL_BITFIT_PAGE_DEFAULT_SIZE ((size_t)1 << PAS_SMALL_BITFIT_PAGE_DEFAULT_SHIFT)

#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_MEDIUM_PAGE_DEFAULT_SHIFT 18
#else
#define PAS_MEDIUM_PAGE_DEFAULT_SHIFT 17
#endif
#define PAS_MEDIUM_PAGE_DEFAULT_SIZE ((size_t)1 << PAS_MEDIUM_PAGE_DEFAULT_SHIFT)

#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT 19
#else
#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT 19
#endif
Comment on lines +62 to +66
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Drop the no-op branch for PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT.

Both branches define 19, so this #if adds maintenance noise without changing behavior. Collapse it to a single define unless a Linux-specific value is still intended.

♻️ Suggested cleanup
-#if PAS_ARM64 && PAS_OS(LINUX)
-#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT    19
-#else
-#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT    19
-#endif
+#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT    19
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT 19
#else
#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT 19
#endif
`#define` PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT 19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Source/bmalloc/libpas/src/libpas/pas_internal_config.h` around lines 62 - 66,
The conditional around PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT is a no-op because
both branches set it to 19; remove the `#if/`#else/#endif and replace with a
single define for PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT 19, eliminating
references to PAS_ARM64 and PAS_OS(LINUX) in this block while preserving the
same macro name and value.

#define PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SIZE ((size_t)1 << PAS_MEDIUM_BITFIT_PAGE_DEFAULT_SHIFT)

#define PAS_MARGE_PAGE_DEFAULT_SHIFT 22
#define PAS_MARGE_PAGE_DEFAULT_SIZE ((size_t)1 << PAS_MARGE_PAGE_DEFAULT_SHIFT)

#if PAS_ARM64 || PAS_PLATFORM(PLAYSTATION)
#if PAS_ARM64 && PAS_OS(LINUX)
#define PAS_GRANULE_DEFAULT_SHIFT 16
#elif PAS_ARM64 || PAS_PLATFORM(PLAYSTATION)
#define PAS_GRANULE_DEFAULT_SHIFT 14
#else
#define PAS_GRANULE_DEFAULT_SHIFT 12
Expand Down Expand Up @@ -131,7 +152,9 @@

#define PAS_NUM_BASELINE_ALLOCATORS 32u

#define PAS_MAX_OBJECTS_PER_PAGE 2048
/* Must be >= max(page_size >> min_align_shift) across all segregated configs.
Utility heap uses PAS_INTERNAL_MIN_ALIGN_SHIFT (3) with the small page. */
#define PAS_MAX_OBJECTS_PER_PAGE (PAS_SMALL_PAGE_DEFAULT_SIZE >> PAS_INTERNAL_MIN_ALIGN_SHIFT)
Comment on lines +155 to +157
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Make PAS_MAX_OBJECTS_PER_PAGE self-checking.

This is correct for the current constants, but the bound now depends on the small-page config remaining the worst case. Please derive it from the actual max across the segregated configs, or add compile-time checks, so future page/alignment tweaks can't silently undersize it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Source/bmalloc/libpas/src/libpas/pas_internal_config.h` around lines 155 -
157, PAS_MAX_OBJECTS_PER_PAGE is currently defined from
PAS_SMALL_PAGE_DEFAULT_SIZE but needs to be self-checking against all segregated
small-page configs; modify the definition to compute the value from the actual
maximum small-page size across the segregated configs (use a compile-time MAX
macro over those PAS_..._SMALL_PAGE_DEFAULT_SIZE symbols) or add a compile-time
check (static_assert/BUILD_ASSERT) that PAS_MAX_OBJECTS_PER_PAGE >=
(MAX_SMALL_PAGE_DEFAULT_SIZE >> PAS_INTERNAL_MIN_ALIGN_SHIFT); update the
macro/compile-time check near PAS_MAX_OBJECTS_PER_PAGE and reference the
segregated config size symbols (the various PAS_*_SMALL_PAGE_DEFAULT_SIZE
macros) and PAS_INTERNAL_MIN_ALIGN_SHIFT so future changes cannot silently
undersize it.


#define PAS_MPROTECT_DECOMMITTED PAS_ENABLE_TESTING

Expand Down
Loading