Bug 63015

Summary:

releaseFastMallocFreeMemory doesn't adjust free counts for scavenger

Product:

WebKit

Reporter:

Michael Saboff <msaboff>

Component:

WebCore Misc.

Assignee:

Michael Saboff <msaboff>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

adachan, ggaren, mrowe, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch to update scavenger counts in ReleaseFreeList	msaboff: review+, webkit-ews: commit-queue-
Updated patch - fixes qt failure	none
Updated Patch with bug reference in ChangeLog	none
Patch with updates from comment #9	ggaren: review+

Michael Saboff

Reported 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.

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 attachment proposed patch, testcase, etc.

Michael Saboff

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

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

WebKit Review Bot

Comment 2 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.

Early Warning System Bot

Comment 3 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

Michael Saboff

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

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

WebKit Review Bot

Comment 5 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.

Darin Adler

Comment 6 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.

Michael Saboff

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

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

WebKit Review Bot

Comment 8 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.

Geoffrey Garen

Comment 9 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)?

Michael Saboff

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

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

WebKit Review Bot

Comment 11 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.

Geoffrey Garen

Comment 12 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.

Geoffrey Garen

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

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

Michael Saboff

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

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

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