Bug 196121 - [BMalloc] No need to delay deallocating chunks based on recent use
Summary: [BMalloc] No need to delay deallocating chunks based on recent use
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-21 17:39 PDT by Michael Saboff
Modified: 2019-03-22 10:43 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.17 KB, patch)
2019-03-21 17:53 PDT, Michael Saboff
mark.lam: review+
msaboff: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews204 for win-future (12.92 MB, application/zip)
2019-03-21 20:32 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.94 MB, application/zip)
2019-03-21 22:53 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2019-03-21 17:39:23 PDT
This is a follow-up for r243144 <https://trac.webkit.org/changeset/243144>.  The code checked in with that change set employed the used since last scavenge logic on small chunks, but that isn't needed for small chunks as their memory isn't decommitted directly.  Instead we "deallocate" small chunks by adding them to a large range free list.  When this large range free list is scavenged, we'll actually decommit the backing memory.  This reduces the number of scavenger passes needed to decommit small chunks from 4 to 3.
Comment 1 Radar WebKit Bug Importer 2019-03-21 17:41:06 PDT
<rdar://problem/49133014>
Comment 2 Michael Saboff 2019-03-21 17:53:24 PDT
Created attachment 365657 [details]
Patch
Comment 3 Mark Lam 2019-03-21 18:26:35 PDT
Comment on attachment 365657 [details]
Patch

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

r=me if EWS bots are happy.

> Source/bmalloc/ChangeLog:9
> +        We can decommit small chunks immediately as that adds them to the LargeRange free list.  That free list employs the

By "decommit" here, do you mean "deallocate" instead?
Comment 4 Michael Saboff 2019-03-21 19:31:13 PDT
Comment on attachment 365657 [details]
Patch

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

>> Source/bmalloc/ChangeLog:9
>> +        We can decommit small chunks immediately as that adds them to the LargeRange free list.  That free list employs the
> 
> By "decommit" here, do you mean "deallocate" instead?

The terms in this case are nearly the same, but I'll change it to deallocate as that is part of the function's name.
Comment 5 Mark Lam 2019-03-21 19:36:35 PDT
Comment on attachment 365657 [details]
Patch

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

>>> Source/bmalloc/ChangeLog:9
>>> +        We can decommit small chunks immediately as that adds them to the LargeRange free list.  That free list employs the
>> 
>> By "decommit" here, do you mean "deallocate" instead?
> 
> The terms in this case are nearly the same, but I'll change it to deallocate as that is part of the function's name.

Yeah, I was just a bit confused because in the previous sentence, it said "small chunks ... their memory isn't decommitted directly", and then, we follow it with this "We can decommit small chunks immediately".  I think "deallocate" referring to the function that we call is less confusing.
Comment 6 EWS Watchlist 2019-03-21 20:32:45 PDT
Comment on attachment 365657 [details]
Patch

Attachment 365657 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11608466

New failing tests:
legacy-animation-engine/animations/resume-after-page-cache.html
Comment 7 EWS Watchlist 2019-03-21 20:32:56 PDT
Created attachment 365673 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 8 EWS Watchlist 2019-03-21 22:53:15 PDT
Comment on attachment 365657 [details]
Patch

Attachment 365657 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11609357

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 9 EWS Watchlist 2019-03-21 22:53:17 PDT
Created attachment 365681 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 10 Michael Saboff 2019-03-22 10:16:25 PDT
The mac-wk2 test results without this patch show a crash in media recorder tests, so that failure doesn't seem relevant.

The windows test legacy-animation-engine/animations/resume-after-page-cache.html
 is failing intermittently in windows builds.

Going to land this patch as these failures don't seem relevant.
Comment 11 Michael Saboff 2019-03-22 10:43:31 PDT
Committed r243389: <https://trac.webkit.org/changeset/243389>