Last Comment Bug 840809 - Shrink initial size of some per-compartment tables
: Shrink initial size of some per-compartment tables
Status: RESOLVED FIXED
[js:t][MemShrink]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla21
Assigned To: Nicholas Nethercote [:njn]
: general
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 681201
  Show dependency treegraph
 
Reported: 2013-02-12 17:01 PST by Nicholas Nethercote [:njn]
Modified: 2013-02-23 09:55 PST (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
(part 1) - Fix a comment and rename a constant in HashTable.h. (3.06 KB, patch)
2013-02-12 17:09 PST, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review
(part 2) - Measure RegExpCompartment::inUse_. (943 bytes, patch)
2013-02-12 17:57 PST, Nicholas Nethercote [:njn]
sstangl: review+
Details | Diff | Splinter Review
(part 3) - Shrink initial size of some per-compartment tables. (1.75 KB, patch)
2013-02-12 20:09 PST, Nicholas Nethercote [:njn]
wmccloskey: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2013-02-12 17:01:22 PST
We have lots of compartments:  150+ at start-up, and hundreds more in a session with lots of tabs.

Each compartment has several hash tables.  The default capacity of 32 is higher than necessary for some of these.  Just by changing the defaults we'll be able to save ~0.5 MB on start-up on a 64-bit build.
Comment 1 Nicholas Nethercote [:njn] 2013-02-12 17:09:13 PST
Created attachment 713200 [details] [diff] [review]
(part 1) - Fix a comment and rename a constant in HashTable.h.

The default capacity of a js::HashTable is 32, not 16 as I thought.  I wonder
if it should be 16, which would match pldhash.
Comment 2 Nicholas Nethercote [:njn] 2013-02-12 17:57:22 PST
Created attachment 713229 [details] [diff] [review]
(part 2) - Measure RegExpCompartment::inUse_.
Comment 3 Nicholas Nethercote [:njn] 2013-02-12 20:09:12 PST
Created attachment 713259 [details] [diff] [review]
(part 3) - Shrink initial size of some per-compartment tables.

In this patch I made four of the tables initialize to the minimum size (which
is a capacity of 4).  I measured the improvements on Linux64 at start-up (with
~150 compartments present) and after running MemBench50 (~500 compartments
present).

* debuggees.  In normal execution all the tables are empty.  Changing to the
  minimum size table saves about 68 KiB at start-up and 217 KiB on MemBench50.

* RegExpCompartment::map_.  70--98% of them fit in the minimum size table.
  RegExpCompartment::inUse_. 97--98% of them fit in the minimum size table.
  Together that saves 201 KiB at start-up and 523 KiB on MemBench50.

* crossCompartmentWrappers.  

  - At start-up, 80% fit in the current default (cap=32).  After shrinking the
    start size:  39.2% fit in a capacity of 4; another 7% within a capacity of
    8, another 17% in a capacity of 16.  

  - In MemBench50, 80% fit in the current default (cap=32).  After shrinking,
    52% fit in 4, another 11% in 8, and another 16% in 16.

  This saves 143 KiB at start-up and 236 KiB on MemBench50.

Altogether, the savings are 412 KiB at start-up and 976 KiB on MemBench50.  Not
bad for a trivial patch.
Comment 6 Jonathan Watt [:jwatt] 2013-02-19 04:01:38 PST
For: http://hg.mozilla.org/mozilla-central/annotate/tip/content/svg/content/src/nsSVGFilters.cpp

Why is blame showing virtually every line as having last been touched by rev 121949, attributed to "(part 1) - Fix a comment and rename a constant in HashTable.h"?
Comment 7 Nicholas Nethercote [:njn] 2013-02-19 04:23:53 PST
> Why is blame showing virtually every line as having last been touched by rev
> 121949, attributed to "(part 1) - Fix a comment and rename a constant in
> HashTable.h"?

That's weird.  Each links in comment 4 and comment 5 each name nsSVGFilter.cpp as being modified by the patch, but don't show any diff.  And when doing |hg log -p| on the relevant revisions that file doesn't show up.

I certainly didn't do anything with that file.  Did it change name recently, or something like that?  Seems like an hg oddity, and one that's hard to do anything about now :/
Comment 8 Jonathan Watt [:jwatt] 2013-02-19 06:02:24 PST
The file has always had that name and been at that location.

I filed a bug over in Mercurial's bugzilla on this:

http://bz.selenic.com/show_bug.cgi?id=3833

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