Closed Bug 421435 Opened 12 years ago Closed 9 years ago
Tune sizes of arenas using jsarenas
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.
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?
(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.
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.
If comment 4 is right we should take this for b5..
Priority: P3 → P1
I'll see if my sunspider results agree. I'm on a Mac, though, which tends to be less sensitive to allocation costs....
If you kickoff a try server build I'll get memory and perf nums on vista...
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.
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?
Okay, two of my three try builds are done. I have to hop on a plane, but will be back later tonight.
Sorry, ran them under the name "poolsize_patch"
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.
(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.
(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?
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.
Attachment #310183 - Attachment is obsolete: true
(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?
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.
(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.
The "thrashing" mentioned in the prior comment should be fixed by 412866. Checking....
Depends on: 420476
Priority: P1 → P3
This change went in via bug 558971.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.