Bug 140900 – Use FastMalloc (bmalloc) instead of BlockAllocator for GC pages

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

Use FastMalloc (bmalloc) instead of BlockAllocator for GC pages

[Geoffrey Garen]

Created attachment 245372 [details]
Patch

[Geoffrey Garen]

Created attachment 245374 [details]
run-jsc-benchmarks results

Uploading performance results.

[Mark Hahnenberg]

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.

[Geoffrey Garen]

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

[Geoffrey Garen]

Created attachment 245382 [details]
Patch

[Geoffrey Garen]

Trying to fix the build...

[Geoffrey Garen]

Committed r179192: <http://trac.webkit.org/changeset/179192>

[Csaba Osztrogonác]

(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

[Csaba Osztrogonác]

and many failures on the ARM bot:
http://build.webkit.sed.hu/builders/EFL%20ARMv7-ARM%20Linux%20Release%20%28Build%29%201404/builds/106

[Csaba Osztrogonác]

and all tests fail on the Yosemite performance bot:
https://build.webkit.org/builders/Apple%20Yosemite%20Release%20%28Perf%29/builds/847

[Alexey Proskuryakov]

Strangely, it seems flaky. I forced clean builds on Yosemite, although there probably is a real problem.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 140951

[Csaba Osztrogonác]

and Windows too:
https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/49296

[Csaba Osztrogonác]

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.

[Csaba Osztrogonác]

any plan to fix this serious regression?

[Alexey Proskuryakov]

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.

[Geoffrey Garen]

Committed r179348: <http://trac.webkit.org/changeset/179348>

[Geoffrey Garen]

Committed r179361: <http://trac.webkit.org/changeset/179361>

[Geoffrey Garen]

Committed r179407: <http://trac.webkit.org/changeset/179407>

[Geoffrey Garen]

Committed r179426: <http://trac.webkit.org/changeset/179426>

[Andreas Kling]

(In reply to comment #20)
> Committed r179426: <http://trac.webkit.org/changeset/179426>

Looks like ~7% memory use regression on the ol' buster.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 141119

[Andreas Kling]

(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

[Geoffrey Garen]

Committed r179500: <http://trac.webkit.org/changeset/179500>

[Geoffrey Garen]

Reopening to attach new patch.

[Geoffrey Garen]

Created attachment 248031 [details]
Patch

[Geoffrey Garen]

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

[Geoffrey Garen]

Committed r181157: <http://trac.webkit.org/changeset/181157>

[Csaba Osztrogonác]

(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

[Csaba Osztrogonác]

and on the Windows bot too:
https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/50194

[Csaba Osztrogonác]

Geoffrey, are you working on fixing the regression on 32 bit bots?

[Geoffrey Garen]

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**)

[Geoffrey Garen]

I think sizeof(CopiedBlock) must not be 8-byte-aligned on 32-bit platforms. I'm regression-testing a fix.

[Geoffrey Garen]

Committed r181177: <http://trac.webkit.org/changeset/181177>

[Csaba Osztrogonác]

(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

[Geoffrey Garen]

Committed revision 181180: <http://trac.webkit.org/changeset/181180>.

[Geoffrey Garen]

Reopening to attach new patch.

[Geoffrey Garen]

Created attachment 248108 [details]
Patch

[Geoffrey Garen]

Committed r181210: <http://trac.webkit.org/changeset/181210>

[Geoffrey Garen]

Reopening to attach new patch.

[Geoffrey Garen]

Created attachment 248166 [details]
Patch

[Geoffrey Garen]

> 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

[Andreas Kling]

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.

[Geoffrey Garen]

Committed r181215: <http://trac.webkit.org/changeset/181215>