Make it easier to enable and disable hashtable assertions.
Categories
(Core :: XPCOM, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox72 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
I've been poking at the places code, and there is scary code like https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/toolkit/components/places/History.cpp#2170 and such that I'm refactoring.
I'm moving them to use more modern hash tables and iterators, and it'd be nice if we could see if this bug still exists and if so what's the root cause more easily (like enabling it when DIAGNOSTIC_ASSERTs are enabled).
I want to land a few patches to make this easier, for now. And will measure the perf impact of enabling them on all hashtables and find a way to opt-into them otherwise.
Assignee | ||
Comment 1•5 years ago
|
||
I want to maybe enable some of these checks in DIAGNOSTIC_ASSERT builds.
The whole type is compiled out in release builds at the moment.
Assignee | ||
Comment 2•5 years ago
|
||
Default constructors of members run if not specified there, and these ifdefs
are ugly.
Assignee | ||
Comment 3•5 years ago
|
||
Put them behind a MOZ_HASH_TABLE_CHECKS_ENABLED define, which right now is only
defined in DEBUG builds, preserving behavior.
MakeImmutable becomes an empty inline function when disabled, which should be
zero-cost.
Assignee | ||
Comment 4•5 years ago
|
||
I'm pretty sure they can be relaxed, as they don't guard any other memory, and
this should reduce their cost.
Also remove useless mutable keywords while at it. The checker is marked as
mutable itself.
Assignee | ||
Comment 5•5 years ago
|
||
It's a bool, should use a bool.
Updated•5 years ago
|
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/761eaa4c4f63 Make hashtable checker use MOZ_RELEASE_ASSERT. r=froydnj https://hg.mozilla.org/integration/autoland/rev/c6cd0b11f18d Remove some useless ifdefs. r=froydnj https://hg.mozilla.org/integration/autoland/rev/25ded18e0a55 Make it easy to switch on and off these assertions in different build configurations. r=froydnj https://hg.mozilla.org/integration/autoland/rev/7a0d7c5c5fad Use an atomic bool for Checker::mIsWritable. r=froydnj
Comment 7•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/761eaa4c4f63
https://hg.mozilla.org/mozilla-central/rev/c6cd0b11f18d
https://hg.mozilla.org/mozilla-central/rev/25ded18e0a55
https://hg.mozilla.org/mozilla-central/rev/7a0d7c5c5fad
Description
•