783820 - rm jsdhash

Status: RESOLVED DUPLICATE of bug 884649

(Core :: JavaScript Engine, defect)

Description

[Andrew McCreight [:mccr8]]

The only remaining users are in XPConnect.

The header is also included in /js/src/vm/Xdr.cpp for no reason.

There is also a reference to it in comments in pldhash.h and pldhash.cpp.

Comment 1

[Luke Wagner [:luke]]

\o/

Comment 2

[Nicholas Nethercote [inactive]]

> The only remaining users are in XPConnect.

But there are many of them.  Is the idea to move jsdhash.{cpp,h} somewhere under js/xpconnect/ in order to minimize its exposure (in bug 647367 I recently did a similar thing by moving jshash.{h,cpp} into js/jsd/) or to replace those many uses with something else?

Comment 3

[Andrew McCreight [:mccr8]]

There are a lot of them, but they are mostly very simple and very similar.  I have a stack of 7 or so patches that remove most of them by converting them to nsTHashtable, but it might make more sense to just create a new XPCMap template that wraps an nsTHashtable.

Comment 4

[Andrew McCreight [:mccr8]]

You can see an example of one of these patches in bug 781589.  Very mechanical for the most part.

Comment 5

[Terrence Cole [:terrence]]

I already converted a couple of them to js::HashTable in bug 738868 and bug 772292.  Shame to use two different hashtable types here.

Comment 6

[Andrew McCreight [:mccr8]]

Yeah, I noticed a few were switched already.  I prefer to use a infallible hash table...

Comment 7

[Luke Wagner [:luke]]

It might actually be better to use nsTHashTable in xpconnect, since XPConnect is part of the size-burdened XUL and, since it is just a shim for PL_DHashTable, nsTHashTable generates a lot less code.

Comment 8

[Terrence Cole [:terrence]]

Good points.  I'd be happy to review patches switching the ones I touched.