WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
236564
[Linux/aarch64] Move page size ceiling to 16k
https://bugs.webkit.org/show_bug.cgi?id=236564
Summary
[Linux/aarch64] Move page size ceiling to 16k
Gustavo Noronha (kov)
Reported
2022-02-13 09:53:53 PST
[Linux/aarch64] Move page size ceiling to 16k
Attachments
Patch
(1.85 KB, patch)
2022-02-13 09:55 PST
,
Gustavo Noronha (kov)
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Gustavo Noronha (kov)
Comment 1
2022-02-13 09:55:51 PST
Created
attachment 451821
[details]
Patch
Gustavo Noronha (kov)
Comment 2
2022-02-13 09:57:39 PST
No visible impact on the binary size: kov@uva ~/P/WebKit (16k)> ls -l WebKitBuild/Release-*/lib/*.so.*.*.* -rwxr-xr-x. 1 kov kov 27263544 fev 13 13:55 WebKitBuild/Release-16k/lib/libjavascriptcoregtk-4.0.so.18.20.2 -rwxr-xr-x. 1 kov kov 98987608 fev 13 14:15 WebKitBuild/Release-16k/lib/libwebkit2gtk-4.0.so.37.56.2 -rwxr-xr-x. 1 kov kov 27263544 fev 13 14:17 WebKitBuild/Release-4k/lib/libjavascriptcoregtk-4.0.so.18.20.2 -rwxr-xr-x. 1 kov kov 98987608 fev 13 14:23 WebKitBuild/Release-4k/lib/libwebkit2gtk-4.0.so.37.56.2 I did check that the files are actually different - the 4k one crashes on my 16k page size Linux on M1.
Yusuke Suzuki
Comment 3
2022-02-14 09:21:40 PST
Comment on
attachment 451821
[details]
Patch r=me
EWS
Comment 4
2022-02-14 12:27:35 PST
gns@gnome.org
does not have committer permissions according to
https://raw.githubusercontent.com/WebKit/WebKit/main/metadata/contributors.json
. Rejecting
attachment 451821
[details]
from commit queue.
EWS
Comment 5
2022-02-14 13:14:06 PST
Committed
r289761
(
247233@main
): <
https://commits.webkit.org/247233@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 451821
[details]
.
Radar WebKit Bug Importer
Comment 6
2022-02-14 13:15:37 PST
<
rdar://problem/88924594
>
Michael Catanzaro
Comment 7
2022-02-15 07:46:16 PST
This might have performance implications for embedded systems, not sure.
Zan Dobersek
Comment 8
2022-02-15 22:19:34 PST
Ideally the page size would be detected and used at runtime. This isn't possible since it's used at build-time in different places. Last time round, the increase was to 64kB, which was causing crashes in general but it also (most prominently) increased the size of each MarkedBlock in JSC, meaning in certain situation the JS memory usage would have increased. See
bug #209670
. This change does the increase to 16kB which matches minimum MarkedBlock size, so there's no change there. But in general, if it were possible to detect at build-time whether the build is intended for Apple silicon due to some specific tune option in GCC/Clang, I only enabling 16kB ceiling for ARM64 under that condition would be preferred.
Gustavo Noronha (kov)
Comment 9
2022-02-19 10:26:08 PST
Only enabling it if building specifically for Apple Silicon would mean distributions would need to have a port specifically for that to support the 16k page size kernel, which would be a shame. I expect it will be more common to have custom builds for specific embedded devices than M1 as Asahi gains features and stabilizes (it probably already is right now with the exception of maybe Raspberry Pi?). Is there a particular platform you are worried about? Maybe we can run some performance measurements for that and make sure it isn't really a concern. All that said, since we are not increasing binary size and are matching the minimum MarkedBlock size we should not really have any issues, be it functional or performance, right? I keep wondering if we should not just increase all of Linux to a 16k ceiling as a minimum tbh, match what Apple did as it will probably be the best supported configuration in the long run.
Gustavo Noronha (kov)
Comment 10
2022-02-19 10:52:55 PST
I was also doing some reading on bmalloc's replacement libpas and it also seems to use 16k as the minimum page size it cares about (unless it is configured to use 64k pages), so I guess unless the system malloc is used (which would probably be a big perf hit) we are already pretty much at a 16k minimum right now (which is what keeps me going back to maybe we should just push all of Linux up and future-proof potentially moving the other arches to bigger page sizes as well). kov@morango ~/P/WebKit (16k)> git grep -e '.small_segregated_page_size =' Source/ Source/bmalloc/libpas/Documentation.md: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ Source/bmalloc/libpas/src/libpas/bmalloc_heap_config.h: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ Source/bmalloc/libpas/src/libpas/hotbit_heap_config.h: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ Source/bmalloc/libpas/src/libpas/iso_heap_config.h: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ Source/bmalloc/libpas/src/libpas/iso_test_heap_config.h: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ Source/bmalloc/libpas/src/libpas/minalign32_heap_config.h: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ Source/bmalloc/libpas/src/libpas/pagesize64k_heap_config.h: .small_segregated_page_size = PAGESIZE64K_SMALL_PAGE_SIZE, \ Source/bmalloc/libpas/src/libpas/thingy_heap_config.h: .small_segregated_page_size = PAS_SMALL_PAGE_DEFAULT_SIZE, \ kov@morango ~/P/WebKit (16k)> git grep 'define PAS_SMALL_PAGE_DEFAULT_SIZE' Source/ Source/bmalloc/libpas/src/libpas/pas_internal_config.h:#define PAS_SMALL_PAGE_DEFAULT_SIZE ((size_t)1 << PAS_SMALL_PAGE_DEFAULT_SHIFT) kov@morango ~/P/WebKit (16k)> git grep 'define PAS_SMALL_PAGE_DEFAULT_SHIFT' Source/ Source/bmalloc/libpas/src/libpas/pas_internal_config.h:#define PAS_SMALL_PAGE_DEFAULT_SHIFT 14 kov@morango ~/P/WebKit (16k)>
Zan Dobersek
Comment 11
2022-02-20 23:20:58 PST
These changes would be easier on the mind if there was data provided in parallel on how memory consumption behaves on different benchmarks. But that's about page sizes in general. This specific change doesn't really affect that. It doesn't affect much in general.
Zan Dobersek
Comment 12
2022-02-20 23:31:15 PST
Concretely, small-page size in vanilla bmalloc is still 4kB. If you're not using libpas, you should be hitting assertions in bmalloc::HeapConstants::HeapConstants() on Asahi. I don't know the state of libpas on ARMv7, I would have to check with others. But yeah, quadrupled small-page size on those platforms is possibly not great.
Gustavo Noronha (kov)
Comment 13
2022-02-21 03:53:13 PST
I'd argue that even if it does affect armv7* it probably makes more sense to add tweaks / build time detection for them and keep 16k as the default, as that is what will make more sense for most people using regular distributions rather than custom-building for their devices, especially going forward. It is quite easy for us to even carry a patch for that kind of thing on our custom images for these boards. * I think you're mainly concerned about the likes of imx6, raspberry pi 3 and other boards of that age? I'll see if I can get my imx6 sabrelite up and running to do some testing.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug