Bug 230124

Summary: [libpas] Update to 38b9b0b92ccc9628627c742523de6200acc08211 and fully enable on AS
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: bmallocAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, benjamin, cdumez, cmarcelo, ews-watchlist, ggaren, jsbell, tsavell, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 230362    
Attachments:
Description Flags
patch for landing, maybe
none
better changelog
none
added RAMification number to changelog
ysuzuki: review+, ews-feeder: commit-queue-
patch for landing? ews-feeder: commit-queue-

Description Filip Pizlo 2021-09-09 14:06:33 PDT
...
Comment 1 Filip Pizlo 2021-09-09 14:16:00 PDT
Created attachment 437783 [details]
patch for landing, maybe
Comment 2 Filip Pizlo 2021-09-09 14:29:37 PDT
Created attachment 437786 [details]
better changelog
Comment 3 Filip Pizlo 2021-09-09 15:20:56 PDT
Created attachment 437793 [details]
added RAMification number to changelog
Comment 4 Yusuke Suzuki 2021-09-09 20:29:42 PDT
Comment on attachment 437793 [details]
added RAMification number to changelog

View in context: https://bugs.webkit.org/attachment.cgi?id=437793&action=review

r=me

> Source/bmalloc/ChangeLog:25
> +        - Replace biasing and magazines with per-thread view caches. A view cache is a bounded-size
> +          queue of exclusive_views (i.e. pages) that is local to a thread. Each thread has a view
> +          cache for every segregated_size_directory that it talks to and that has view caching
> +          enabled. The size directories can control view cache size somewhat dynamically (different
> +          directories can have different size view caches). Views get enqueued when a thread frees
> +          the first object in the page. Views get dequeued whenever a thread would have asked the
> +          directory for a view but the view cache was non-empty.
> +
> +          This change increases the efficiency of local_allocator_refill, since pages have a longer
> +          time to "cook" before anyone allocates from them (since the view cache is a queue). As
> +          well, view caches' thread-locality means that there is no locking or contention when
> +          accessing them. The scavenger has clever tricks for clearing out view caches, similarly to
> +          how it clears out local allocators (it uses thread_suspend).

Yeah, I tried using magazine in bmalloc, and the result was, thread-local one was better (lock is not necessary, cpu-number can change relatively easily etc.).

> Source/bmalloc/bmalloc/BPlatform.h:323
> +#if defined(BENABLE_LIBPAS) && BENABLE_LIBPAS

We can use `if BENABLE(LIBPAS)`.
Comment 5 Filip Pizlo 2021-09-15 18:39:25 PDT
Created attachment 438314 [details]
patch for landing?
Comment 7 Filip Pizlo 2021-09-16 06:50:13 PDT
Landed in https://trac.webkit.org/changeset/282556/webkit
Comment 8 Radar WebKit Bug Importer 2021-09-16 06:54:30 PDT
<rdar://problem/83195529>
Comment 9 Truitt Savell 2021-09-16 10:06:52 PDT
It looks like the changes in nhttps://trac.webkit.org/changeset/282556/webkit

broke multiple tests on arm64 JSC testers:

https://build.webkit.org/#/builders/166/builds/2069
https://build.webkit.org/#/builders/102/builds/3448
Comment 10 Filip Pizlo 2021-09-16 11:05:36 PDT
(In reply to Truitt Savell from comment #9)
> It looks like the changes in nhttps://trac.webkit.org/changeset/282556/webkit
> 
> broke multiple tests on arm64 JSC testers:
> 
> https://build.webkit.org/#/builders/166/builds/2069
> https://build.webkit.org/#/builders/102/builds/3448

Fix: https://bugs.webkit.org/show_bug.cgi?id=230362