Last Comment Bug 421435 - Tune sizes of arenas using jsarenas
: Tune sizes of arenas using jsarenas
Status: RESOLVED FIXED
: footprint, perf
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P3 enhancement with 2 votes (vote)
: ---
Assigned To: Robin Bate Boerop
:
Mentors:
Depends on: 412866 420476
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-06 18:36 PST by Robin Bate Boerop
Modified: 2011-07-29 00:15 PDT (History)
12 users (show)
mtschrep: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Increase sizes of tempPool and properties pool (2.08 KB, patch)
2008-03-18 00:07 PDT, Robin Bate Boerop
no flags Details | Diff | Review
Change tempPool to 4KB (1.12 KB, patch)
2008-03-18 12:00 PDT, Robin Bate Boerop
no flags Details | Diff | Review
Weird SunSpider results (3.50 KB, text/plain)
2008-03-19 02:01 PDT, Robin Bate Boerop
no flags Details

Description Robin Bate Boerop 2008-03-06 18:36:57 PST
As mentioned in comment #4 of bug 420476, typical uses of Firefox would do better with arenas larger than they are currently.  This bug is to monitor and track attempts to tune the CALLERS of jsarena code, so that better arena sizes are chosen.
Comment 1 Brian Crowder 2008-03-07 00:02:01 PST
This one deserves better than P5; we shouldn't lose the work done in malloc_size to gross-not-net work.  That said, I'd really like to do the malloc_size stuff for PRArenas before nearly anything else.  What bug is that?
Comment 2 Robin Bate Boerop 2008-03-07 13:29:00 PST
(In reply to comment #1)
> This one deserves better than P5; we shouldn't lose the work done in
> malloc_size to gross-not-net work.  That said, I'd really like to do the
> malloc_size stuff for PRArenas before nearly anything else.  What bug is that?
Bug 419131.
Comment 3 Robin Bate Boerop 2008-03-13 13:42:01 PDT
Documenting relationship to bug 422245.
Comment 4 Robin Bate Boerop 2008-03-18 00:07:23 PDT
Created attachment 310183 [details] [diff] [review]
Increase sizes of tempPool and properties pool

This patch halves the number of calls to malloc from JSArena code during my informal "use Gmail" test.  It does this while also significantly decreasing the real memory use at the high water mark.  With patches from bug 415967 and bug 420476, the real memory use will remain unchanged (instead of decreasing), but the malloc calls will remain reduced.

This patch produces a 5-6% speed increase in Sunspider, in my tests.  It would be nice if someone could repro the performance increase.
Comment 5 Mike Schroepfer 2008-03-18 10:20:36 PDT
If comment 4 is right we should take this for b5..
Comment 6 Brian Crowder 2008-03-18 10:24:07 PDT
I'll see if my sunspider results agree.  I'm on a Mac, though, which tends to be less sensitive to allocation costs....
Comment 7 Mike Schroepfer 2008-03-18 10:25:33 PDT
If you kickoff a try server build I'll get memory and perf nums on vista...
Comment 8 Brian Crowder 2008-03-18 10:32:12 PDT
SunSpider results are definitely not conclusive on my machine.  I have started TryServer builds with the patch from comment #4, and I'll post a link for those shortly.
Comment 9 Brian Crowder 2008-03-18 10:52:02 PDT
Robin:  do you have a bug implementing a similar idea for PLArenas?  I recall having seen you post statistics about which arenas in the browser are using them most?  Perhaps we can hit the top PLArena users with a patch like this for some other easy wins?
Comment 10 Brian Crowder 2008-03-18 11:17:57 PDT
Okay, two of my three try builds are done.  I have to hop on a plane, but will be back later tonight.
Comment 11 Brian Crowder 2008-03-18 11:18:35 PDT
Sorry, ran them under the name "poolsize_patch"
Comment 12 Robin Bate Boerop 2008-03-18 11:25:44 PDT
I have found that the patch actually produces a SLOWDOWN on Sunspider, on my Mac, without my logging code enabled.  It produces favourable improvements in my "informal Gmail" test, and in the 'start-and-stop the browser' test, but NOT Sunspider.  I am investigating.

I suggest we not pay this bug too much attention until I've figured out why the significant negative effect on Sunspider.  Sorry for raising expectations - I should have benchmarked without my logging code in.
Comment 13 Robin Bate Boerop 2008-03-18 11:30:23 PDT
(In reply to comment #9)
> Robin:  do you have a bug implementing a similar idea for PLArenas?  I recall
> having seen you post statistics about which arenas in the browser are using
> them most?  Perhaps we can hit the top PLArena users with a patch like this for
> some other easy wins?

Yes, that is bug 422245.  It is waiting for review.  (I did that one first.)

I was able to make some substantial improvements, though not as significant as with this change.  That is in part because PLArenas have a freelist of unused arenas (which helps them), and JSArenas do not.  I am investigating the possible effects of adding some kind of freelist to JSArenas, but first need to investigate the Sunspider performance reduction.


Comment 14 Robin Bate Boerop 2008-03-18 11:32:02 PDT
(In reply to comment #8)
> SunSpider results are definitely not conclusive on my machine.  I have started
> TryServer builds with the patch from comment #4, and I'll post a link for those
> shortly.

On my machine, the results are conclusive: there is a slowdown, not a speedup.  Yours too?
Comment 15 Robin Bate Boerop 2008-03-18 12:00:24 PDT
Created attachment 310301 [details] [diff] [review]
Change tempPool to 4KB

The performance improvement of the prior patch was really a performance reduction, when measured without logging code enabled.  That proved that Sunspider is highly sensitive to memory allocations.

This patch produces a reproducible 1.4% speedup on my Mac (and no memory stats logging code is active).  I continue to investigate improvements.
Comment 16 Robin Bate Boerop 2008-03-18 12:03:40 PDT
(In reply to comment #7)
> If you kickoff a try server build I'll get memory and perf nums on vista...

I'm very interested in the effects on a jemalloc-enabled Windows machine.  What was the performance change, there?
Comment 17 Robin Bate Boerop 2008-03-19 02:01:58 PDT
Created attachment 310429 [details]
Weird SunSpider results

The attached file shows output from a Sunspider run comparison.  The "before"
is an unmodified build, and the "after" is a build which includes the changes
to JSArenaPool sizes that I thought was best (first attachment in this bug).

One can see that in all but two tests, the changed pool sizes produce a faster
Sunspider.  However, the 2 runs that are slower are much slower.  Overall, the
pool size changes slowed SunSpider.

The updated pool sizes produce better performance on all of my other tests.

So, what's the deal with the 2 tests that are slower?  (The slowed ones are
date-format-totfe and string-tagcloud.)

The answer is that they "thrash" arenas.  That is, arenas are allocated and
deallocated over and over again, without being fully used.

It takes slightly longer to allocate a large arena, compared with a small
arena.  Normally, when the arena sizes are increased, the number of arena
allocations are decreased.  However, in the case of date-format-tofte and
string-tagcloud, the number of arena allocations does not decrease when the
arena sizes increase.

This means that Spidermonkey is thrashing arenas, and not really using the
memory in those arenas.

If someone who knows about both Spidermonkey and Sunspider would care to
offer an explanation about why those two tests do that, it would be useful.

Meanwhile, I will try to make some changes which help with the thrashing.
Comment 18 Robin Bate Boerop 2008-03-19 02:04:25 PDT
(In reply to comment #17)
> Meanwhile, I will try to make some changes which help with the thrashing.

I have created bug 423815 for that.  Also, I have made a patch which proves that thrashing is a problem, and which partially solves it, with an accompanying improvement in SunSpider results.
Comment 19 Robin Bate Boerop 2008-04-02 14:21:39 PDT
The "thrashing" mentioned in the prior comment should be fixed by 412866.  Checking....
Comment 20 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-07-29 00:15:13 PDT
This change went in via bug 558971.

Note You need to log in before you can comment on or make changes to this bug.