Closed
Bug 679191
Opened 13 years ago
Closed 13 years ago
Improve XPT Arena size increasing behavior
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: khuey, Assigned: khuey)
Details
(Whiteboard: [MemShrink])
Attachments
(1 file)
914 bytes,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
PT arenas behave well, unless you ask them for a single allocation larger than the block size. In that case, they allocate uneven blocks. This is probably a micro-optimization, but it's easy enough to do.
Attachment #553297 -
Flags: review?(nnethercote)
Comment 1•13 years ago
|
||
Do you have data that this is needed?
Assignee | ||
Comment 2•13 years ago
|
||
Depends on how you define "needed". My ad-hoc instrumentation showed that it looked like we're wasting about 10% of the memory allocated here. Using a total of 1 MB from the explicit/xpti-working-set reporter we're looking at 100 KB wasted for the lifetime of the browser. Not a big contribution, but given that it's a two line fix ...
Comment 3•13 years ago
|
||
Comment on attachment 553297 [details] [diff] [review] Patch Review of attachment 553297 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/typelib/xpt/src/xpt_arena.c @@ +215,5 @@ > size_t block_header_size = ALIGN_RND(sizeof(BLK_HDR), arena->alignment); > size_t new_space = arena->block_size; > > + while (bytes > new_space - block_header_size) > + new_space += arena->block_size; A comment here would be good, explaining that block_size is a power-of-two (I hope that's correct) and so this avoids slop.
Attachment #553297 -
Flags: review?(nnethercote) → review+
Comment 4•13 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2) > Depends on how you define "needed". My ad-hoc instrumentation showed that > it looked like we're wasting about 10% of the memory allocated here. Using > a total of 1 MB from the explicit/xpti-working-set reporter we're looking at > 100 KB wasted for the lifetime of the browser. Not a big contribution, but > given that it's a two line fix ... Ok, that's very reasonable. If you'd included that info in the patch submission I wouldn't need to have asked :)
Updated•13 years ago
|
Whiteboard: [MemShrink]
Assignee | ||
Comment 5•13 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #4) > (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2) > > Depends on how you define "needed". My ad-hoc instrumentation showed that > > it looked like we're wasting about 10% of the memory allocated here. Using > > a total of 1 MB from the explicit/xpti-working-set reporter we're looking at > > 100 KB wasted for the lifetime of the browser. Not a big contribution, but > > given that it's a two line fix ... > > Ok, that's very reasonable. If you'd included that info in the patch > submission I wouldn't need to have asked :) Well, I didn't measure until you asked ;-). I was just reading the code because I expected it had the same type of fail as PLArena and noticed this.
Comment 6•13 years ago
|
||
PLArena (and JSArena) have the same issue on oversized allocations but I don't plan to do anything about it until profiling tells me it's important.
Assignee | ||
Comment 7•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/1248e3a8a8c8
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in
before you can comment on or make changes to this bug.
Description
•