| Summary: | Use FastMalloc (bmalloc) instead of BlockAllocator for GC pages | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||||||||||||
| Component: | New Bugs | Assignee: | Geoffrey Garen <ggaren> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | achristensen, ap, benjamin, bfulgham, cmarcelo, commit-queue, kling, mhahnenb, ossy, roger_fong, sam | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Bug Depends on: | 140951, 140953, 141119 | ||||||||||||||||
| Bug Blocks: | |||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Geoffrey Garen
2015-01-26 13:44:41 PST
Created attachment 245372 [details]
Patch
Created attachment 245374 [details]
run-jsc-benchmarks results
Uploading performance results.
Comment on attachment 245372 [details]
Patch
Lots of red, me gusta :-) Looks like builds are angry. r=me w/ fixes.
What's bmalloc's story for returning memory to the OS? I'm assuming it's something similar to what BlockAllocator did, just curious.
> What's bmalloc's story for returning memory to the OS? I'm assuming it's
> something similar to what BlockAllocator did, just curious.
bmalloc uses a timer on a background thread with back-off if other threads start asking the heap to grow -- very similar to what BlockAllocator did.
Created attachment 245382 [details]
Patch
Trying to fix the build... Committed r179192: <http://trac.webkit.org/changeset/179192> (In reply to comment #7) > Committed r179192: <http://trac.webkit.org/changeset/179192> It made almost all tests assert on the 32 bit bot: https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/7055 and many failures on the ARM bot: http://build.webkit.sed.hu/builders/EFL%20ARMv7-ARM%20Linux%20Release%20%28Build%29%201404/builds/106 and all tests fail on the Yosemite performance bot: https://build.webkit.org/builders/Apple%20Yosemite%20Release%20%28Perf%29/builds/847 Strangely, it seems flaky. I forced clean builds on Yosemite, although there probably is a real problem. Re-opened since this is blocked by bug 140951 and Windows too: https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/49296 more info: There are 2 Yosemite bots: - https://build.webkit.org/buildslaves/bot191 (pass) - https://build.webkit.org/buildslaves/bot190 (fail) - https://build.webkit.org/buildslaves/bot192 (fail) - https://build.webkit.org/buildslaves/bot193 (pass) ... but failures occure only one of them. Maybe there are some black magic intalled on them which is necessary to make tests pass. any plan to fix this serious regression? Matt rolled this out in r179211, however Mac Yosemite bots went green even before that. We did a clean build, and also there was a seemingly unrelated patch landed in the meanwhile (http://trac.webkit.org/changeset/179202). Not sure about ARM and 32-bit tests. Committed r179348: <http://trac.webkit.org/changeset/179348> Committed r179361: <http://trac.webkit.org/changeset/179361> Committed r179407: <http://trac.webkit.org/changeset/179407> Committed r179426: <http://trac.webkit.org/changeset/179426> (In reply to comment #20) > Committed r179426: <http://trac.webkit.org/changeset/179426> Looks like ~7% memory use regression on the ol' buster. Re-opened since this is blocked by bug 141119 (In reply to comment #18) > Committed r179361: <http://trac.webkit.org/changeset/179361> Looks like ~40% regression on Bindings/create-element from this change: https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mavericks%22%2C%22Bindings%2Fcreate-element%3ARuns%22%5D%5D Committed r179500: <http://trac.webkit.org/changeset/179500> Reopening to attach new patch. Created attachment 248031 [details]
Patch
> > Committed r179361: <http://trac.webkit.org/changeset/179361>
>
> Looks like ~40% regression on Bindings/create-element from this change:
>
> https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-
> mavericks%22%2C%22Bindings%2Fcreate-element%3ARuns%22%5D%5D
This regression mysteriously disappeared after a bmalloc refactoring patch.
Committed r181157: <http://trac.webkit.org/changeset/181157> (In reply to comment #28) > Committed r181157: <http://trac.webkit.org/changeset/181157> It broke zillion tests on the 32 bit bot: https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/8148 and on the Windows bot too: https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/50194 Geoffrey, are you working on fixing the regression on 32 bit bots? sunspider-1.0/access-nsieve.js.default: ASSERTION FAILED: is8ByteAligned(reinterpret_cast<void*>(m_remaining)) sunspider-1.0/access-nsieve.js.default: /Volumes/Data/slave/mavericks-32bitJSC-debug/build/Source/JavaScriptCore/heap/CopiedBlock.h(156) : JSC::CopiedBlock::CopiedBlock(size_t) sunspider-1.0/access-nsieve.js.default: 1 0xaf561d WTFCrash sunspider-1.0/access-nsieve.js.default: 2 0x2752f2 JSC::CopiedBlock::CopiedBlock(unsigned long) sunspider-1.0/access-nsieve.js.default: 3 0x2751d4 JSC::CopiedBlock::CopiedBlock(unsigned long) sunspider-1.0/access-nsieve.js.default: 4 0x2751a1 JSC::CopiedBlock::createNoZeroFill(unsigned long) sunspider-1.0/access-nsieve.js.default: 5 0x2723e7 JSC::CopiedBlock::create(unsigned long) sunspider-1.0/access-nsieve.js.default: 6 0x270bc1 JSC::CopiedSpace::tryAllocateOversize(unsigned long, void**) I think sizeof(CopiedBlock) must not be 8-byte-aligned on 32-bit platforms. I'm regression-testing a fix. Committed r181177: <http://trac.webkit.org/changeset/181177> (In reply to comment #34) > Committed r181177: <http://trac.webkit.org/changeset/181177> The Windows bot is still broken: https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/50202 Committed revision 181180: <http://trac.webkit.org/changeset/181180>. Reopening to attach new patch. Created attachment 248108 [details]
Patch
Committed r181210: <http://trac.webkit.org/changeset/181210> Reopening to attach new patch. Created attachment 248166 [details]
Patch
> Looks like ~7% memory use regression on the ol' buster. This regression did not reappear in re-landing this patch broken out into pieces, probably because of these improvements to bmalloc: https://bugs.webkit.org/show_bug.cgi?id=142055 https://bugs.webkit.org/show_bug.cgi?id=142058 https://bugs.webkit.org/show_bug.cgi?id=142194 Comment on attachment 248166 [details]
Patch
rs=me to remove, but one thing needs fixing:
/Volumes/Data/EWS/WebKit/Source/WebCore/platform/cocoa/MemoryPressureHandlerCocoa.mm:156:42: error: no member named 'blockAllocator' in 'JSC::Heap'
JSDOMWindowBase::commonVM().heap.blockAllocator().releaseFreeRegions();
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
1 error generated.
Committed r181215: <http://trac.webkit.org/changeset/181215> |