Last Comment Bug 697933 - Allow HashTables to have a capacity as small as four
: Allow HashTables to have a capacity as small as four
Status: RESOLVED FIXED
[MemShrink]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla10
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-27 22:15 PDT by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2011-10-31 17:43 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.67 KB, patch)
2011-10-27 22:15 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
n.nethercote: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2011-10-27 22:15:59 PDT
Created attachment 570187 [details] [diff] [review]
patch

With the patch from bug 697931 applied, we see this distribution of KidsHash sizes:

30880 counts:
( 1)  12576 (40.7%, 40.7%): isHash: 2 / 16
( 2)   4704 (15.2%, 56.0%): isHash: 3 / 16
( 3)   2048 ( 6.6%, 62.6%): isHash: 1096 / 2048
( 4)   1680 ( 5.4%, 68.0%): isHash: 4 / 16
( 5)   1024 ( 3.3%, 71.3%): isHash: 455 / 1024
( 6)   1024 ( 3.3%, 74.7%): isHash: 480 / 1024
( 7)    752 ( 2.4%, 77.1%): isHash: 5 / 16
( 8)    512 ( 1.7%, 78.8%): isHash: 252 / 512
( 9)    512 ( 1.7%, 80.4%): isHash: 6 / 16

(where the data is of the form "isHash: count / capacity").  These numbers are weighted by capacities.

In other words, we have lots of KidsHashes with very few elements, and the current minimum HashTable size of 16 is excessive, esp. as KidsHashes grow rarely.

The attached patch, which luke r+'d via IRC, changes the minimum size from 16 to 4.  If you don't specify a size, however, the initial capacity still defaults to 16.  KidsHash already specifies a minimum size of 2, so this patch shrinks it without any explicit change.

With this patch applied, the "shape-kids" number for Gmail drops from ~300KB to ~185KB.
Comment 1 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-10-30 16:58:17 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9b279e5adbe
Comment 2 Matt Brubeck (:mbrubeck) 2011-10-31 11:21:14 PDT
https://hg.mozilla.org/mozilla-central/rev/d9b279e5adbe
Comment 3 Paul Wright 2011-10-31 17:00:09 PDT
This patch seemed to have made a not-insignificant improvement in the Kraken benchmark on AWFY (64 bit.. 32 bit seems to be affected much less).  I wonder why?

On another note, is there a particular systemic reason that our JS performance on 32 bit is better than 64 bit?  The competition appears to have the opposite performance delta 32 vs 64.
Comment 4 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-10-31 17:43:33 PDT
(In reply to Paul Wright from comment #3)
> This patch seemed to have made a not-insignificant improvement in the Kraken
> benchmark on AWFY (64 bit.. 32 bit seems to be affected much less).  I
> wonder why?

Huh.  It's entirely due to the improvement in gaussian-blur, AFAICT.  I did some profiling and couldn't see anything.  I suspect it's just noise, the patch saved some space but did nothing else and Kraken shouldn't really be affected since its compute-bound.

> On another note, is there a particular systemic reason that our JS
> performance on 32 bit is better than 64 bit?  The competition appears to
> have the opposite performance delta 32 vs 64.

The JITs have had more work done for 32-bit platforms, AIUI.

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