Skip to content

Commit

Permalink
Fixed a bug in insert_into_new_node() and insert_into_existing_node()…
Browse files Browse the repository at this point in the history
… overloads for parent_nodes.

Fixes for various compilation warnings and errors.
Minor cleanups.
  • Loading branch information
psiha committed Sep 27, 2024
1 parent f43f166 commit 7924f16
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 39 deletions.
74 changes: 49 additions & 25 deletions include/psi/vm/containers/b+tree.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <psi/vm/allocation.hpp>
#include <psi/vm/vector.hpp>

#include <psi/build/disable_warnings.hpp>

#include <boost/assert.hpp>
#include <boost/config_ex.hpp>
#include <boost/stl_interfaces/iterator_interface.hpp>
Expand All @@ -14,13 +16,26 @@
#include <cstdint>
#include <functional>
#include <iterator>
#include <std_fix/const_iterator.hpp>
#include <ranges>
#include <span>
#include <utility>
//------------------------------------------------------------------------------
#if __cpp_lib_shift < 202202L
namespace std::ranges
{
auto shift_left ( auto && rng, auto d ) noexcept { return std::shift_left ( rng.begin(), rng.end(), d ); }
auto shift_right( auto && rng, auto d ) noexcept { return std::shift_right( rng.begin(), rng.end(), d ); }
}
#endif
//------------------------------------------------------------------------------
namespace psi::vm
{
//------------------------------------------------------------------------------

PSI_WARNING_DISABLE_PUSH()
PSI_WARNING_MSVC_DISABLE( 5030 ) // unrecognized attribute

namespace detail
{
template <typename Header>
Expand Down Expand Up @@ -54,8 +69,8 @@ class bptree_base

bool has_attached_storage() const noexcept { return nodes_.has_attached_storage(); }

storage_result map_file ( auto const file, flags::named_object_construction_policy const policy ) noexcept { return init_header( nodes_.map_file ( file, policy ) ); }
storage_result map_memory( size_type const initial_capacity_in_nodes = 0 ) noexcept { return init_header( nodes_.map_memory( initial_capacity_in_nodes ) ); }
storage_result map_file ( auto const file, flags::named_object_construction_policy const policy ) noexcept { return init_header( nodes_.map_file ( file, policy ) ); }
storage_result map_memory( std::uint32_t const initial_capacity_as_number_of_nodes = 0 ) noexcept { return init_header( nodes_.map_memory( initial_capacity_as_number_of_nodes ) ); }

protected:
static constexpr auto node_size{ page_size };
Expand Down Expand Up @@ -258,6 +273,8 @@ class bptree_base::base_random_access_iterator : public base_iterator

// should implicitly handle end iterator comparison also (this requires the start_index constructor argument for the construction of end iterators)
friend constexpr bool operator==( base_random_access_iterator const & left, base_random_access_iterator const & right ) noexcept { return left.index_ == right.index_; }

auto operator<=>( base_random_access_iterator const & other ) const noexcept { return this->index_ <=> other.index_; }
}; // class base_random_access_iterator


Expand Down Expand Up @@ -357,13 +374,18 @@ class bptree_base_wkey : public bptree_base
auto const max{ N::max_values }; // or 'order'
auto const mid{ max / 2 };

new_node.num_vals = max - mid;

value_type key_to_propagate;
if ( new_insert_pos == 0 ) {
if ( new_insert_pos == 0 )
{
umove<&N::keys >( node, mid , node.num_vals , new_node, 0 );
umove<&N::children>( node, mid + 1, node.num_vals + 1, new_node, 1 );
children( new_node )[ new_insert_pos ] = key_right_child;
key_to_propagate = std::move( value );
} else {
}
else
{
key_to_propagate = std::move( node.keys[ mid ] );

umove<&N::keys >( node, mid + 1, insert_pos , new_node, 0 );
Expand All @@ -376,8 +398,7 @@ class bptree_base_wkey : public bptree_base
children( new_node )[ new_insert_pos ] = key_right_child;
}

node .num_vals = mid ;
new_node.num_vals = max - mid;
node.num_vals = mid;

return key_to_propagate;
}
Expand Down Expand Up @@ -424,12 +445,12 @@ class bptree_base_wkey : public bptree_base
umove<&N::keys >( node, insert_pos , mid - 1, node, insert_pos + 1 );
umove<&N::children>( node, insert_pos + 1, mid , node, insert_pos + 2 );

keys ( node )[ insert_pos ] = value;
children( node )[ insert_pos + 1 ] = key_right_child;

node .num_vals = mid;
new_node.num_vals = max - mid;

keys ( node )[ insert_pos ] = value;
children( node )[ insert_pos + 1 ] = key_right_child;

return key_to_propagate;
}

