Skip to content

Commit 468717f

Browse files
danpoedanpoe
committed
Make the shared pointers and write_* methods of the sharing nodes protected
The data member and the write_* methods of sharing_node_innert and sharing_node_leaft are made protected and existing external callers are refactored to not use write_* directly.
1 parent 3800284 commit 468717f

File tree

4 files changed

+117
-80
lines changed

4 files changed

+117
-80
lines changed

‎src/util/sharing_map.h

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ class sharing_mapt
350350

351351
std::size_t count_unmarked_nodes(
352352
bool leafs_only,
353-
std::set<void *> &marked,
353+
std::set<const void *> &marked,
354354
bool mark = true) const;
355355

356356
// dummy element returned when no element was found
@@ -423,8 +423,10 @@ ::iterate(
423423
}
424424

425425
SHARING_MAPT(std::size_t)
426-
::count_unmarked_nodes(bool leafs_only, std::set<void *> &marked, bool mark)
427-
const
426+
::count_unmarked_nodes(
427+
bool leafs_only,
428+
std::set<const void *> &marked,
429+
bool mark) const
428430
{
429431
if(empty())
430432
return 0;
@@ -447,8 +449,10 @@ ::count_unmarked_nodes(bool leafs_only, std::set<void *> &marked, bool mark)
447449

448450
// internal node or container node
449451
const innert *ip = static_cast<const innert *>(bp);
450-
const unsigned use_count = ip->data.use_count();
451-
void *raw_ptr = ip->data.get();
452+
const unsigned use_count = ip->use_count();
453+
const void *raw_ptr = ip->is_internal()
454+
? (const void *)&ip->read_internal()
455+
: (const void *)&ip->read_container();
452456

453457
if(use_count >= 2)
454458
{
@@ -488,8 +492,8 @@ ::count_unmarked_nodes(bool leafs_only, std::set<void *> &marked, bool mark)
488492

489493
for(const auto &l : ll)
490494
{
491-
const unsigned leaf_use_count = l.data.use_count();
492-
void *leaf_raw_ptr = l.data.get();
495+
const unsigned leaf_use_count = l.use_count();
496+
const void *leaf_raw_ptr = &l.read();
493497

494498
if(leaf_use_count >= 2)
495499
{
@@ -528,7 +532,7 @@ ::get_sharing_stats(
528532
Iterator end,
529533
std::function<sharing_mapt &(const Iterator)> f)
530534
{
531-
std::set<void *> marked;
535+
std::set<const void *> marked;
532536
sharing_map_statst sms;
533537

534538
// We do a separate pass over the tree for each statistic. This is not very

‎src/util/sharing_node.h

Lines changed: 72 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ class sharing_node_baset
105105
SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
106106
{
107107
public:
108+
typedef small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS> datat;
109+
typedef typename datat::use_countt use_countt;
110+
108111
typedef d_internalt<SN_TYPE_ARGS> d_it;
109112
typedef d_containert<SN_TYPE_ARGS> d_ct;
110113

@@ -134,6 +137,11 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
134137
return data == other.data;
135138
}
136139

140+
use_countt use_count() const
141+
{
142+
return data.use_count();
143+
}
144+
137145
void swap(sharing_node_innert &other)
138146
{
139147
data.swap(other.data);
@@ -151,45 +159,13 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
151159
return data.is_derived_v();
152160
}
153161

154-
d_it &write_internal()
155-
{
156-
if(!data)
157-
{
158-
data = make_shared_derived_u<SN_PTR_TYPE_ARGS>();
159-
}
160-
else if(data.use_count() > 1)
161-
{
162-
data = make_shared_derived_u<SN_PTR_TYPE_ARGS>(*data.get_derived_u());
163-
}
164-
165-
SN_ASSERT(data.use_count() == 1);
166-
167-
return *data.get_derived_u();
168-
}
169-
170162
const d_it &read_internal() const
171163
{
172164
SN_ASSERT(!empty());
173165

174166
return *data.get_derived_u();
175167
}
176168

177-
d_ct &write_container()
178-
{
179-
if(!data)
180-
{
181-
data = make_shared_derived_v<SN_PTR_TYPE_ARGS>();
182-
}
183-
else if(data.use_count() > 1)
184-
{
185-
data = make_shared_derived_v<SN_PTR_TYPE_ARGS>(*data.get_derived_v());
186-
}
187-
188-
SN_ASSERT(data.use_count() == 1);
189-
190-
return *data.get_derived_v();
191-
}
192-
193169
const d_ct &read_container() const
194170
{
195171
SN_ASSERT(!empty());
@@ -338,7 +314,40 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
338314
SN_ASSERT_USE(r, r == 1);
339315
}
340316

341-
small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS> data;
317+
protected:
318+
d_it &write_internal()
319+
{
320+
if(!data)
321+
{
322+
data = make_shared_derived_u<SN_PTR_TYPE_ARGS>();
323+
}
324+
else if(data.use_count() > 1)
325+
{
326+
data = make_shared_derived_u<SN_PTR_TYPE_ARGS>(*data.get_derived_u());
327+
}
328+
329+
SN_ASSERT(data.use_count() == 1);
330+
331+
return *data.get_derived_u();
332+
}
333+
334+
d_ct &write_container()
335+
{
336+
if(!data)
337+
{
338+
data = make_shared_derived_v<SN_PTR_TYPE_ARGS>();
339+
}
340+
else if(data.use_count() > 1)
341+
{
342+
data = make_shared_derived_v<SN_PTR_TYPE_ARGS>(*data.get_derived_v());
343+
}
344+
345+
SN_ASSERT(data.use_count() == 1);
346+
347+
return *data.get_derived_v();
348+
}
349+
350+
datat data;
342351
};
343352

344353
// Leafs
@@ -357,6 +366,9 @@ SN_TYPE_PAR_DECL class d_leaft : public small_shared_pointeet<unsigned>
357366
SN_TYPE_PAR_DEF class sharing_node_leaft : public sharing_node_baset
358367
{
359368
public:
369+
typedef small_shared_ptrt<SN_PTR_TYPE_ARG> datat;
370+
typedef decltype(datat().use_count()) use_countt;
371+
360372
typedef d_leaft<SN_TYPE_ARGS> d_lt;
361373

362374
sharing_node_leaft(const keyT &k, const valueT &v)
@@ -392,30 +404,14 @@ SN_TYPE_PAR_DEF class sharing_node_leaft : public sharing_node_baset
392404
return data == other.data;
393405
}
394406

395-
void swap(sharing_node_leaft &other)
396-
{
397-
data.swap(other.data);
398-
}
399-
400-
d_lt &write()
407+
use_countt use_count() const
401408
{
402-
if(!data)
403-
{
404-
data = make_small_shared_ptr<d_lt>();
405-
}
406-
else if(data.use_count() > 1)
407-
{
408-
data = make_small_shared_ptr<d_lt>(*data);
409-
}
410-
411-
SN_ASSERT(data.use_count() == 1);
412-
413-
return *data;
409+
return data.use_count();
414410
}
415411

416-
const d_lt &read() const
412+
void swap(sharing_node_leaft &other)
417413
{
418-
return *data;
414+
data.swap(other.data);
419415
}
420416

421417
// Accessors
@@ -445,7 +441,29 @@ SN_TYPE_PAR_DEF class sharing_node_leaft : public sharing_node_baset
445441
return write().v;
446442
}
447443

448-
small_shared_ptrt<SN_PTR_TYPE_ARG> data;
444+
const d_lt &read() const
445+
{
446+
return *data;
447+
}
448+
449+
protected:
450+
d_lt &write()
451+
{
452+
if(!data)
453+
{
454+
data = make_small_shared_ptr<d_lt>();
455+
}
456+
else if(data.use_count() > 1)
457+
{
458+
data = make_small_shared_ptr<d_lt>(*data);
459+
}
460+
461+
SN_ASSERT(data.use_count() == 1);
462+
463+
return *data;
464+
}
465+
466+
datat data;
449467
};
450468

451469
#endif

‎unit/util/sharing_map.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ void sharing_map_sharing_stats_test()
385385
{
386386
SECTION("count nodes")
387387
{
388-
std::set<void *> marked;
388+
std::set<const void *> marked;
389389
smt sm;
390390
int count = 0;
391391

@@ -410,7 +410,7 @@ void sharing_map_sharing_stats_test()
410410

411411
SECTION("marking")
412412
{
413-
std::set<void *> marked;
413+
std::set<const void *> marked;
414414
smt sm;
415415

416416
fill(sm);

‎unit/util/sharing_node.cpp

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,37 @@
77
#include <testing-utils/use_catch.h>
88
#include <util/sharing_node.h>
99

10-
void sharing_node_test()
10+
class leaft : public sharing_node_leaft<int, int>
11+
{
12+
public:
13+
leaft(const int &a, const int &b) : sharing_node_leaft<int, int>(a, b)
14+
{
15+
}
16+
friend void sharing_node_internals_test();
17+
};
18+
19+
void sharing_node_internals_test()
20+
{
21+
SECTION("Leaf test")
22+
{
23+
// Detaching
24+
{
25+
leaft leaf(1, 2);
26+
27+
auto p = leaf.data.get();
28+
leaf.write();
29+
30+
REQUIRE(leaf.data.get() == p);
31+
}
32+
}
33+
}
34+
35+
TEST_CASE("Sharing node internals", "[core][util]")
36+
{
37+
sharing_node_internals_test();
38+
}
39+
40+
TEST_CASE("Sharing node", "[core][util]")
1141
{
1242
SECTION("Leaf test")
1343
{
@@ -45,16 +75,6 @@ void sharing_node_test()
4575
REQUIRE(leaf2.get_value() == 3);
4676
REQUIRE(!leaf2.shares_with(leaf1));
4777
}
48-
49-
// Detaching
50-
{
51-
leaft leaf(1, 2);
52-
53-
auto p = leaf.data.get();
54-
leaf.write();
55-
56-
REQUIRE(leaf.data.get() == p);
57-
}
5878
}
5979

6080
SECTION("Inner node test")
@@ -201,8 +221,3 @@ void sharing_node_test()
201221
REQUIRE(map2.shares_with(map));
202222
}
203223
}
204-
205-
TEST_CASE("Sharing node")
206-
{
207-
sharing_node_test();
208-
}

0 commit comments

Comments
 (0)