Slay nsCheapSets.h

RESOLVED FIXED in mozilla13

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
The pointer twiddling bits in here are just gross, and it uses ns{String,Int32}HashSet, which need to die per bug 700659.  Something like:

template<class EntryType>
class nsCheapSet
{
private:
  union {
    nsTHashtable<EntryType> *table;
    EntryType::KeyType single;
  } u;
  enum {
    ZERO,
    ONE,
    MANY
  } state;
public:
  /* CheapSet API here */
};

typedef nsCheapSet<nsStringHashKey> nsStringCheapSet;
typedef nsCheapSet<nsUint32HashKey> nsInt32CheapSet;

or somesuch.  The above is a little more expensive than pointer twiddling (in both time and space), but it is more correct.  Given that we have one user of the CheapSets interface, I'm not too worried about the minor ill effects.
(Assignee)

Comment 1

5 years ago
Created attachment 596150 [details] [diff] [review]
patch

Here's an initial patch.  I suppose I have to get approval elsewhere for the content bits, but let's focus on the xpcom bits for now.  WDYT about this approach?
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Attachment #596150 - Flags: review?(benjamin)
Attachment #596150 - Flags: review?(benjamin) → review+
(Assignee)

Updated

5 years ago
Whiteboard: [autoland-try]

Updated

5 years ago
Whiteboard: [autoland-try] → [autoland-in-queue]

Comment 2

5 years ago
Autoland Patchset:
	Patches: 596150
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=8a3a88481bb7
Try run started, revision 8a3a88481bb7. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=8a3a88481bb7

Comment 3

5 years ago
Try run for 8a3a88481bb7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=8a3a88481bb7
Results (out of 216 total builds):
    exception: 3
    success: 173
    warnings: 25
    failure: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-8a3a88481bb7

Updated

5 years ago
Whiteboard: [autoland-in-queue]
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/34e48e4d676f
Keywords: checkin-needed
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/34e48e4d676f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.