Radix tree: make mt mode a template parameter by igchor · Pull Request #1124 · pmem/libpmemobj-cpp

igchor · 2021-07-05T10:30:49Z

This change is

codecov · 2021-07-05T13:06:10Z

Codecov Report

Merging #1124 (4572774) into master (3477efe) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1124      +/-   ##
==========================================
+ Coverage   94.23%   94.28%   +0.05%     
==========================================
  Files          52       52              
  Lines        5325     5356      +31     
==========================================
+ Hits         5018     5050      +32     
+ Misses        307      306       -1

Flag	Coverage Δ
tests_clang_debug_cpp17	`93.82% <ø> (+0.10%)`	⬆️
tests_gcc_debug	`91.18% <ø> (+0.25%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
include/libpmemobj++/experimental/radix_tree.hpp	`98.64% <ø> (+0.63%)`	⬆️
include/libpmemobj++/transaction.hpp	`79.83% <0.00%> (-0.81%)`	⬇️
...ude/libpmemobj++/container/concurrent_hash_map.hpp	`94.04% <0.00%> (-0.32%)`	⬇️
include/libpmemobj++/detail/tagged_ptr.hpp	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3477efe...4572774. Read the comment docs.

KFilipek

Reviewed 8 of 8 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @igchor)

lukaszstolarczuk

Reviewed 5 of 8 files at r1.
Reviewable status: 7 of 8 files reviewed, 5 unresolved discussions (waiting on @igchor, @KFilipek, and @lukaszstolarczuk)

include/libpmemobj++/experimental/radix_tree.hpp, line 106 at r1 (raw file):

 * - erase and clear does not free nodes/leaves immediately, instead they are
 * added to a garbage list which can be freed by calling garbage_collect()
 * - insert_or_assign and iterator.assign_val do not perform in-place update,

an in-place update

include/libpmemobj++/experimental/radix_tree.hpp, line 110 at r1 (raw file):

MtMode enables single-writer multiple-readers

what about the default value - then you can have only one thread...? pls extend the description

include/libpmemobj++/experimental/radix_tree.hpp, line 1080 at r1 (raw file):

template <bool Mt, typename Enable>
void
radix_tree<Key, Value, BytesView, MtMode>::runtime_initialize_mt(ebr *e)

what happens if I call this method with MtMode=false?

include/libpmemobj++/experimental/radix_tree.hpp, line 2193 at r1 (raw file):

template <typename T>
void
radix_tree<Key, Value, BytesView, MtMode>::free(persistent_ptr<T> ptr)

what about a separate free method for MtMode=false?

tests/radix_tree/radix.hpp, line 34 at r1 (raw file):

using container_inline_s_u8t =
	nvobjex::radix_tree<nvobjex::basic_inline_string<uint8_t>,
			    nvobjex::basic_inline_string<uint8_t>>;

I know that MtMode is false by default, but pls use, at least once, false explicitly, e.g. here

This allows for better optimization and getting rid of atomic operations in single-threaded workloads.

if !MtMode. This improves performance for single-threaded workloads. After this change performance difference when using MtMode true vs false is ~ 15% for readseq in pmemkv-bench benchmarks for radix.

igchor

Reviewable status: 7 of 8 files reviewed, 5 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

include/libpmemobj++/experimental/radix_tree.hpp, line 106 at r1 (raw file):