Closed Bug 722689 Opened 10 years ago Closed 10 years ago

Slay nsCheapSets.h

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file)

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.
Attached patch patchSplinter Review
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+
Whiteboard: [autoland-try]
Whiteboard: [autoland-try] → [autoland-in-queue]
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
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
Whiteboard: [autoland-in-queue]
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/34e48e4d676f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.