Expand Down Expand Up @@ -481,7 +502,6 @@ class bptree_base_wkey : public bptree_base
source.num_vals = 0;
target.next = source.next;
BOOST_ASSUME( target.num_vals == target.max_values - 1 );

std::ranges::shift_left( keys ( parent ).subspan( parent_key_idx ), 1 );
std::ranges::shift_left( children( parent ).subspan( parent_child_idx ), 1 );
BOOST_ASSUME( parent.num_vals );
Expand Down Expand Up @@ -744,22 +764,24 @@ class bp_tree

private:
[[ gnu::pure, gnu::hot, clang::preserve_most, gnu::noinline ]]
auto find( Key const keys[], node_size_type const num_vals, key_const_arg value ) const noexcept {
auto find( Key const keys[], node_size_type const num_vals, key_const_arg value ) const noexcept
{
BOOST_ASSUME( num_vals > 0 );
auto const posIter { std::lower_bound( &keys[ 0 ], &keys[ num_vals ], value, comp() ) };
auto const posIndex{ std::distance( &keys[ 0 ], posIter ) };
return static_cast<node_size_type>( posIndex );
auto const pos_iter{ std::lower_bound( &keys[ 0 ], &keys[ num_vals ], value, comp() ) };
auto const pos_idx { std::distance( &keys[ 0 ], pos_iter ) };
return static_cast<node_size_type>( pos_idx );
}
auto find( auto const & node, key_const_arg value ) const noexcept {
auto find( auto const & node, key_const_arg value ) const noexcept
{
return find( node.keys, node.num_vals, value );
}

template <typename N>
void split_to_insert( N * p_node, node_size_type const insert_pos, key_const_arg value, node_slot const key_right_child ) {
verify( *p_node );

auto const max{ N::max_values }; // or 'order'
auto const mid{ max / 2 };
node_size_type const max{ N::max_values }; // or 'order'
node_size_type const mid{ max / 2 };

auto const node_slot{ slot_of( *p_node ) };
auto * p_new_node{ &bptree_base::new_node<N>() };
Expand All @@ -778,8 +800,8 @@ class bp_tree
p_node ->next = new_slot;
}

auto const new_insert_pos { insert_pos - mid };
auto const insertion_into_new_node{ insert_pos >= mid };
node_size_type const new_insert_pos ( insert_pos - mid );
bool const insertion_into_new_node{ insert_pos >= mid };
Key key_to_propagate{ insertion_into_new_node
? base::insert_into_new_node ( *p_node, *p_new_node, value, insert_pos, new_insert_pos, key_right_child )
: base::insert_into_existing_node( *p_node, *p_new_node, value, insert_pos, key_right_child )
Expand Down Expand Up @@ -866,12 +888,12 @@ class bp_tree
verify( parent );

auto const parent_key_idx { find( parent, node.keys[ 0 ] ) }; BOOST_ASSUME( parent_key_idx != parent.num_vals || !leaf_node_type );
auto const parent_has_key_copy{ leaf_node_type && ( parent.keys[ parent_key_idx ] == node.keys[ 0 ] ) };
auto const parent_child_idx { parent_key_idx + parent_has_key_copy };
bool const parent_has_key_copy{ leaf_node_type && ( parent.keys[ parent_key_idx ] == node.keys[ 0 ] ) };
auto const parent_child_idx { static_cast<node_size_type>( parent_key_idx + parent_has_key_copy ) };
BOOST_ASSUME( parent.children[ parent_child_idx ] == node_slot );

auto const right_separator_key_idx{ parent_key_idx + parent_has_key_copy };
auto const left_separator_key_idx{ std::min<node_size_type>( right_separator_key_idx - 1, parent.num_vals ) }; // (ab)use unsigned wraparound
auto const right_separator_key_idx{ static_cast<node_size_type>( parent_key_idx + parent_has_key_copy ) };
auto const left_separator_key_idx{ std::min( static_cast<node_size_type>( right_separator_key_idx - 1 ), parent.num_vals ) }; // (ab)use unsigned wraparound
auto const has_right_sibling { right_separator_key_idx != parent.num_vals };
auto const has_left_sibling { left_separator_key_idx != parent.num_vals };
auto const p_right_separator_key { has_right_sibling ? &parent.keys[ right_separator_key_idx ] : nullptr };
Expand Down Expand Up @@ -964,7 +986,7 @@ class bp_tree
BOOST_ASSUME( parent_key_idx == 0 );
BOOST_ASSUME( parent.keys[ parent_key_idx ] <= p_right_sibling->keys[ 0 ] );
// Merge right sibling -> node
this->merge_nodes( *p_right_sibling, node, parent, right_separator_key_idx, parent_child_idx + 1 );
this->merge_nodes( *p_right_sibling, node, parent, right_separator_key_idx, static_cast<node_size_type>( parent_child_idx + 1 ) );
}

// propagate underflow
Expand Down Expand Up @@ -993,7 +1015,7 @@ class bp_tree
key_locations find_nodes_for( key_const_arg key ) noexcept
{
node_slot separator_key_node;
node_size_type separator_key_offset;
node_size_type separator_key_offset{};
// if the root is a (lone) leaf_node is implicitly handled by the loop condition:
// depth_ == 1 so the loop is skipped entirely and the lone root is never examined
// through the incorrectly typed reference
Expand All @@ -1014,6 +1036,8 @@ class bp_tree
}
}; // class bp_tree

PSI_WARNING_DISABLE_POP()

//------------------------------------------------------------------------------
} // namespace psi::vm
//------------------------------------------------------------------------------
4 changes: 2 additions & 2 deletions include/psi/vm/containers/b+tree_print.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void bptree_base_wkey<Key>::print() const
auto & ln{ as<leaf_node>( *node ) };
level_key_count += num_vals( ln );
std::putchar( '[' );
for ( auto i{ 0 }; i < num_vals( ln ); ++i )
for ( auto i{ 0U }; i < num_vals( ln ); ++i )
{
std::print( "{}", keys( ln )[ i ] );
if ( i < num_vals( ln ) - 1 )
Expand All @@ -56,7 +56,7 @@ void bptree_base_wkey<Key>::print() const
// Internal node, print keys and add children to the queue
level_key_count += num_vals( *node );
std::putchar( '<' );
for ( auto i{ 0 }; i < num_vals( *node ); ++i )
for ( auto i{ 0U }; i < num_vals( *node ); ++i )
{
std::print( "{}", keys( *node )[ i ] );
if ( i < num_vals( *node ) - 1 )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ namespace detail
std::memcpy( prefixed_name + shm_prefix.size(), name, name_length + 1 );
}

BOOST_ATTRIBUTES( BOOST_MINSIZE, BOOST_EXCEPTIONLESS, BOOST_WARN_UNUSED_RESULT )
BOOST_ATTRIBUTES( BOOST_MINSIZE, BOOST_EXCEPTIONLESS ) [[ nodiscard ]]
file_handle::reference BOOST_CC_REG shm_open
(
char const * const name,
Expand Down
4 changes: 1 addition & 3 deletions include/psi/vm/vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ class contiguous_container_storage
[[ gnu::pure, nodiscard ]] size_type & stored_size() noexcept requires( !headerless )
{
auto const p_size{ contiguous_container_storage_base::data() + header_size() - size_size };
BOOST_ASSERT( reinterpret_cast<std::intptr_t>( p_size ) % alignof( size_type ) == 0 );
BOOST_ASSERT( reinterpret_cast<std::uintptr_t>( p_size ) % alignof( size_type ) == 0 );
return *reinterpret_cast<size_type *>( p_size );
}
[[ gnu::pure, nodiscard ]] size_type stored_size() const noexcept requires( !headerless )
Expand Down Expand Up @@ -1180,8 +1180,6 @@ class vector

iterator make_space_for_insert( const_iterator const position, size_type const n )
{
using ssize_type = std::make_signed_t<size_type>;

verify_iterator( position );
auto const position_index{ index_of( position ) };
auto const current_size { size() };
Expand Down
9 changes: 5 additions & 4 deletions src/containers/b+tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,13 @@ bptree_base::base_random_access_iterator::operator+=( difference_type const n )
{
if ( n < 0 )
{
BOOST_ASSUME( static_cast<size_type>( -n ) < index_ );
auto const absolute_pos{ index_ + n };
node_slot_ = leaves_start_;
auto const un{ static_cast<size_type>( -n ) };
BOOST_ASSUME( un < index_ );
auto const absolute_pos{ index_ - un };
node_slot_ = leaves_start_;
value_offset_ = 0;
index_ = 0;
return *this += absolute_pos;
return *this += static_cast<difference_type>( absolute_pos );
}

BOOST_ASSERT_MSG( node_slot_, "Iterator at end: not incrementable" );
Expand Down
5 changes: 5 additions & 0 deletions src/detail/nt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
//------------------------------------------------------------------------------
#include <psi/vm/detail/nt.hpp>

#include <psi/build/disable_warnings.hpp>

#include <boost/assert.hpp>

#pragma comment( lib, "ntdll.lib" )
Expand All @@ -37,8 +39,11 @@ namespace detail
[[ gnu::constructor( 101 ) ]]
void init_ntdll_handle() noexcept { ntdll = ::GetModuleHandleW( L"ntdll.dll" ); }
#else
PSI_WARNING_DISABLE_PUSH()
PSI_WARNING_MSVC_DISABLE( 4073 ) // initializers put in library initialization area
#pragma init_seg( lib )
HMODULE const ntdll{ ::GetModuleHandleW( L"ntdll.dll" ) };
PSI_WARNING_DISABLE_POP()
#endif

BOOST_ATTRIBUTES( BOOST_COLD, BOOST_RESTRICTED_FUNCTION_L3, BOOST_RESTRICTED_FUNCTION_RETURN )
Expand Down
2 changes: 1 addition & 1 deletion src/mappable_objects/shared_memory/mem.posix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ namespace detail
return result;
}

BOOST_ATTRIBUTES( BOOST_MINSIZE, BOOST_EXCEPTIONLESS, BOOST_WARN_UNUSED_RESULT )
[[ nodiscard ]] BOOST_ATTRIBUTES( BOOST_MINSIZE, BOOST_EXCEPTIONLESS )
bool named_semaphore::semop( short const opcode, bool const nowait /*= false*/ ) noexcept
{
// http://linux.die.net/man/2/semop
Expand Down
4 changes: 3 additions & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ set( vm_test_sources

add_executable( vm_unit_tests EXCLUDE_FROM_ALL ${vm_test_sources} )
target_link_libraries( vm_unit_tests PRIVATE GTest::gtest_main psi::vm )
target_precompile_headers( vm_unit_tests REUSE_FROM psi_vm )
if ( NOT MSVC OR CLANG_CL ) #:wat: msvc fails to find the pch file in release builds!?
target_precompile_headers( vm_unit_tests REUSE_FROM psi_vm )
endif()

set_target_properties(
vm_unit_tests
Expand Down
7 changes: 5 additions & 2 deletions test/b+tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <filesystem>
#include <random>
#include <ranges>
#include <utility>
#include <vector>
//------------------------------------------------------------------------------
namespace psi::vm
Expand All @@ -30,7 +31,9 @@ TEST( bp_tree, playground )
for ( auto const & n : numbers )
bpt.insert( n );

EXPECT_TRUE( std::ranges::is_sorted( bpt ) );
static_assert( std::forward_iterator<bp_tree<int>::const_iterator> );

EXPECT_TRUE( std::ranges::is_sorted( std::as_const( bpt ) ) );
EXPECT_TRUE( std::ranges::equal( bpt, sorted_numbers ) );
EXPECT_NE( bpt.find( +42 ), bpt.end() );
EXPECT_EQ( bpt.find( -42 ), bpt.end() );
Expand All @@ -45,7 +48,7 @@ TEST( bp_tree, playground )
bpt.insert( +42 );

EXPECT_TRUE( std::ranges::is_sorted( bpt ) );
EXPECT_TRUE( std::ranges::equal( bpt, sorted_numbers ) );
EXPECT_TRUE( std::ranges::equal( std::as_const( bpt ), sorted_numbers ) );
EXPECT_NE( bpt.find( +42 ), bpt.end() );
EXPECT_EQ( bpt.find( -42 ), bpt.end() );

Expand Down

0 comments on commit 7924f16

Please sign in to comment.