Bug 127067 – Remove ENABLE_GLOBAL_FASTMALLOC_NEW

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Remove ENABLE_GLOBAL_FASTMALLOC_NEW

[Anders Carlsson]

Created attachment 221309 [details]
Patch

[Geoffrey Garen]

Comment on attachment 221309 [details]
Patch

r=me

Before landing, I think we need to test performance and make sure we're not missing any significant WTF_MAKE_FAST_ALLOCATED declarations.

Would also be nice just to set a breakpoint / add a shim method and print out all allocations in WebKit that miss FastMalloc, and drive to 0.

[Geoffrey Garen]

> Would also be nice just to set a breakpoint / add a shim method and print out all allocations in WebKit that miss FastMalloc, and drive to 0.

To clarify, I don't think we need to do this task before landing -- I just think it would be fun!

[Anders Carlsson]

I'm going to hold off on landing this until some PLT regressions are tracked down.

[Stephanie Lewis]

Haven't looked at memory numbers but the PLT shows this to be a significant improvement. :)

[Geoffrey Garen]

> Haven't looked at memory numbers but the PLT shows this to be a significant improvement. :)

Wat? What are the exact numbers?

I can't imagine why this might be a speedup.

[Stephanie Lewis]

Ran again with production roots and the patch is still slightly faster, but nothing to write home about.

A quick check with membuster shows that in the user scenario ~15 MB is now allocated with System Malloc where it used to be allocated with FastMalloc.  Given that I'll want to check how memory returning and fragmentation is affected.

[Anders Carlsson]

(In reply to comment #7)
> Ran again with production roots and the patch is still slightly faster, but nothing to write home about.

My guess is that it's just variance.

> A quick check with membuster shows that in the user scenario ~15 MB is now allocated with System Malloc where it used to be allocated with FastMalloc.  Given that I'll want to check how memory returning and fragmentation is affected.

This is interesting. Is there any way to compare the heaps to see what data we're now allocating with System Malloc (and fix the code so it uses fastMalloc instead)?

[Stephanie Lewis]

We could run membuster with malloc history enabled and write a script to compare the two and dump the differences.

[Anders Carlsson]

Turns out it's really easy to see where we call operator new() vs fastMalloc in Instruments. With the changes I landed in http://trac.webkit.org/projects/webkit/changeset/163757 we now allocated pretty much everything with fastMalloc (tested loading the WK1 spec). I'm going to land this.

[WebKit Commit Bot]

Comment on attachment 221309 [details]
Patch

Clearing flags on attachment: 221309

Committed r164261: <http://trac.webkit.org/changeset/164261>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.