Skip to content

C
capnproto
Commit 5c4ed12c authored by Kenton Varda's avatar Kenton Varda
Browse files
Options

Refactor: Add keyForRow() callback so that other callbacks don't need to be overloaded for rows. 

See map.h changes to see why this makes this cleaner.
parent 9b20a942
Hide whitespace changes
Inline Side-by-side
Showing with 75 additions and 61 deletions
c++/src/kj/map.h
View file @ 5c4ed12c
......@@ -105,6 +105,9 @@ public:
private:
class Callbacks {
public:
inline const Key& keyForRow(const Entry& entry) const { return entry.key; }
inline Key& keyForRow(Entry& entry) const { return entry.key; }
template <typename KeyLike>
inline bool matches(Entry& e, KeyLike&& key) const {
return e.key == key;
......@@ -117,17 +120,6 @@ private:
inline bool hashCode(KeyLike&& key) const {
return kj::hashCode(key);
}
inline bool matches(Entry& e, Entry& e2) const { return e.key == e2.key; }
inline bool matches(const Entry& e, Entry& e2) const { return e.key == e2.key; }
inline bool matches(Entry& e, const Entry& e2) const { return e.key == e2.key; }
inline bool matches(const Entry& e, const Entry& e2) const { return e.key == e2.key; }
inline bool hashCode(Entry& entry) const {
return kj::hashCode(entry.key);
}
inline bool hashCode(const Entry& entry) const {
return kj::hashCode(entry.key);
}
};
kj::Table<Entry, HashIndex<Callbacks>> table;
......@@ -212,6 +204,9 @@ public:
private:
class Callbacks {
public:
inline const Key& keyForRow(const Entry& entry) const { return entry.key; }
inline Key& keyForRow(Entry& entry) const { return entry.key; }
template <typename KeyLike>
inline bool matches(Entry& e, KeyLike&& key) const {
return e.key == key;
......@@ -228,15 +223,6 @@ private:
inline bool isBefore(const Entry& e, KeyLike&& key) const {
return e.key < key;
}
inline bool matches(Entry& e, Entry& e2) const { return e.key == e2.key; }
inline bool matches(Entry& e, const Entry& e2) const { return e.key == e2.key; }
inline bool matches(const Entry& e, Entry& e2) const { return e.key == e2.key; }
inline bool matches(const Entry& e, const Entry& e2) const { return e.key == e2.key; }
inline bool isBefore(Entry& e, Entry& e2) const { return e.key < e2.key; }
inline bool isBefore(Entry& e, const Entry& e2) const { return e.key < e2.key; }
inline bool isBefore(const Entry& e, Entry& e2) const { return e.key < e2.key; }
inline bool isBefore(const Entry& e, const Entry& e2) const { return e.key < e2.key; }
};
kj::Table<Entry, TreeIndex<Callbacks>> table;
......@@ -246,6 +232,9 @@ namespace _ { // private
class HashSetCallbacks {
public:
template <typename Row>
inline Row& keyForRow(Row& row) const { return row; }
template <typename T, typename U>
inline bool matches(T& a, U& b) const { return a == b; }
template <typename KeyLike>
......@@ -256,6 +245,9 @@ public:
class TreeSetCallbacks {
public:
template <typename Row>
inline Row& keyForRow(Row& row) const { return row; }
template <typename T, typename U>
inline bool matches(T& a, U& b) const { return a == b; }
template <typename T, typename U>
......
c++/src/kj/table-test.c++
View file @ 5c4ed12c
......@@ -55,6 +55,8 @@ KJ_TEST("_::tryReserveSize() works") {
class StringHasher {
public:
StringPtr keyForRow(StringPtr s) const { return s; }
bool matches(StringPtr a, StringPtr b) const {
return a == b;
}
......@@ -185,6 +187,8 @@ class BadHasher {
// String hash that always returns the same hash code. This should not affect correctness, only
// performance.
public:
StringPtr keyForRow(StringPtr s) const { return s; }
bool matches(StringPtr a, StringPtr b) const {
return a == b;
}
......@@ -273,12 +277,7 @@ struct SiPair {
class SiPairStringHasher {
public:
bool matches(SiPair a, SiPair b) const {
return a.str == b.str;
}
uint hashCode(SiPair pair) const {
return inner.hashCode(pair.str);
}
StringPtr keyForRow(SiPair s) const { return s.str; }
bool matches(SiPair a, StringPtr b) const {
return a.str == b;
......@@ -293,12 +292,7 @@ private:
class SiPairIntHasher {
public:
bool matches(SiPair a, SiPair b) const {
return a.i == b.i;
}
uint hashCode(SiPair pair) const {
return pair.i;
}
uint keyForRow(SiPair s) const { return s.i; }
bool matches(SiPair a, uint b) const {
return a.i == b;
......@@ -376,6 +370,8 @@ KJ_TEST("double-index table") {
class UintHasher {
public:
uint keyForRow(uint i) const { return i; }
bool matches(uint a, uint b) const {
return a == b;
}
......@@ -628,6 +624,8 @@ KJ_TEST("B-tree internals") {
class StringCompare {
public:
StringPtr keyForRow(StringPtr s) const { return s; }
bool isBefore(StringPtr a, StringPtr b) const {
return a < b;
}
......@@ -774,6 +772,8 @@ KJ_TEST("simple tree table") {
class UintCompare {
public:
uint keyForRow(uint i) const { return i; }
bool isBefore(uint a, uint b) const {
return a < b;
}
......
c++/src/kj/table.h
View file @ 5c4ed12c
......@@ -70,16 +70,16 @@ class Table {
//
// Each index is a class that looks like:
//
// template <typename Key>
// class Index {
// public:
// void reserve(size_t size);
// // Called when Table::reserve() is called.
//
// SearchParam& keyForRow(const Row& row) const;
// // Given a row, return a value appropriate to pass as SearchParams to the other functions.
//
// // In all function calls below, `SearchPrams` refers to whatever parameters the index
// // supports for looking up a row in the table. At the very least, the table row type
// // itself must be supported. However, most indexes will also want to support some sort
// // of "key" type that is not a whole row.
// // supports for looking up a row in the table.
//
// template <typename... SearchParams>
// kj::Maybe<size_t> insert(kj::ArrayPtr<const Row> table, size_t pos, SearchParams&&...);
......@@ -293,18 +293,22 @@ class HashIndex;
//
// class Callbacks {
// public:
// bool matches(const Row&, const Row&) const;
// // Returns true if the two rows are matching, for the purpose of this index.
// // In this interface, `SearchParams...` means whatever parameters you want to support in
// // a call to table.find(...). By overloading the calls to support various inputs, you can
// // affect what table.find(...) accepts.
//
// SearchParam& keyForRow(const Row& row);
// // Given a row of the table, return the SearchParams that might be passed to the other
// // methods to match this row.
//
// uint hashCode(const Row&) const;
// // Computes the hash code of the given row. Matching rows (as determined by match()) must
// // have the same hash code. Non-matching rows should have different hash codes, to the
// // maximum extent possible. Non-matching rows with the same hash code hurt performance.
// bool matches(const Row&, SearchParams&&...) const;
// // Returns true if the the row on the left matches thes search params on the right.
//
// bool matches(const Row&, ...) const;
// uint hashCode(...) const;
// // If you wish to support find(...) with parameters other than `const Row&`, you can do
// // so by implementing overloads of hashCode() and match() with those parameters.
// uint hashCode(SearchParams&&...) const;
// // Computes the hash code of the given search params. Matching rows (as determined by
// // matches()) must have the same hash code. Non-matching rows should have different hash
// // codes, to the maximum extent possible. Non-matching rows with the same hash code hurt
// // performance.
// };
//
// If your `Callbacks` type has dynamic state, you may pass its constructor parameters as the
......@@ -320,19 +324,19 @@ class TreeIndex;
//
// class Callbacks {
// public:
// bool isBefore(const Row&, const Row&) const;
// // Returns true if the row on the left comes before the row on the right.
// // In this interface, `SearchParams...` means whatever parameters you want to support in
// // a call to table.find(...). By overloading the calls to support various inputs, you can
// // affect what table.find(...) accepts.
//
// SearchParam& keyForRow(const Row& row);
// // Given a row of the table, return the SearchParams that might be passed to the other
// // methods to match this row.
//
// bool matches(const Row&, const Row&) const;
// // Returns true if the rows "match".
// //
// // This could be computed by checking whether isBefore() returns false regardless of the
// // order of the parameters, but often equality is somewhat faster to check.
// bool isBefore(const Row&, SearchParams&&...) const;
// // Returns true if the row on the left comes before the search params on the right.
//
// bool isBefore(const Row&, ...) const;
// bool matches(const Row&, ...) const;
// // If you wish to support find(...) with parameters other than `const Row&`, you can do
// // so by implementing overloads of isBefore() and isAfter() with those parameters.
// bool matches(const Row&, SearchParams&&...) const;
// // Returns true if the row "matches" the search params.
// };
// =======================================================================================
......@@ -456,24 +460,29 @@ public:
if (skip == index) {
return Impl<index + 1>::insert(table, pos, row, skip);
}
KJ_IF_MAYBE(existing, get<index>(table.indexes).insert(table.rows.asPtr(), pos, row)) {
auto& indexObj = get<index>(table.indexes);
KJ_IF_MAYBE(existing, indexObj.insert(table.rows.asPtr(), pos, indexObj.keyForRow(row))) {
return *existing;
}
bool success = false;
KJ_DEFER(if (!success) { get<index>(table.indexes).erase(table.rows.asPtr(), pos, row); });
KJ_DEFER(if (!success) {
indexObj.erase(table.rows.asPtr(), pos, indexObj.keyForRow(row));
});
auto result = Impl<index + 1>::insert(table, pos, row, skip);
success = result == nullptr;
return result;
}
static void erase(Table<Row, Indexes...>& table, size_t pos, Row& row) {
get<index>(table.indexes).erase(table.rows.asPtr(), pos, row);
auto& indexObj = get<index>(table.indexes);
indexObj.erase(table.rows.asPtr(), pos, indexObj.keyForRow(row));
Impl<index + 1>::erase(table, pos, row);
}
static void move(Table<Row, Indexes...>& table, size_t oldPos, size_t newPos, Row& row) {
get<index>(table.indexes).move(table.rows.asPtr(), oldPos, newPos, row);
auto& indexObj = get<index>(table.indexes);
indexObj.move(table.rows.asPtr(), oldPos, newPos, indexObj.keyForRow(row));
Impl<index + 1>::move(table, oldPos, newPos, row);
}
};
......@@ -872,6 +881,11 @@ public:
memset(buckets.begin(), 0, buckets.asBytes().size());
}
template <typename Row>
decltype(auto) keyForRow(Row&& row) const {
return cb.keyForRow(kj::fwd<Row>(row));
}
template <typename Row, typename... Params>
kj::Maybe<size_t> insert(kj::ArrayPtr<Row> table, size_t pos, Params&&... params) {
if (buckets.size() * 2 < (table.size() + 1 + erasedCount) * 3) {
......@@ -1411,6 +1425,11 @@ public:
inline auto begin() const { return impl.begin(); }
inline auto end() const { return impl.end(); }
template <typename Row>
decltype(auto) keyForRow(Row&& row) const {
return cb.keyForRow(kj::fwd<Row>(row));
}
template <typename Row, typename... Params>
kj::Maybe<size_t> insert(kj::ArrayPtr<Row> table, size_t pos, Params&&... params) {
auto iter = impl.insert(searchKey(table, params...));
......@@ -1537,6 +1556,9 @@ public:
uint pos;
};
template <typename Row>
Row& keyForRow(Row& row) const { return row; }
void reserve(size_t size);
void clear();
inline Iterator begin() const { return Iterator(links, links[0].next); }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment