Bug 184176

Summary: bmalloc should do partial scavenges more frequently
Product: WebKit Reporter: Saam Barati <saam>
Component: bmallocAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, rniwa, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 184442    
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
WIP
none
WIP
ews-watchlist: commit-queue-
patch
none
patch
none
patch
fpizlo: review+
Archive of layout-test-results from ews100 for mac-sierra
none
patch for landing
none
patch for landing none

Saam Barati
Reported 2018-03-29 23:09:46 PDT
...
Attachments
WIP (9.83 KB, patch)
2018-04-02 20:02 PDT, Saam Barati
no flags
WIP (25.00 KB, patch)
2018-04-03 20:23 PDT, Saam Barati
no flags
WIP (29.44 KB, patch)
2018-04-04 13:42 PDT, Saam Barati
no flags
WIP (47.34 KB, patch)
2018-04-04 19:27 PDT, Saam Barati
no flags
WIP (40.16 KB, patch)
2018-04-04 23:07 PDT, Saam Barati
ews-watchlist: commit-queue-
patch (58.58 KB, patch)
2018-04-05 16:43 PDT, Saam Barati
no flags
patch (57.77 KB, patch)
2018-04-05 16:57 PDT, Saam Barati
no flags
patch (61.36 KB, patch)
2018-04-09 23:02 PDT, Saam Barati
fpizlo: review+
Archive of layout-test-results from ews100 for mac-sierra (2.21 MB, application/zip)
2018-04-10 00:09 PDT, EWS Watchlist
no flags
patch for landing (61.35 KB, patch)
2018-04-10 14:02 PDT, Saam Barati
no flags
patch for landing (66.04 KB, patch)
2018-04-10 14:08 PDT, Saam Barati
no flags
Saam Barati
Comment 1 2018-04-02 20:02:43 PDT
Saam Barati
Comment 2 2018-04-03 20:23:26 PDT
Created attachment 337141 [details] WIP I'm playing around with the heuristic of when to drive the scavenger. I'm tuning against JetStream on a MBP. It's tricky to do anything that isn't a performance regression but a memory progression since ToT essentially never runs the scavenger in ToT.
Saam Barati
Comment 3 2018-04-04 13:42:05 PDT
Created attachment 337220 [details] WIP Added some preliminary support for calling madvise while not holding the global heap lock.
Saam Barati
Comment 4 2018-04-04 19:27:58 PDT
Created attachment 337252 [details] WIP Trying out more stuff. Giving Fil's idea of keeping track of high water mark and scavenging anything above that point a go.
Saam Barati
Comment 5 2018-04-04 22:27:49 PDT
(In reply to Saam Barati from comment #4) > Created attachment 337252 [details] > WIP > > Trying out more stuff. Giving Fil's idea of keeping track of high water mark > and scavenging anything above that point a go. High water mark is the way to go.
Saam Barati
Comment 6 2018-04-04 23:07:33 PDT
EWS Watchlist
Comment 7 2018-04-04 23:10:24 PDT
Attachment 337256 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:39: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Heap.cpp:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Heap.cpp:624: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 4 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 8 2018-04-05 00:23:41 PDT
Comment on attachment 337256 [details] WIP Attachment 337256 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/7212687 New failing tests: jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-2.js.layout-no-llint jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-2.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-2.js.layout-ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-2.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-2.js.layout
Saam Barati
Comment 9 2018-04-05 16:43:48 PDT
Saam Barati
Comment 10 2018-04-05 16:57:15 PDT
EWS Watchlist
Comment 11 2018-04-05 16:59:07 PDT
Attachment 337312 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Heap.cpp:41: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Heap.cpp:570: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:40: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 4 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 12 2018-04-09 23:02:45 PDT
Created attachment 337594 [details] patch This patch addresses some feedback Fil gave me in person. IsoHeapImpl now tracks its directory high water mark and scavenges that directory and any directory with a larger index than it.
EWS Watchlist
Comment 13 2018-04-09 23:04:40 PDT
Attachment 337594 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Heap.cpp:570: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 14 2018-04-10 00:09:26 PDT
Comment on attachment 337594 [details] patch Attachment 337594 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7264889 New failing tests: imported/w3c/web-platform-tests/workers/name-property.html
EWS Watchlist
Comment 15 2018-04-10 00:09:28 PDT
Created attachment 337596 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Saam Barati
Comment 16 2018-04-10 09:02:04 PDT
(In reply to Build Bot from comment #14) > Comment on attachment 337594 [details] > patch > > Attachment 337594 [details] did not pass mac-ews (mac): > Output: http://webkit-queues.webkit.org/results/7264889 > > New failing tests: > imported/w3c/web-platform-tests/workers/name-property.html This seems like a flaky or broken test.
Saam Barati
Comment 17 2018-04-10 14:02:56 PDT
Created attachment 337633 [details] patch for landing
EWS Watchlist
Comment 18 2018-04-10 14:06:04 PDT
Attachment 337633 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Heap.cpp:570: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 19 2018-04-10 14:08:16 PDT
Created attachment 337634 [details] patch for landing
EWS Watchlist
Comment 20 2018-04-10 14:11:25 PDT
Attachment 337634 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Heap.cpp:570: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Commit Bot
Comment 21 2018-04-10 16:34:48 PDT
Comment on attachment 337634 [details] patch for landing Clearing flags on attachment: 337634 Committed r230501: <https://trac.webkit.org/changeset/230501>
WebKit Commit Bot
Comment 22 2018-04-10 16:34:50 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 23 2018-04-10 16:35:28 PDT
Note You need to log in before you can comment on or make changes to this bug.