Last Comment Bug 738868 - Use js::HashTable instead of JSDHashTable in XPConnect
: Use js::HashTable instead of JSDHashTable in XPConnect
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Terrence Cole [:terrence]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 721557
  Show dependency treegraph
 
Reported: 2012-03-23 17:40 PDT by Terrence Cole [:terrence]
Modified: 2012-03-30 13:01 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0: reimplement JSObject2JSObjectMap (9.33 KB, patch)
2012-03-23 17:40 PDT, Terrence Cole [:terrence]
mrbkap: review+
Details | Diff | Splinter Review
v1: Updated with review feedback. (9.54 KB, patch)
2012-03-27 10:42 PDT, Terrence Cole [:terrence]
terrence.d.cole: review+
Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-03-23 17:40:03 PDT
Created attachment 608931 [details] [diff] [review]
v0: reimplement JSObject2JSObjectMap

The new JS HashTable is easier to use, easier to understand, and, most importantly, we are building tools to make the new hash tables work well with a moving GC. The JSDHashTables in XPConnect are the last in the tree and it's time for them to go.

This is my first patch against XPConnect, so I've limited myself ot a single table: the JSObject2JSObjectMap used for waivers.  The diffstat for this patch is:

4 files changed, 40 insertions(+), 103 deletions(-)

I think it's a good start.
Comment 1 Blake Kaplan (:mrbkap) 2012-03-27 07:11:08 PDT
Comment on attachment 608931 [details] [diff] [review]
v0: reimplement JSObject2JSObjectMap

Review of attachment 608931 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCMaps.h
@@ +786,5 @@
>  
> +    void Sweep() {
> +        for (Map::Enum e(mTable); !e.empty(); e.popFront())
> +            if (JS_IsAboutToBeFinalized(e.front().key) || JS_IsAboutToBeFinalized(e.front().value))
> +                e.popFront();

Nit: Braces around the for loop's body.

@@ +791,3 @@
>      }
>  
> +    void Reparent(JSContext *aCx, JSObject *aNewInner) {

This function lost a comment that should have moved with it.
Comment 2 Terrence Cole [:terrence] 2012-03-27 10:42:18 PDT
Created attachment 609778 [details] [diff] [review]
v1: Updated with review feedback.

https://tbpl.mozilla.org/?tree=Try&rev=8945819e2b7a
Comment 3 Terrence Cole [:terrence] 2012-03-29 18:09:01 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/61b214270824
Comment 4 Ed Morley [:emorley] 2012-03-30 13:01:22 PDT
https://hg.mozilla.org/mozilla-central/rev/61b214270824

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