The old approach was more complex and also had a very bad edge case
with lots of collisions. This approach eliminates that possiblility.
It also makes both reading and writing lookups a little bit faster.
(...and ASSERT_NOT_REACHED => VERIFY_NOT_REACHED)
Since all of these checks are done in release builds as well,
let's rename them to VERIFY to prevent confusion, as everyone is
used to assertions being compiled out in release.
We can introduce a new ASSERT macro that is specifically for debug
checks, but I'm doing this wholesale conversion first since we've
accumulated thousands of these already, and it's not immediately
obvious which ones are suitable for ASSERT.
Problem:
- Using regular functions rather than function templates results in
the arguments not being deduced. This then requires the same
function to be written multiple times and for `move` to be used
rather than `forward`.
Solution:
- Collapse multiple function overloads to a single function template
with a deduced argument. This allows the argument to be a forwarding
reference and bind to either an l-value or r-value and forward the
value.
Problem:
- Many constructors are defined as `{}` rather than using the ` =
default` compiler-provided constructor.
- Some types provide an implicit conversion operator from `nullptr_t`
instead of requiring the caller to default construct. This violates
the C++ Core Guidelines suggestion to declare single-argument
constructors explicit
(https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c46-by-default-declare-single-argument-constructors-explicit).
Solution:
- Change default constructors to use the compiler-provided default
constructor.
- Remove implicit conversion operators from `nullptr_t` and change
usage to enforce type consistency without conversion.
Double the capacity when used+deleted buckets crosses 60% of capacity.
This appears to be a sweet spot for performance based on some ad-hoc
testing with test-js. :^)
Instead of each hash bucket being a SinglyLinkedList, switch to using
closed hashing (open addressing). Buckets are chained together via
double hashing (hashing the hash until we find an unused bucket.)
This greatly reduces malloc traffic, since each added element no longer
allocates a new linked list node.
Appears performance neutral on test-js. Can definitely be tuned and
could use proper management of load factor, etc.
This allows performing an action based on whether something
was actually added or removed without having to look it up
prior to calling set() or remove().
This commit replaces SinglyLinkedListIterator::universal_end() with an
empty SinglyLinkedListIterator(). Piano needs this in order to
initialize a member array of iterators without 84 lines of
universal_end().
This was only used by HashTable::dump() which I used when doing the
first HashTable implementation. Removing this allows us to also remove
most includes of <AK/kstdio.h>.
As suggested by Joshua, this commit adds the 2-clause BSD license as a
comment block to the top of every source file.
For the first pass, I've just added myself for simplicity. I encourage
everyone to add themselves as copyright holders of any file they've
added or modified in some significant way. If I've added myself in
error somewhere, feel free to replace it with the appropriate copyright
holder instead.
Going forward, all new source files should include a license header.
It doesn't seem sane to try to iterate over a HashTable while it's in
the middle of being cleared. Since this might cause strange problems,
this patch adds an assertion if an iterator is constructed during
clear() or rehash() of a HashTable.
Solve this by adding find() overloads to HashTable and SinglyLinkedList
that take a templated functor for comparing the values.
This allows HashMap to call HashTable::find() without having to create
a temporary Entry for use as the table key. :^)
This is prep work for supporting HashMap with NonnullRefPtr<T> as values.
It's currently not possible because many HashTable functions require being
able to default-construct the value type.
Get rid of the ConstIterator classes for these containers and use templated
FooIterator<T, ...> and FooIterator<const T, ...> helpers.
This makes the HashTable class a lot easier to read.
Also run it across the whole tree to get everything using the One True Style.
We don't yet run this in an automated fashion as it's a little slow, but
there is a snippet to do so in makeall.sh.
