radix_tree improvements

[JanDorniak99]

#828

• optimize insert
• allow inline_string modifications (range/at methods in inline string)

---

This change is 

[codecov]

Codecov Report

Merging #907 into master will decrease coverage by 0.05%.
The diff coverage is 96.55%.

@@            Coverage Diff             @@
##           master     #907      +/-   ##
==========================================
- Coverage   95.90%   95.85%   -0.06%     
==========================================
  Files          48       48              
  Lines        6087     6109      +22     
==========================================
+ Hits         5838     5856      +18     
- Misses        249      253       +4     

Flag	Coverage Δ
#tests_clang_debug_cpp17	95.76% <96.55%> (-0.06%)	⬇️
#tests_gcc_debug	92.30% <92.85%> (-0.08%)	⬇️

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

Impacted Files	Coverage Δ
...nclude/libpmemobj++/experimental/inline_string.hpp	98.80% <96.15%> (-1.20%)	⬇️
include/libpmemobj++/experimental/radix_tree.hpp	99.72% <100.00%> (+0.27%)	⬆️
include/libpmemobj++/condition_variable.hpp	73.80% <0.00%> (-4.77%)	⬇️
...ude/libpmemobj++/container/concurrent_hash_map.hpp	93.26% <0.00%> (-0.31%)	⬇️

---

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 49f108b...6bf33d2. Read the comment docs.

[igchor]

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @JanDorniak99)

---

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

template <typename CharT, typename Traits>
typename basic_inline_string<CharT, Traits>::pointer
basic_inline_string<CharT, Traits>::data() noexcept

that's not noexcept anymore

---

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

typename basic_inline_string<CharT, Traits>::pointer
basic_inline_string<CharT, Traits>::snapshotted_data(size_t p, size_t n)
{

you could add some assert to make sure that p + n <= size()

---

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

template <typename Key, typename Value, typename BytesView>
std::pair<typename radix_tree<Key, Value, BytesView>::iterator, bool>
radix_tree<Key, Value, BytesView>::insert(const value_type &v)

Could you add for insert tests similar to the ones you made for try_emplace? (with Moveable and MoveableWrapper).

[JanDorniak99]

Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @igchor)

---

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

Previously, igchor (Igor Chorążewicz) wrote…

that's not noexcept anymore

Done.

---

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

Previously, igchor (Igor Chorążewicz) wrote…

you could add some assert to make sure that p + n <= size()

Done.

---

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

Previously, igchor (Igor Chorążewicz) wrote…

Could you add for insert tests similar to the ones you made for try_emplace? (with Moveable and MoveableWrapper).

Done.

[lukaszstolarczuk]

Reviewed 2 of 3 files at r2.
Reviewable status: 3 of 4 files reviewed, 5 unresolved discussions (waiting on @igchor and @JanDorniak99)

---

include/libpmemobj++/experimental/inline_string.hpp, line 308 at r2 (raw file):

/**
 * Returns reference to a character at position @param[in] p with bounds
 * checking

add '.' at the end of the sentence

---

include/libpmemobj++/experimental/inline_string.hpp, line 352 at r2 (raw file):

 * Return pointer to data at position p and if there is an active transaction
 * snapshot elements from p to p + n.
 */

params not described

[JanDorniak99]

Reviewable status: 2 of 4 files reviewed, 5 unresolved discussions (waiting on @igchor and @lukaszstolarczuk)

---

include/libpmemobj++/experimental/inline_string.hpp, line 308 at r2 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

add '.' at the end of the sentence

Done.

---

include/libpmemobj++/experimental/inline_string.hpp, line 352 at r2 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

params not described

Done.

[lukaszstolarczuk]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @igchor and @JanDorniak99)

---

tests/inline_string/inline_string.cpp, line 102 at r2 (raw file):

	UT_ASSERT(nvobj::basic_string_view<T>(r->o2->s).data()[7] == '\0');
	UT_ASSERT(r->o1->s[4] == '\0');
	UT_ASSERT(r->o2->s[7] == '\0');

perhaps we can check for proper size here, instead

---

tests/inline_string/inline_string.cpp, line 189 at r2 (raw file):

	}
	UT_ASSERT(nvobj::basic_string_view<T>(r->o1->s).data()[0] == '\0');
	UT_ASSERT(r->o1->s[0] == '\0');

.

[igchor]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JanDorniak99)

---

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

{
	return make_internal(std::piecewise_construct,
			     std::forward_as_tuple(std::forward<K>(p.first)),

Could you also change this std::forward to std::move (similarly to the version with detail::pair). It won't affect the behavior but will be more readable I guess.

---

tests/external/libcxx/map/map.modifiers/insert_rv.pass.cpp, line 217 at r3 (raw file):

	UT_ASSERT(r.first->MAP_KEY.get().get() == 3);
	UT_ASSERT(r.first->MAP_VALUE.get().get() == 3);
	UT_ASSERT(v.first.moved() && v.second.moved());

Hm, why this is moved? We should call insert(key_type&&) and later try_empalce, right?

[JanDorniak99]

Reviewable status: 1 of 4 files reviewed, 4 unresolved discussions (waiting on @igchor and @lukaszstolarczuk)

---

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

Previously, igchor (Igor Chorążewicz) wrote…

Could you also change this std::forward to std::move (similarly to the version with detail::pair). It won't affect the behavior but will be more readable I guess.

Done.

---

tests/external/libcxx/map/map.modifiers/insert_rv.pass.cpp, line 217 at r3 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

Hm, why this is moved? We should call insert(key_type&&) and later try_empalce, right?

Done.

---

tests/inline_string/inline_string.cpp, line 102 at r2 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

perhaps we can check for proper size here, instead

Done.

---

tests/inline_string/inline_string.cpp, line 189 at r2 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

.

Done.

[lukaszstolarczuk]

Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @igchor, @JanDorniak99, and @lukaszstolarczuk)

---

tests/inline_string/inline_string.cpp, line 102 at r2 (raw file):

Previously, JanDorniak99 (Jan Dorniak) wrote…

Done.

actually I meant to check for r->o2->s since for o1->s it's already checked few lines above ;)

[igchor]

Reviewed 2 of 3 files at r4.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

[szyrom]

Reviewed 1 of 2 files at r3, 3 of 3 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)