63015 – releaseFastMallocFreeMemory doesn't adjust free counts for scavenger

Bug 63015 - releaseFastMallocFreeMemory doesn't adjust free counts for scavenger

Summary: releaseFastMallocFreeMemory doesn't adjust free counts for scavenger

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	WebCore Misc. (show other bugs)
Version:	528+ (Nightly build)
Hardware:	All All

Importance:	P2 Normal
Assignee:	Michael Saboff

URL:
Keywords:

Depends on:
Blocks:

Reported:	2011-06-20 14:49 PDT by Michael Saboff
Modified:	2011-06-24 17:12 PDT (History)
CC List:	4 users (show)

See Also:

Attachments
Patch to update scavenger counts in ReleaseFreeList (4.67 KB, patch) 2011-06-20 15:05 PDT, Michael Saboff	msaboff: review+ webkit-ews: commit-queue-	Details \| Formatted Diff \| Diff
Updated patch - fixes qt failure (4.65 KB, patch) 2011-06-20 15:37 PDT, Michael Saboff	no flags	Details \| Formatted Diff \| Diff
Updated Patch with bug reference in ChangeLog (4.78 KB, patch) 2011-06-20 17:01 PDT, Michael Saboff	no flags	Details \| Formatted Diff \| Diff
Patch with updates from comment #9 (4.97 KB, patch) 2011-06-20 22:59 PDT, Michael Saboff	ggaren: review+	Details \| Formatted Diff \| Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Michael Saboff 2011-06-20 14:49:24 PDT

When WTF::releaseFastMallocFreeMemory() is invoked, the variables free_committed_pages_ and min_free_committed_pages_since_last_scavenge_ are not updated for the released pages.

Comment 1 Michael Saboff 2011-06-20 15:05:30 PDT

Created attachment 97867 [details]
Patch to update scavenger counts in ReleaseFreeList

Comment 2 WebKit Review Bot 2011-06-20 15:09:57 PDT

Attachment 97867 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1364:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2127:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2128:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2142:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2148:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2148:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2153:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2163:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2167:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2167:  TCMalloc_PageHeap::ReleaseFreeList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2171:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 20 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 3 Early Warning System Bot 2011-06-20 15:23:11 PDT

Comment on attachment 97867 [details]
Patch to update scavenger counts in ReleaseFreeList

Attachment 97867 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8910658

Comment 4 Michael Saboff 2011-06-20 15:37:17 PDT

Created attachment 97871 [details]
Updated patch - fixes qt failure

Comment 5 WebKit Review Bot 2011-06-20 15:41:50 PDT

Attachment 97871 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1364:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2127:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2141:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2147:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2147:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2162:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2166:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2166:  TCMalloc_PageHeap::ReleaseFreeList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2170:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 19 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 6 Darin Adler 2011-06-20 16:23:25 PDT

Comment on attachment 97871 [details]
Updated patch - fixes qt failure

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

>> Source/JavaScriptCore/ChangeLog:1
>> +2011-06-20  Michael Saboff  <msaboff@apple.com>
> 
> ChangeLog entry has no bug number  [changelog/bugnumber] [5]

Stylebot is right about this!

>>>> Source/JavaScriptCore/wtf/FastMalloc.cpp:1360
>>>> +  size_t CheckList(Span* list, Length min_pages, Length max_pages, bool decommitted);
>>> 
>>> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
>> 
>> min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
> 
> max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]

We need to tell the Stylebot that this file is imported code with an unusual style.

Comment 7 Michael Saboff 2011-06-20 17:01:28 PDT

Created attachment 97890 [details]
Updated Patch with bug reference in ChangeLog

Comment 8 WebKit Review Bot 2011-06-20 17:05:40 PDT

Attachment 97890 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1364:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2127:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2141:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2147:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2147:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2162:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2166:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2166:  TCMalloc_PageHeap::ReleaseFreeList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2170:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 18 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 9 Geoffrey Garen 2011-06-20 21:20:06 PDT

Comment on attachment 97890 [details]
Updated Patch with bug reference in ChangeLog

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

This ChangeLog would benefit from an explanation of *why* ReleaseFreeList needs to update these variables, and not just the *what* of updating them. What was the symptom when the variables weren't updated?

> Source/JavaScriptCore/wtf/FastMalloc.cpp:2149
>    return true;
>  }

Can this be right (returning true for size_t)?

Comment 10 Michael Saboff 2011-06-20 22:59:06 PDT

Created attachment 97932 [details]
Patch with updates from comment #9

Comment 11 WebKit Review Bot 2011-06-20 23:00:24 PDT

Attachment 97932 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1360:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:1364:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2127:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2141:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2147:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2147:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2148:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  TCMalloc_PageHeap::CheckList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  min_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2151:  max_pages is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2152:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2162:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2166:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2166:  TCMalloc_PageHeap::ReleaseFreeList is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/FastMalloc.cpp:2170:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 19 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 12 Geoffrey Garen 2011-06-24 14:31:42 PDT

> What was the symptom when the variables weren't updated?

The symptom is that sometimes the scavenger wakes up every 2 seconds and scans the heap, even though there's no work to do.

^ I think you should put this statement in the ChangeLog.

Comment 13 Geoffrey Garen 2011-06-24 14:31:56 PDT

Comment on attachment 97932 [details]
Patch with updates from comment #9

r=me

Comment 14 Michael Saboff 2011-06-24 17:12:10 PDT

Committed r89716: <http://trac.webkit.org/changeset/89716>