Closed
Bug 420476
Opened 17 years ago
Closed 13 years ago
jsarena code allocates too much due to alignment math
Categories
(Core :: JavaScript Engine, defect, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 675545
People
(Reporter: moz, Assigned: moz)
References
Details
(Keywords: memory-footprint, perf)
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3
Build Identifier:
The code in jsarena.c adds overhead to allocations to account for alignment. The code currently adds too many bytes in some cases, resulting in unnecessarily large mallocations.
See 'gross' in JS_ArenaAllocate, and consider the case of nonzero pool->mask.
Reproducible: Always
Updated•17 years ago
|
Assignee: general → moz
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P3
Assignee | ||
Comment 1•17 years ago
|
||
This patch provides a sneak peak at what I'm doing in this bug. The patch works. It reduces the 'gross' size requested from malloc()s. It also makes code changes which clarify the alignment math that was previously allocating slightly too much in some circumstances.
This patch is relative to changes that I made in my last patch in bug 415967.
We expect the memory use impact of this patch to be negligible relative to that of the patch in bug 415967. It's the latter that saves us the memory. This patch just fixes the confusing alignment math. I'll post memstats soon.
Note that the changes in this patch should make it very easy to fix bug 408921. That bug is the real motivator for this patch, in fact.
Assignee | ||
Comment 2•17 years ago
|
||
This patch is a completed version of the "sneak peak" patch. The patch is relative to the patch from bug 415967. I will upload the real patch when bug 415967 is resolved.
Stats coming up.
Attachment #307750 -
Attachment is obsolete: true
Assignee | ||
Comment 3•17 years ago
|
||
Attachment 307817 [details] [diff] yields measurable results on a 'start and then stop the browser' test:
Before the patch:
numMallocs: 6355
numReallocs: 5
numFrees: 6355
hwmRequested: 332818
hwmReal: 374560
After the patch:
numMallocs: 6354
numReallocs: 5
numFrees: 6354
hwmRequested: 324176
hwmReal: 365840
So, a mild improvement in allocated memory. More interesting are the histograms of memory allocations, which is we this patch is intended to fix. In the histograms, the value ((x,y),z) means that an allocation of size x was made, and it really had size y when you include the malloc overhead; and there were z such allocations.
Before patch:
(275,288),297
(303,304),1
(671,1024),1
(1040,1536),1499
(1047,1536),4396
(2075,2560),5
(8211,8704),37
(12307,12800),124
After the patch:
(272,272),296
(300,304),1
(668,1024),1
(1040,1536),5895
(2072,2560),5
(8208,8704),38
(12304,12800),123
Of course, this is not the same kind of improvement as with the patch from bug 415967.
Assignee | ||
Comment 4•17 years ago
|
||
I have made a further modification to the code, in order to propose a solution
to bug 408921. On top of my prior patches, I also make this change:
pool->first.base = pool->first.avail = pool->first.limit =
JS_ARENA_ALIGN(pool, &pool->first + 1);
pool->current = &pool->first;
- pool->arenasize = JS_ARENA_ALIGN(pool, size);
+ JS_ASSERT(META_SIZE < size);
+ pool->arenasize = JS_ARENA_ALIGN(pool, size) - META_SIZE;
pool->quotap = quotap;
#ifdef JS_ARENAMETER
memset(&pool->stats, 0, sizeof pool->stats);
then bug 408921 goes away, and memory use is improved, at the cost of more
calls to malloc. The stats after the above change are:
numMallocs: 6458
numReallocs: 16
numFrees: 6458
hwmRequested: 323864
hwmReal: 324352
The histogram shows that the change has had its intended effect:
(256,256),380
(300,304),1
(668,1024),1
(1024,1024),5909
(1048,1536),18
(2072,2560),5
(8192,8192),37
(12288,12288),123
This is a further reduction of real memory use (by a further 5%) over the prior
patch. The cumulative effect of this change, bug 415967 and bug 420476, is a
14% reduction in real memory use by jsarenas, in the 'start-stop' test.
In my informal 'use Gmail' test, this patch made the results slightly worse
than with only 415967 applied. This makes sense, because it means that larger
arenas (than requested) are doing the code some good. I will open a low
priority defect to tune the arena sizes.
Assignee | ||
Updated•17 years ago
|
Assignee | ||
Comment 5•17 years ago
|
||
(In reply to comment #4)
> I will open a low priority defect to tune the arena sizes.
It's bug 421435.
Assignee | ||
Comment 6•17 years ago
|
||
Minor improvements to prior patch.
Attachment #307817 -
Attachment is obsolete: true
Attachment #309173 -
Flags: review?(crowder)
Assignee | ||
Comment 7•17 years ago
|
||
I'll make a patch which does not depend on 415967.
No longer depends on: 415967
Assignee | ||
Comment 8•17 years ago
|
||
... as promised. I continue to investigate performance characteristics of this patch.
Attachment #309173 -
Attachment is obsolete: true
Attachment #309173 -
Flags: review?(crowder)
Assignee | ||
Comment 9•17 years ago
|
||
This is an updated patch which includes changes from bug 412866 and 424287. (See bug 408921 for the motivation for this bug.)
Attachment #310721 -
Attachment is obsolete: true
Comment 10•13 years ago
|
||
Bug 675545 will fix this by only allowing alignment to be one of 1, 2, 4 or 8, which makes everything else much simpler.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•