LineMetadata is dependent only to vm page size. Precalculate them and store it in constexpr array makes benefits as it skips run time calculation and also page allocation.
Created attachment 387296 [details] FIRST TRIAL
size of LineMetadata array is: sizeof(LineMetadata) * sizeClass(smallLineSize) * smallLineCount(VMPageSize) = 2 * maskSizeClass(256) * vmPageSize / 256 = 62 * (vmPageSize / 256) VMPageSize|size of LineMetadata array ----------+--------------- 4k | 62 * 16 = 992 16k | 62 * 64 = 3968
I'm curious about VM page size for watchOS and tvOS.
Created attachment 387351 [details] PATCH
Comment on attachment 387351 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=387351&action=review r=me so long as we have fallback mechanism which has dynamic calculation b/c some weird CPU has some weird page size (PowerPC has 64KB). But I also want to have some comments from Geoff about page size assumption. If we can dynamically change the page size (maybe we don't especially in macOS / iOS), we should use dynamic calculation version. > Source/bmalloc/bmalloc/BPlatform.h:270 > +#if BPLATFORM(iOS) || BPLATFORM(PLAYSTATION) Typo, BPLATFORM(iOS) => BPLATFORM(IOS). BTW, I'm not sure whether all iOS uses 16KB pages. @Geoff, do we use 16KB for all iOS (not including watchOS, tvOS and Catalyst)? For macOS, I think 4KB is OK. Hugepage exists, but from the point of application, anyway unit of page size is 4KB.
Comment on attachment 387351 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=387351&action=review Looks great to me -- but perhaps we should remove the #ifdefs. >> Source/bmalloc/bmalloc/BPlatform.h:270 >> +#if BPLATFORM(iOS) || BPLATFORM(PLAYSTATION) > > Typo, BPLATFORM(iOS) => BPLATFORM(IOS). BTW, I'm not sure whether all iOS uses 16KB pages. > @Geoff, do we use 16KB for all iOS (not including watchOS, tvOS and Catalyst)? > > For macOS, I think 4KB is OK. Hugepage exists, but from the point of application, anyway unit of page size is 4KB. I believe some iOS variants still use 4kB pages. (Certainly they have used 4kB pages in the past.) > Source/bmalloc/bmalloc/BPlatform.h:271 > +#define BUSE_PRECOMPUTED_CONSTANTS_VMPAGE16K 1 Since some Apple platforms vary page size at boot time or runtime rather than compile time, I think I'd prefer it if all Apple ports compiled in these values, and then selected the right variant to use at runtime. And perhaps that's the best strategy for all platforms -- unless the compiled size is prohibitive?
(In reply to Geoffrey Garen from comment #6) > Comment on attachment 387351 [details] > PATCH > > View in context: > https://bugs.webkit.org/attachment.cgi?id=387351&action=review > > Looks great to me -- but perhaps we should remove the #ifdefs. > > >> Source/bmalloc/bmalloc/BPlatform.h:270 > >> +#if BPLATFORM(iOS) || BPLATFORM(PLAYSTATION) > > > > Typo, BPLATFORM(iOS) => BPLATFORM(IOS). BTW, I'm not sure whether all iOS uses 16KB pages. > > @Geoff, do we use 16KB for all iOS (not including watchOS, tvOS and Catalyst)? > > > > For macOS, I think 4KB is OK. Hugepage exists, but from the point of application, anyway unit of page size is 4KB. > > I believe some iOS variants still use 4kB pages. (Certainly they have used > 4kB pages in the past.) > > > Source/bmalloc/bmalloc/BPlatform.h:271 > > +#define BUSE_PRECOMPUTED_CONSTANTS_VMPAGE16K 1 > > Since some Apple platforms vary page size at boot time or runtime rather > than compile time, I think I'd prefer it if all Apple ports compiled in > these values, and then selected the right variant to use at runtime. And > perhaps that's the best strategy for all platforms -- unless the compiled > size is prohibitive? Thanks for the review. Okay, I'll stop defining those macro in BPlatform.h, but define them explicitly in outside code. We want our code size minimum, I will add #if !defined() macro definition to make default precalculated for all page sizes and opt-out by defining them as zero explicitly.
Created attachment 387679 [details] PATCH
Created attachment 387683 [details] PATCH
Comment on attachment 387683 [details] PATCH Clearing flags on attachment: 387683 Committed r254527: <https://trac.webkit.org/changeset/254527>
All reviewed patches have been landed. Closing bug.
<rdar://problem/58577961>