423815 - JSArena code "thrashes" mallocated arenas

Status: RESOLVED DUPLICATE of bug 412866

(Core :: JavaScript Engine, defect, P4)

Description

[Robin Bate Boerop]

Allocating memory with malloc() is costly, and JSArena code does too much of it.  Worse, some of the SunSpider benchmarks repeatedly allocate and deallocate arenas, costing performance.

JSArenaPools should buffer one or more arenas, to avoid the unnecessary calls to malloc.

Comment 1

[Robin Bate Boerop]

I have sloppy patch which yields a reproducible 6% speed increase on SunSpider.  It does the following:

Associate with each JSArenaPool a single free arena.  It is always an arena equal to the arenasize of the pool.  Before malloc is called when a new arena is needed, the pool's free arena is checked for suitability (which is likely).  If it's suitable, then the already-allocated arena is used.

When arenas are freed, one of them is selected to be the pool's free arena, if it does not already have one.

This solution will likely have negative effects on footprint, but those could be addressed in another bug.  I will investigate, and post a patch soon.

Comment 2

[Robin Bate Boerop]

Attachment: Effects of sloppy patch on SunSpider (debug build)

One can see the across-the-board speedup achieved by the sloppy patch, giving a total speedup of 6%.

Comment 3

[Brian Crowder]

Debug builds will produce deceptive results on a benchmark like this, you should use opt-only for your sunspider testing.

Comment 4

[Robin Bate Boerop]

Attachment: Sloppy patch

This patch adds an "arena buffering" feature to JSArenaPools.  Gets me 6% performance improvement on SunSpider on a debug build.  The patch's effects on memory footprint are still unknown.

Comment 5

[Brian Crowder]

Please refresh your benchmark results in this and other bugs by running them against optimized shell builds.  I'll work on review after that.  For bugs like this, it would be nice to know differences in memory utilization (peak, malloc counts), as well.

Comment 6

[Mike Schroepfer]

Get me a try server build and I'll test on win/mac

Comment 7

[Robin Bate Boerop]

(In reply to comment #6)
> Get me a try server build and I'll test on win/mac

I have no Try server access.

Comment 8

[Brian Crowder]

The patch from comment #4 appears to be in an invalid format, did you add or remove lines manually?  Can you regenerate the patch with cvs diff -up8?  I will launch a Try build as soon as it's up.

Comment 9

[Robin Bate Boerop]

(In reply to comment #8)
> The patch from comment #4 appears to be in an invalid format, did you add or
> remove lines manually?  Can you regenerate the patch with cvs diff -up8?  I
> will launch a Try build as soon as it's up.

Yes, that's why I called it "the sloppy patch".  It was meant as a proof-of-concept only, to show what changes I made and what effect they had.  My only goal for the patch was to show that arena thrashing was occurring.  I'll upload a valid patch, now.

Comment 10

[Robin Bate Boerop]

Attachment: Non-sloppy patch

Comment 11

[Robin Bate Boerop]

In my tests, the most recent patch produces a small slowdown on optimized shell SunSpider runs.  There is no speedup.  Presumably, in the tradeoff of "fewer mallocs" versus "smaller footprint", "fewer mallocs" lost.

I will instrument my build, collect memory stats, and investigate.

It is possible that the effects of this change on a debug build are positive because of arena clearing (JS_CLEAR_UNUSED, etc.).  On debug builds, we actually write to the entire arena before freeing it, setting all the bytes to 0xDA.  This is expensive to do.  So, reducing the number of times that we do it makes a substantial performance improvement.

That is why the performance effects are opposite on a debug build from an optimized build, I suspect.

Comment 12

[Brian Crowder]

TryServer builds of the non-sloppy patch are up:
https://build.mozilla.org/tryserver-builds/2008-03-19_16:[e-mail]-bug_423815/

Comment 13

[Robin Bate Boerop]

Reducing the priority.  Bug 412866 is the likely culprit of the thrashing, and the solution there is better than the solution in this bug's patches.  I expect to eventually mark this bug a dup.

Comment 14

[Robin Bate Boerop]

Attachment: Final patch.

I have re-tested SunSpider performance on an optimized build of the js shell, using the attached patch.  Performance improves a negligible 0.1% for me.  So, I conclude that the arena thrashing was indeed caused by the other bug.  I will mark this a dup.