Skip to content

Fix inheritance of type compactness in TZone type specification #47390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix inheritance of type compactness in TZone type specification #47390

wants to merge 1 commit into from
+3 −3

Conversation

ddegazio
Copy link
Contributor

@ddegazio ddegazio commented Jun 30, 2025 •
edited by webkit-early-warning-system
Loading

0c5b870

Fix inheritance of type compactness in TZone type specification
https://bugs.webkit.org/show_bug.cgi?id=295215
rdar://154691668

Reviewed by NOBODY (OOPS!).

Ensures that whenever bmalloc::api::requiresCompactPointers<T> is true,
the allocation mode of T's TZone type specification is Compact.

* Source/bmalloc/bmalloc/TZoneHeap.h:
* Source/bmalloc/bmalloc/TZoneHeapInlines.h:

0c5b870

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@ddegazio
Fix inheritance of type compactness in TZone type specification
0c5b870 
https://bugs.webkit.org/show_bug.cgi?id=295215
rdar://154691668

Reviewed by NOBODY (OOPS!).

Ensures that whenever bmalloc::api::requiresCompactPointers<T> is true,
the allocation mode of T's TZone type specification is Compact.

* Source/bmalloc/bmalloc/TZoneHeap.h:
* Source/bmalloc/bmalloc/TZoneHeapInlines.h:
@ddegazio ddegazio requested a review from a team as a code owner June 30, 2025 20:09
@ddegazio ddegazio self-assigned this Jun 30, 2025
@ddegazio ddegazio added the bmalloc For bugs in bmalloc label Jun 30, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on current version of this PR (hash 0c5b870)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

MenloDorian
MenloDorian approved these changes Jun 30, 2025
View reviewed changes
Copy link

@MenloDorian MenloDorian left a comment

Choose a reason for hiding this comment

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

r=me with suggested improvement.

Source/bmalloc/bmalloc/TZoneHeap.h
@@ -194,7 +194,7 @@ private: \
private: \
static _exportMacro BNO_INLINE void* operatorNewSlow(size_t size) \
{ \
static const TZoneSpecification s_heapSpec = { &s_heapRef, sizeof(_type), CompactAllocationMode:: _compactMode, SizeAndAlignment::encode<_type>() TZONE_SPEC_NAME_ARG(#_type) }; \
static const TZoneSpecification s_heapSpec = { &s_heapRef, sizeof(_type), ::bmalloc::api::requiresCompactPointers<_type>() ? CompactAllocationMode::Compact : CompactAllocationMode:: _compactMode, SizeAndAlignment::encode<_type>() TZONE_SPEC_NAME_ARG(#_type) }; \
Copy link

Choose a reason for hiding this comment

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

Would it be possible to create a constexpr template function compactModeForType that implements this ?: logic by taking a CompactAllocationMode argument. It can be a peer of requiresCompactPointers. Seems that would be better than having this logic in 3 places.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MenloDorian MenloDorian MenloDorian approved these changes

Assignees

@ddegazio ddegazio

Labels
bmalloc For bugs in bmalloc merging-blocked Applied to prevent a change from being merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ddegazio @webkit-early-warning-system @MenloDorian @webkit-ews-buildbot