Bug 38368 - When FastMalloc Returns memory often left with lots of small spans
Summary: When FastMalloc Returns memory often left with lots of small spans
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-04-29 19:44 PDT by Stephanie Lewis
Modified: 2010-05-03 19:40 PDT (History)
4 users (show)

See Also:


Attachments
patch (4.00 KB, patch)
2010-04-29 19:53 PDT, Stephanie Lewis
no flags Details | Formatted Diff | Diff
second patch (4.02 KB, patch)
2010-04-29 20:23 PDT, Stephanie Lewis
eric: review-
Details | Formatted Diff | Diff
patch with correct fix (4.02 KB, patch)
2010-04-30 17:39 PDT, Stephanie Lewis
ggaren: review-
Details | Formatted Diff | Diff
patch (4.04 KB, patch)
2010-05-03 15:52 PDT, Stephanie Lewis
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephanie Lewis 2010-04-29 19:44:07 PDT
When FastMalloc Returns memory we're often left with lots of 1 page spans and none of the large sizes.  Change the scavenging algorithm to always keep some large size spans around.  Also change it distribute the returns among the list more equally. 
<rdar://problem/7834433> REGRESSSION: 1.5% PLT regression due to 56028 (return memory quicker).
Comment 1 Stephanie Lewis 2010-04-29 19:53:54 PDT
Created attachment 54775 [details]
patch

Attaching Patch.
Comment 2 Eric Seidel (no email) 2010-04-29 20:05:50 PDT
Attachment 54775 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1792206
Comment 3 WebKit Review Bot 2010-04-29 20:06:46 PDT
Attachment 54775 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/wtf/FastMalloc.cpp:1281:  Missing spaces around /  [whitespace/operators] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1546:  Extra space before )  [whitespace/parens] [2]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Stephanie Lewis 2010-04-29 20:23:10 PDT
Created attachment 54776 [details]
second patch

fixed some issues.
Comment 5 Eric Seidel (no email) 2010-04-29 20:28:15 PDT
Attachment 54776 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1935031
Comment 6 Eric Seidel (no email) 2010-04-30 11:48:23 PDT
Comment on attachment 54776 [details]
second patch

The EWS thinks this will break leopard:
fzzgiiisjukfwleasczjmdxaevcz/JavaScriptCorePrefix.h -c /Users/eseidel/Projects/MacEWS/JavaScriptCore/wtf/FastMalloc.cpp -o /Users/eseidel/Projects/MacEWS/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/i386/FastMalloc.o
cc1plus: warnings being treated as errors
/Users/eseidel/Projects/MacEWS/JavaScriptCore/wtf/FastMalloc.cpp:1281: warning: implicit conversion shortens 64-bit value into a 32-bit value
/Users/eseidel/Projects/MacEWS/JavaScriptCore/wtf/FastMalloc.cpp: In member function ‘void WTF::TCMalloc_PageHeap::scavenge()’:
/Users/eseidel/Projects/MacEWS/JavaScriptCore/wtf/FastMalloc.cpp:1546: warning: implicit conversion shortens 64-bit value into a 32-bit value
distcc[85602] ERROR: compile /Users/eseidel/Projects/MacEWS/JavaScriptCore/wtf/FastMalloc.cpp on localhost failed
Comment 7 Stephanie Lewis 2010-04-30 17:39:22 PDT
Created attachment 54841 [details]
patch with correct fix

and now it builds on 32bit
Comment 8 Geoffrey Garen 2010-05-03 14:47:43 PDT
Comment on attachment 54841 [details]
patch with correct fix

I think the "l" postfix is wrong, since it will give you a long where you want a floating point number. The simplest way to appease the 64-bit warning is probably to use float, since it's 32-bit.
Comment 9 Stephanie Lewis 2010-05-03 15:52:42 PDT
Created attachment 54971 [details]
patch

one more time
Comment 10 Geoffrey Garen 2010-05-03 16:24:17 PDT
Comment on attachment 54971 [details]
patch

r=me

You can just declare "j" to be size_t, to avoid the static_cast.
Comment 11 Stephanie Lewis 2010-05-03 19:40:42 PDT
Committed http://trac.webkit.org/changeset/58730