WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
184121
bmalloc should compute its own estimate of its footprint
https://bugs.webkit.org/show_bug.cgi?id=184121
Summary
bmalloc should compute its own estimate of its footprint
Saam Barati
Reported
2018-03-28 18:18:50 PDT
...
Attachments
WIP
(22.38 KB, patch)
2018-03-28 18:24 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
WIP
(22.46 KB, patch)
2018-03-28 19:26 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
WIP
(32.06 KB, patch)
2018-03-29 15:54 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
WIP
(26.64 KB, patch)
2018-03-29 18:04 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
WIP
(32.64 KB, patch)
2018-03-30 00:37 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
WIP
(39.13 KB, patch)
2018-03-30 21:58 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
patch
(47.23 KB, patch)
2018-03-30 23:15 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
patch
(47.43 KB, patch)
2018-03-30 23:36 PDT
,
Saam Barati
ews-watchlist
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ews206 for win-future
(12.16 MB, application/zip)
2018-03-31 02:14 PDT
,
EWS Watchlist
no flags
Details
patch
(47.43 KB, patch)
2018-04-02 11:41 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Saam Barati
Comment 1
2018-03-28 18:24:22 PDT
Created
attachment 336742
[details]
WIP This is just a stupid implementation that walks a bunch of data structures each time. Using this to do some testing. We should probably not make it walk everything every time we ask for footprint...
Saam Barati
Comment 2
2018-03-28 19:26:29 PDT
Created
attachment 336747
[details]
WIP This time without the deadlock
Saam Barati
Comment 3
2018-03-29 15:54:25 PDT
Created
attachment 336812
[details]
WIP I think I have the accounting down to do it in place now.
Saam Barati
Comment 4
2018-03-29 15:55:39 PDT
(In reply to Saam Barati from
comment #3
)
> Created
attachment 336812
[details]
> WIP > > I think I have the accounting down to do it in place now.
I had to make LargeRange remember how many total physical bytes it represents, even if that may not account for the physical bytes at the beginning of the range.
Saam Barati
Comment 5
2018-03-29 18:04:17 PDT
Created
attachment 336826
[details]
WIP
Saam Barati
Comment 6
2018-03-30 00:37:51 PDT
Created
attachment 336846
[details]
WIP Close to done. Just need to figure out concurrency in one place w.r.t IsoHeaps.
Saam Barati
Comment 7
2018-03-30 21:58:51 PDT
Created
attachment 336909
[details]
WIP Now with bmalloc API to commit/decommit.
Saam Barati
Comment 8
2018-03-30 23:15:47 PDT
Created
attachment 336916
[details]
patch I still need to do some performance testing on iOS. The footprint measurement is accurate, but unlike phys_footprint, it doesn't account for compressed memory on iOS. This means I may need to tune some of our isUnderMemoryPressure heuristics to account for the change in measurement.
EWS Watchlist
Comment 9
2018-03-30 23:17:23 PDT
Attachment 336916
[details]
did not pass style-queue: ERROR: Source/bmalloc/bmalloc/bmalloc.h:100: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/bmalloc.h:101: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 10
2018-03-30 23:36:46 PDT
Created
attachment 336917
[details]
patch Try to fix some of the builds
EWS Watchlist
Comment 11
2018-03-30 23:38:38 PDT
Attachment 336917
[details]
did not pass style-queue: ERROR: Source/bmalloc/bmalloc/bmalloc.h:100: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/bmalloc.h:101: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 12
2018-03-31 02:14:10 PDT
Comment on
attachment 336917
[details]
patch
Attachment 336917
[details]
did not pass win-ews (win): Output:
http://webkit-queues.webkit.org/results/7156164
New failing tests: http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html
EWS Watchlist
Comment 13
2018-03-31 02:14:21 PDT
Created
attachment 336922
[details]
Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Saam Barati
Comment 14
2018-04-02 11:40:29 PDT
(In reply to Saam Barati from
comment #8
)
> Created
attachment 336916
[details]
> patch > > I still need to do some performance testing on iOS. The footprint > measurement is accurate, but unlike phys_footprint, it doesn't account for > compressed memory on iOS. This means I may need to tune some of our > isUnderMemoryPressure heuristics to account for the change in measurement.
Perf seemed fine on JetStream. I'll let the bots handle testing many different devices.
Saam Barati
Comment 15
2018-04-02 11:41:32 PDT
Created
attachment 337008
[details]
patch test EWS again.
EWS Watchlist
Comment 16
2018-04-02 11:45:11 PDT
Attachment 337008
[details]
did not pass style-queue: ERROR: Source/bmalloc/bmalloc/bmalloc.h:100: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/bmalloc.h:101: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Commit Bot
Comment 17
2018-04-02 14:09:51 PDT
Comment on
attachment 337008
[details]
patch Clearing flags on attachment: 337008 Committed
r230187
: <
https://trac.webkit.org/changeset/230187
>
WebKit Commit Bot
Comment 18
2018-04-02 14:09:53 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 19
2018-04-02 14:11:52 PDT
<
rdar://problem/39118176
>
Ryan Haddad
Comment 20
2018-04-03 08:56:07 PDT
This change seems to have caused flaky crashes on macOS Debug WK1 bots: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x0000000108481175 bmalloc::LargeRange::LargeRange(void*, unsigned long, unsigned long, unsigned long) + 133 (LargeRange.h:57) 1 com.apple.JavaScriptCore 0x000000010847d115 bmalloc::LargeRange::LargeRange(void*, unsigned long, unsigned long, unsigned long) + 53 (LargeRange.h:59) 2 com.apple.JavaScriptCore 0x000000010848049a bmalloc::LargeRange::split(unsigned long) const + 1226 (LargeRange.h:138) 3 com.apple.JavaScriptCore 0x000000010847fc97 bmalloc::Heap::splitAndAllocate(std::__1::lock_guard<bmalloc::StaticMutex>&, bmalloc::LargeRange&, unsigned long, unsigned long) + 407 (Heap.cpp:479) 4 com.apple.JavaScriptCore 0x000000010848099a bmalloc::Heap::tryAllocateLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long, unsigned long) + 698 (Heap.cpp:541) 5 com.apple.JavaScriptCore 0x0000000108480a3d bmalloc::Heap::allocateLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long, unsigned long) + 45 (Heap.cpp:546) 6 com.apple.JavaScriptCore 0x000000010847535b bmalloc::Allocator::allocateLarge(unsigned long) + 107 (Allocator.cpp:172) 7 com.apple.JavaScriptCore 0x000000010847566a bmalloc::Allocator::allocateSlowCase(unsigned long) + 362 (Allocator.cpp:199) 8 com.apple.JavaScriptCore 0x00000001083e1e68 bmalloc::Allocator::allocate(unsigned long) + 72 (Allocator.h:91) 9 com.apple.JavaScriptCore 0x00000001083e1db6 bmalloc::Cache::allocate(bmalloc::HeapKind, unsigned long) + 166 (Cache.h:79) 10 com.apple.JavaScriptCore 0x00000001083e157b bmalloc::api::malloc(unsigned long, bmalloc::HeapKind) + 27 (bmalloc.h:49) Full crash log here:
https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r230190%20(7008)/results.html
Saam Barati
Comment 21
2018-04-03 10:06:38 PDT
(In reply to Ryan Haddad from
comment #20
)
> This change seems to have caused flaky crashes on macOS Debug WK1 bots: > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 com.apple.JavaScriptCore 0x0000000108481175 > bmalloc::LargeRange::LargeRange(void*, unsigned long, unsigned long, > unsigned long) + 133 (LargeRange.h:57) > 1 com.apple.JavaScriptCore 0x000000010847d115 > bmalloc::LargeRange::LargeRange(void*, unsigned long, unsigned long, > unsigned long) + 53 (LargeRange.h:59) > 2 com.apple.JavaScriptCore 0x000000010848049a > bmalloc::LargeRange::split(unsigned long) const + 1226 (LargeRange.h:138) > 3 com.apple.JavaScriptCore 0x000000010847fc97 > bmalloc::Heap::splitAndAllocate(std::__1::lock_guard<bmalloc::StaticMutex>&, > bmalloc::LargeRange&, unsigned long, unsigned long) + 407 (Heap.cpp:479) > 4 com.apple.JavaScriptCore 0x000000010848099a > bmalloc::Heap::tryAllocateLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, > unsigned long, unsigned long) + 698 (Heap.cpp:541) > 5 com.apple.JavaScriptCore 0x0000000108480a3d > bmalloc::Heap::allocateLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, > unsigned long, unsigned long) + 45 (Heap.cpp:546) > 6 com.apple.JavaScriptCore 0x000000010847535b > bmalloc::Allocator::allocateLarge(unsigned long) + 107 (Allocator.cpp:172) > 7 com.apple.JavaScriptCore 0x000000010847566a > bmalloc::Allocator::allocateSlowCase(unsigned long) + 362 (Allocator.cpp:199) > 8 com.apple.JavaScriptCore 0x00000001083e1e68 > bmalloc::Allocator::allocate(unsigned long) + 72 (Allocator.h:91) > 9 com.apple.JavaScriptCore 0x00000001083e1db6 > bmalloc::Cache::allocate(bmalloc::HeapKind, unsigned long) + 166 (Cache.h:79) > 10 com.apple.JavaScriptCore 0x00000001083e157b > bmalloc::api::malloc(unsigned long, bmalloc::HeapKind) + 27 (bmalloc.h:49) > > > Full crash log here: >
https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/
>
r230190
%20(7008)/results.html
Interesting. I'll look into this. These asserts I added should always hold, so the bug is probably elsewhere if they don't hold.
Saam Barati
Comment 22
2018-04-03 11:00:07 PDT
(In reply to Saam Barati from
comment #21
)
> (In reply to Ryan Haddad from
comment #20
) > > This change seems to have caused flaky crashes on macOS Debug WK1 bots: > > > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > > 0 com.apple.JavaScriptCore 0x0000000108481175 > > bmalloc::LargeRange::LargeRange(void*, unsigned long, unsigned long, > > unsigned long) + 133 (LargeRange.h:57) > > 1 com.apple.JavaScriptCore 0x000000010847d115 > > bmalloc::LargeRange::LargeRange(void*, unsigned long, unsigned long, > > unsigned long) + 53 (LargeRange.h:59) > > 2 com.apple.JavaScriptCore 0x000000010848049a > > bmalloc::LargeRange::split(unsigned long) const + 1226 (LargeRange.h:138) > > 3 com.apple.JavaScriptCore 0x000000010847fc97 > > bmalloc::Heap::splitAndAllocate(std::__1::lock_guard<bmalloc::StaticMutex>&, > > bmalloc::LargeRange&, unsigned long, unsigned long) + 407 (Heap.cpp:479) > > 4 com.apple.JavaScriptCore 0x000000010848099a > > bmalloc::Heap::tryAllocateLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, > > unsigned long, unsigned long) + 698 (Heap.cpp:541) > > 5 com.apple.JavaScriptCore 0x0000000108480a3d > > bmalloc::Heap::allocateLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, > > unsigned long, unsigned long) + 45 (Heap.cpp:546) > > 6 com.apple.JavaScriptCore 0x000000010847535b > > bmalloc::Allocator::allocateLarge(unsigned long) + 107 (Allocator.cpp:172) > > 7 com.apple.JavaScriptCore 0x000000010847566a > > bmalloc::Allocator::allocateSlowCase(unsigned long) + 362 (Allocator.cpp:199) > > 8 com.apple.JavaScriptCore 0x00000001083e1e68 > > bmalloc::Allocator::allocate(unsigned long) + 72 (Allocator.h:91) > > 9 com.apple.JavaScriptCore 0x00000001083e1db6 > > bmalloc::Cache::allocate(bmalloc::HeapKind, unsigned long) + 166 (Cache.h:79) > > 10 com.apple.JavaScriptCore 0x00000001083e157b > > bmalloc::api::malloc(unsigned long, bmalloc::HeapKind) + 27 (bmalloc.h:49) > > > > > > Full crash log here: > >
https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/
> >
r230190
%20(7008)/results.html > > Interesting. I'll look into this. These asserts I added should always hold, > so the bug is probably elsewhere if they don't hold.
This might actually be a bug in my code with how I handle rounding.
Saam Barati
Comment 23
2018-04-03 12:05:09 PDT
Fix coming in:
https://bugs.webkit.org/show_bug.cgi?id=184275
Geoffrey Garen
Comment 24
2018-07-24 17:16:19 PDT
Comment on
attachment 337008
[details]
patch Seems wrong to me that bmalloc and Iso subspaces need to communicate about madvise. If Iso subspaces want isolated VM whose physical pages they manage, and whose virtual pages are never shared with other objects, they should just call mmap() themselves and not use bmalloc.
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