Bug 141097 - GC marking threads should clear malloc caches
Summary: GC marking threads should clear malloc caches
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-30 13:35 PST by Geoffrey Garen
Modified: 2015-01-30 17:16 PST (History)
4 users (show)

See Also:

Attachments
Patch (7.69 KB, patch)
2015-01-30 14:11 PST, Geoffrey Garen 		no flags Details | Formatted Diff | Diff
Patch (1.90 KB, patch)
2015-01-30 16:27 PST, Geoffrey Garen 		sam: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 Geoffrey Garen 2015-01-30 13:35:57 PST 
GC marking threads should clear malloc caches
Comment 1 Geoffrey Garen 2015-01-30 14:11:14 PST 
Created attachment 245740 [details]
Patch
Comment 2 Andreas Kling 2015-01-30 14:18:53 PST 
Comment on attachment 245740 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:8
> +        This is an attempt to amelirate a potential memory use regression

ameliorate
Comment 3 Geoffrey Garen 2015-01-30 14:22:52 PST 
Committed r179411: <http://trac.webkit.org/changeset/179411>
Comment 4 Geoffrey Garen 2015-01-30 14:27:51 PST 
Committed r179412: <http://trac.webkit.org/changeset/179412>
Comment 5 Mark Hahnenberg 2015-01-30 14:50:03 PST 
Comment on attachment 245740 [details]
Patch

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

> Source/JavaScriptCore/heap/GCThread.cpp:79
> +    WTF::releaseFastMallocFreeMemoryForThisThread();

I wonder if it makes more sense to do this at the end of collection rather than between phases? Seems a shame to clear out the per-thread malloc cache between marking and copying since copying might want to reuse some stuff from the marking phase.
Comment 6 Mark Hahnenberg 2015-01-30 14:54:11 PST 
Also, maybe I dunno what scavenging is...
Comment 7 Geoffrey Garen 2015-01-30 15:47:39 PST 
> I wonder if it makes more sense to do this at the end of collection rather
> than between phases?

Yeah, end of collection is better. How to achieve?

Maybe move the code to the end of the 'case Copy' block?
Comment 8 Mark Hahnenberg 2015-01-30 16:14:46 PST 
Yeah that seems ok. There shouldn't be any parallel work left to do at the end of the CopyPhase, so that sounds like a good time to clear the malloc caches on those threads.
Comment 9 Mark Hahnenberg 2015-01-30 16:17:21 PST 
Another idea might be to only clear the caches during collections caused by the GCTimer.
Comment 10 Geoffrey Garen 2015-01-30 16:27:53 PST 
Reopening to attach new patch.
Comment 11 Geoffrey Garen 2015-01-30 16:27:54 PST 
Created attachment 245755 [details]
Patch
Comment 12 Geoffrey Garen 2015-01-30 16:29:17 PST 
Committed r179422: <http://trac.webkit.org/changeset/179422>
Comment 13 Chris Dumez 2015-01-30 16:34:33 PST 
Looks like some JSC tests are failing on the bots after this change?
https://build.webkit.org/builders/Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/1796/steps/jscore-test/logs/stdio
Comment 15 Geoffrey Garen 2015-01-30 17:16:34 PST 
Turned out to be a sporadic failure in that date test.