Closed Bug 1057928 Opened 5 years ago Closed 5 years ago

Use |ops| instead of other fields to indicate PL_DHashTable liveness

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(2 files)

SpanningCellSorter has a hash table. It zeroes the |entryCount| field to
indicate when the hash table is not live. This goes against the standard idiom
of using the |ops| field for this purpose, and blocks my goal of making
|entryCount| private (see bug 1057914).
I think at the time I wrote this I was trying to change the idiom from using null-ops to using 0-entryCount, although I don't remember why.
Attachment #8478080 - Flags: review?(roc) → review+
Thanks for the fast review, dbaron!
Sure.

I didn't see any clues in bug 363329 as to why I was using entryCount.
Blocks: 1057914
Blocks: 1057912
No longer blocks: 1057914
Summary: In SpanningCellSorter, use |ops| instead of |entryCount| to indicate table liveness → Use |ops| instead of |entryCount| to indicate PL_DHashTable liveness
I found another case to convert. (Nb: if you're wondering, there are dozens and
dozens of places that use |ops| to indicate liveness, as opposed to the two
that use |entryStore|.)
Attachment #8478084 - Flags: review?(dbaron)
Comment on attachment 8478084 [details] [diff] [review]
(part 2) - In nsTHashtable, use |ops| instead of |entryCount| to indicate table liveness

You also need to adjust the assertions in nsBaseHashtable::EnumerateRead and nsBaseHashtable::Enumerate.

You should also fix the commit message to refer to entrySize instead of entryCount, since that's what was being used here.

r=dbaron with that
Attachment #8478084 - Flags: review?(dbaron) → review+
Summary: Use |ops| instead of |entryCount| to indicate PL_DHashTable liveness → Use |ops| instead of other fields to indicate PL_DHashTable liveness
https://hg.mozilla.org/mozilla-central/rev/c4d86cfb8fa2
https://hg.mozilla.org/mozilla-central/rev/a17805b07d4f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.