Skip to content

Commit 420359e

Browse files
authored
Cleanup of previous commit "simplify relocate" (#99)
1 parent 5ce28bf commit 420359e

File tree

8 files changed

+23
-189
lines changed

8 files changed

+23
-189
lines changed

CHANGELOG.md

+5-2
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1010
- Added B+tree multimap for internal (future) use. [#93](https://github.com/tzaeschke/phtree-cpp/issues/93)
1111

1212
### Changed
13-
- Rewrote relocate(). This should be much cleaner now and slightly faster. [#98](https://github.com/tzaeschke/phtree-cpp/pull/98)
13+
- Rewrote relocate(). This should be much cleaner now and slightly faster.
14+
[#98](https://github.com/tzaeschke/phtree-cpp/pull/98), [#99](https://github.com/tzaeschke/phtree-cpp/pull/99)
15+
1416
- Cleaned up HandleCollision() and key comparison functions. [#97](https://github.com/tzaeschke/phtree-cpp/pull/97)
1517
- Improved performance by eliminating memory indirection for DIM > 3.
1618
This was enabled by referencing "Node" directly in "Entry" which was enabled by
1719
implanting an indirection in array_map. [#96](https://github.com/tzaeschke/phtree-cpp/pull/96)
1820
- Improved performance of window queries by executing them partially as point queries.
1921
This works best for point datasets, and somewhat for box datasets with "include" queries.
2022
There is no benefit for "intersection" queries. [#88](https://github.com/tzaeschke/phtree-cpp/issues/88)
21-
- Improved benchmarks for insert and query to use a more compact format. [#91](https://github.com/tzaeschke/phtree-cpp/pull/91)
23+
- Improved benchmarks for insert and query to use a more compact format.
24+
[#91](https://github.com/tzaeschke/phtree-cpp/pull/91)
2225
- Improved performance of window queries by optimizing calculation of min/max masks.
2326
Improved performance of queries and updates by changing bit-width of min/max masks and
2427
hc_pos_t. [#95](https://github.com/tzaeschke/phtree-cpp/pull/95)

benchmark/update_mm_d_benchmark.cc

+2-5
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ typename std::enable_if<
166166
UpdateEntry(TestMap<SCENARIO, DIM>& tree, std::vector<UpdateOp<DIM>>& updates) {
167167
size_t n = 0;
168168
for (auto& update : updates) {
169-
n += tree.relocate(update.old_, update.new_, update.id_, false);
169+
n += tree.relocate(update.old_, update.new_, update.id_);
170170
}
171171
return n;
172172
}
@@ -177,10 +177,7 @@ typename std::enable_if<SCENARIO == Scenario::MM_SET_RELOCATE_IF, size_t>::type
177177
size_t n = 0;
178178
for (auto& update : updates) {
179179
n += tree.relocate_if(
180-
update.old_,
181-
update.new_,
182-
[&update](const payload_t& v) { return v == update.id_; },
183-
false);
180+
update.old_, update.new_, [&update](const payload_t& v) { return v == update.id_; });
184181
}
185182
return n;
186183
}

include/phtree/common/common.h

-35
Original file line numberDiff line numberDiff line change
@@ -103,21 +103,6 @@ static bit_width_t NumberOfDivergingBits(
103103
return MAX_BIT_WIDTH<SCALAR> - CountLeadingZeros(diff2);
104104
}
105105

106-
//template <dimension_t DIM, typename SCALAR>
107-
//static bit_width_t NumberOfDivergingBits2(
108-
// const PhPoint<DIM, SCALAR>& v1, const PhPoint<DIM, SCALAR>& v2) {
109-
// // write all differences to diff, we just check diff afterwards
110-
// SCALAR diff = 0;
111-
// //bit_mask_t<SCALAR> diff = 0;
112-
// for (dimension_t i = 0; i < DIM; ++i) {
113-
// diff |= v1[i] ^ v2[i];
114-
// }
115-
// bit_mask_t<SCALAR> diff2 = reinterpret_cast<bit_mask_t<SCALAR>&>(diff);
116-
// assert(CountLeadingZeros(diff2) <= MAX_BIT_WIDTH<SCALAR>);
117-
// return MAX_BIT_WIDTH<SCALAR> - CountLeadingZeros(diff2);
118-
//}
119-
120-
121106
template <dimension_t DIM, typename SCALAR>
122107
static bool KeyEquals(
123108
const PhPoint<DIM, SCALAR>& key_a, const PhPoint<DIM, SCALAR>& key_b, bit_width_t ignore_bits) {
@@ -127,26 +112,6 @@ static bool KeyEquals(
127112
}
128113
return diff >> ignore_bits == 0;
129114
}
130-
//template <dimension_t DIM, typename SCALAR>
131-
//static bool KeyEquals0(
132-
// const PhPoint<DIM, SCALAR>& key_a, const PhPoint<DIM, SCALAR>& key_b, SCALAR mask) {
133-
// for (dimension_t i = 0; i < DIM; ++i) {
134-
// if (((key_a[i] ^ key_b[i]) & mask) != 0) {
135-
// return false;
136-
// }
137-
// }
138-
// return true;
139-
//}
140-
//
141-
//template <dimension_t DIM, typename SCALAR>
142-
//static bool KeyEquals1(
143-
// const PhPoint<DIM, SCALAR>& key_a, const PhPoint<DIM, SCALAR>& key_b, SCALAR mask) {
144-
// SCALAR sum = 0;
145-
// for (dimension_t i = 0; i < DIM; ++i) {
146-
// sum |= (key_a[i] ^ key_b[i]);
147-
// }
148-
// return (sum & mask) == 0;
149-
//}
150115

151116
// ************************************************************************
152117
// String helpers

include/phtree/common/flat_sparse_map.h

+5-19
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,15 @@ class sparse_map {
9696
}
9797

9898
template <typename... Args>
99-
auto emplace(Args&&... args) {
100-
return try_emplace_base(std::forward<Args>(args)...);
99+
auto emplace(size_t key, Args&&... args) {
100+
auto iter = lower_bound(key);
101+
return try_emplace_base(iter, key, std::forward<Args>(args)...);
101102
}
102103

103104
template <typename... Args>
104105
auto try_emplace(size_t key, Args&&... args) {
105-
return try_emplace_base(key, std::forward<Args>(args)...);
106+
auto iter = lower_bound(key);
107+
return try_emplace_base(iter, key, std::forward<Args>(args)...);
106108
}
107109

108110
template <typename... Args>
@@ -136,22 +138,6 @@ class sparse_map {
136138
}
137139
}
138140

139-
template <typename... Args>
140-
auto try_emplace_base(KeyT key, Args&&... args) {
141-
auto it = lower_bound(key);
142-
if (it != end() && it->first == key) {
143-
return std::make_pair(it, false);
144-
} else {
145-
auto x = data_.emplace(
146-
it,
147-
std::piecewise_construct,
148-
std::forward_as_tuple(key),
149-
std::forward_as_tuple(std::forward<Args>(args)...));
150-
return std::make_pair(x, true);
151-
}
152-
}
153-
154-
// TODO merge with above
155141
template <typename... Args>
156142
auto try_emplace_base(const iterator& it, KeyT key, Args&&... args) {
157143
if (it != end() && it->first == key) {

include/phtree/v16/entry.h

-5
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,6 @@ class Entry {
177177
kd_key_ = key;
178178
}
179179

180-
void SetValue(T&& value) noexcept {
181-
assert(union_type_ == VALUE);
182-
value_ = std::move(value);
183-
}
184-
185180
void SetNode(NodeT&& node, bit_width_t postfix_len) noexcept {
186181
postfix_len_ = static_cast<std::uint16_t>(postfix_len);
187182
DestroyUnion();

include/phtree/v16/node.h

+2-5
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,9 @@ using EntryMap = typename std::conditional_t<
4747
DIM <= 8,
4848
sparse_map<hc_pos_dim_t<DIM>, Entry>,
4949
b_plus_tree_map<std::uint64_t, Entry, (uint64_t(1) << DIM)>>>;
50-
//template <dimension_t DIM, typename Entry>
51-
//using EntryMap = std::map<hc_pos_dim_t<DIM>, Entry>;
5250

5351
template <dimension_t DIM, typename Entry>
54-
using EntryIterator = typename std::remove_const<decltype(EntryMap<DIM, Entry>().begin())>::type;
52+
using EntryIterator = typename std::remove_const_t<decltype(EntryMap<DIM, Entry>().begin())>;
5553
template <dimension_t DIM, typename Entry>
5654
using EntryIteratorC = decltype(EntryMap<DIM, Entry>().cbegin());
5755

@@ -167,8 +165,7 @@ class Node {
167165
return const_cast<Node&>(*this).Find(key, postfix_len);
168166
}
169167

170-
// TODO rename to lower_bound()
171-
auto FindIter(const KeyT& key, bit_width_t postfix_len, bool& found) {
168+
auto LowerBound(const KeyT& key, bit_width_t postfix_len, bool& found) {
172169
hc_pos_t hc_pos = CalcPosInArray(key, postfix_len);
173170
auto iter = entries_.lower_bound(hc_pos);
174171
found =

include/phtree/v16/phtree_v16.h

+9-117
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ class PhTreeV16 {
278278
* whose second element is a bool that is true if the value was actually relocated.
279279
*/
280280
template <typename PRED>
281-
size_t relocate_if2(const KeyT& old_key, const KeyT& new_key, PRED pred) {
281+
[[deprecated]] size_t relocate_if2(const KeyT& old_key, const KeyT& new_key, PRED pred) {
282282
auto pair = _find_two(old_key, new_key);
283283
auto& iter_old = pair.first;
284284
auto& iter_new = pair.second;
@@ -315,8 +315,6 @@ class PhTreeV16 {
315315
return 1;
316316
}
317317

318-
// TODO is this a memory leak problem?????
319-
320318
/*
321319
* Relocate (move) an entry from one position to another, subject to a predicate.
322320
*
@@ -327,77 +325,6 @@ class PhTreeV16 {
327325
* @return A pair, whose first element points to the possibly relocated value, and
328326
* whose second element is a bool that is true if the value was actually relocated.
329327
*/
330-
// TODO: test work with old relocate_if(). It also work with std::map
331-
// TODO test also FAILS with B-Plus_tree_map; but not with array_map!
332-
// WITHOUT ITERATOR
333-
template <typename PRED>
334-
auto relocate_ifX(const KeyT& old_key, const KeyT& new_key, PRED&& pred) {
335-
bit_width_t n_diverging_bits = NumberOfDivergingBits(old_key, new_key);
336-
337-
EntryT* current_entry = &root_; // An entry.
338-
EntryT* old_node_entry = nullptr; // Node that contains entry to be removed
339-
EntryT* old_node_entry_parent = nullptr; // Parent of the old_node_entry
340-
EntryT* new_node_entry = nullptr; // Node that will contain new entry
341-
// Find node for removal
342-
while (current_entry && current_entry->IsNode()) {
343-
old_node_entry_parent = old_node_entry;
344-
old_node_entry = current_entry;
345-
auto postfix_len = old_node_entry->GetNodePostfixLen();
346-
if (postfix_len + 1 >= n_diverging_bits) {
347-
new_node_entry = old_node_entry;
348-
}
349-
// TODO stop earlier, we are going to have to redo this after insert....
350-
current_entry = current_entry->GetNode().Find(old_key, postfix_len);
351-
}
352-
EntryT* old_entry = current_entry; // Entry to be removed
353-
354-
// Can we stop already?
355-
if (old_entry == nullptr || !pred(old_entry->GetValue())) {
356-
return 0; // old_key not found!
357-
}
358-
359-
// Are the keys equal? Or is the quadrant the same? -> same entry
360-
if (n_diverging_bits == 0 || old_node_entry->GetNodePostfixLen() >= n_diverging_bits) {
361-
old_entry->SetKey(new_key);
362-
return 1;
363-
}
364-
365-
// Find node for insertion
366-
auto new_entry = new_node_entry;
367-
while (new_entry && new_entry->IsNode()) {
368-
new_node_entry = new_entry;
369-
new_entry = new_entry->GetNode().Find(new_key, new_entry->GetNodePostfixLen());
370-
}
371-
if (new_entry != nullptr) {
372-
return 0; // Entry exists!
373-
}
374-
bool is_inserted = false;
375-
// TODO remove "if"
376-
if (new_entry == nullptr) { // TODO use in-node pointer
377-
new_entry = &new_node_entry->GetNode().Emplace(
378-
is_inserted,
379-
new_key,
380-
new_node_entry->GetNodePostfixLen(),
381-
std::move(old_entry->ExtractValue()));
382-
}
383-
384-
// Erase old value. See comments in try_emplace(iterator) for details.
385-
if (old_node_entry_parent == new_node_entry) {
386-
// In this case the old_node_entry may have been invalidated by the previous
387-
// insertion.
388-
old_node_entry = old_node_entry_parent;
389-
}
390-
391-
bool found = false;
392-
while (old_node_entry) {
393-
old_node_entry = old_node_entry->GetNode().Erase(
394-
old_key, old_node_entry, old_node_entry != &root_, found);
395-
}
396-
assert(found);
397-
return 1;
398-
}
399-
400-
// WITH ITERATOR
401328
template <typename PRED>
402329
auto relocate_if(const KeyT& old_key, const KeyT& new_key, PRED&& pred) {
403330
bit_width_t n_diverging_bits = NumberOfDivergingBits(old_key, new_key);
@@ -418,7 +345,7 @@ class PhTreeV16 {
418345
}
419346
// TODO stop earlier, we are going to have to redo this after insert....
420347
bool is_found = false;
421-
iter = current_entry->GetNode().FindIter(old_key, postfix_len, is_found);
348+
iter = current_entry->GetNode().LowerBound(old_key, postfix_len, is_found);
422349
current_entry = is_found ? &iter->second : nullptr;
423350
}
424351
EntryT* old_entry = current_entry; // Entry to be removed
@@ -439,15 +366,13 @@ class PhTreeV16 {
439366
bool is_found = false;
440367
while (new_entry && new_entry->IsNode()) {
441368
new_node_entry = new_entry;
442-
iter = new_entry->GetNode().FindIter(new_key, new_entry->GetNodePostfixLen(), is_found);
369+
iter =
370+
new_entry->GetNode().LowerBound(new_key, new_entry->GetNodePostfixLen(), is_found);
443371
new_entry = is_found ? &iter->second : nullptr;
444372
}
445373
if (is_found) {
446374
return 0; // Entry exists
447375
}
448-
if (new_entry != nullptr) {
449-
return 0; // Entry exists!
450-
}
451376
bool is_inserted = false;
452377
new_entry = &new_node_entry->GetNode().Emplace(
453378
iter,
@@ -463,13 +388,13 @@ class PhTreeV16 {
463388
old_node_entry = old_node_entry_parent;
464389
}
465390

466-
bool found = false;
467-
// TODO use in-node pointer
391+
is_found = false;
392+
// TODO use in-node iterator if possible
468393
while (old_node_entry) {
469394
old_node_entry = old_node_entry->GetNode().Erase(
470-
old_key, old_node_entry, old_node_entry != &root_, found);
395+
old_key, old_node_entry, old_node_entry != &root_, is_found);
471396
}
472-
assert(found);
397+
assert(is_found);
473398
return 1;
474399
}
475400

@@ -525,18 +450,6 @@ class PhTreeV16 {
525450
return std::make_pair(iter1, iter2);
526451
}
527452

528-
// TODO what is different/required for MM:
529-
// - "old" is not always removed
530-
// - "new" may exist, but may not result in collision
531-
// TODO i.e. we need three conditions:
532-
// - pred_move ( {return is_valid(old); } )
533-
// - pred_can_be_moved ( { return destination.emplace() == true; } )
534-
// - pred_remove_old ( { source.erase(); return source.empty(); } )
535-
536-
// TODO
537-
// - relocate(key, key, value) relocates 0 or 1 entries...?
538-
// - relocate_if(key, key) relocates potentially many keys
539-
540453
public:
541454
/*
542455
* Tries to locate two entries that are 'close' to each other.
@@ -555,7 +468,7 @@ class PhTreeV16 {
555468
bit_width_t n_diverging_bits = NumberOfDivergingBits(old_key, new_key);
556469

557470
if (!verify_exists && n_diverging_bits == 0) {
558-
return 1; // TODO COUNT()?
471+
return 1; // We omit calling because that would require looking up the entry...
559472
}
560473

561474
EntryT* new_entry = &root_; // An entry.
@@ -682,7 +595,6 @@ class PhTreeV16 {
682595
return std::make_pair(iter1, iter2);
683596
}
684597

685-
public:
686598
/*
687599
* Iterates over all entries in the tree. The optional filter allows filtering entries and nodes
688600
* (=sub-trees) before returning / traversing them. By default, all entries are returned. Filter
@@ -862,26 +774,6 @@ class PhTreeV16 {
862774
return {parent, entry_iter};
863775
}
864776

865-
std::pair<EntryT*, EntryIteratorC<DIM, EntryT>> find_starting_node(
866-
const KeyT& key1, const KeyT& key2, bit_width_t& max_conflicting_bits) {
867-
auto& prefix = key1;
868-
max_conflicting_bits = NumberOfDivergingBits(key1, key2);
869-
EntryT* parent = &root_;
870-
if (max_conflicting_bits > root_.GetNodePostfixLen()) {
871-
// Abort early if we have no shared prefix in the query
872-
return {&root_, root_.GetNode().Entries().end()};
873-
}
874-
EntryIterator<DIM, EntryT> entry_iter =
875-
root_.GetNode().FindPrefix(prefix, max_conflicting_bits, root_.GetNodePostfixLen());
876-
while (entry_iter != parent->GetNode().Entries().end() && entry_iter->second.IsNode() &&
877-
entry_iter->second.GetNodePostfixLen() >= max_conflicting_bits) {
878-
parent = &entry_iter->second;
879-
entry_iter = parent->GetNode().FindPrefix(
880-
prefix, max_conflicting_bits, parent->GetNodePostfixLen());
881-
}
882-
return {parent, entry_iter};
883-
}
884-
885777
size_t num_entries_;
886778
// Contract: root_ contains a Node with 0 or more entries. The root node is the only Node
887779
// that is allowed to have less than two entries.

test/phtree_test.cc

-1
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,6 @@ TEST(PhTreeTest, TestUpdateWithRelocateIf) {
668668
TestPoint<dim> pNew{pOld[0] + delta, pOld[1] + delta, pOld[2] + delta};
669669
if ((delta > 0.0 && tree.find(pNew) != tree.end()) || (i % 2 != 0)) {
670670
// Skip this, there is already another entry
671-
std::cout << "x = " << x << " i=" << i << std::endl;
672671
ASSERT_EQ(0, tree.relocate_if(pOld, pNew, pred));
673672
} else {
674673
ASSERT_EQ(1, tree.relocate_if(pOld, pNew, pred));

0 commit comments

Comments
 (0)