From fe349c61bed6aa5e1c6f511e1f6dd21afdfcba4d Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Sat, 3 Sep 2022 13:56:17 +0200 Subject: [PATCH 01/18] feat(Database): new databse for geode objects --- include/geode/basic/database.h | 141 ++++++++++ include/geode/basic/identifier.h | 2 +- src/geode/basic/CMakeLists.txt | 2 + src/geode/basic/database.cpp | 345 ++++++++++++++++++++++++ src/geode/mesh/core/bitsery_archive.cpp | 6 + tests/basic/CMakeLists.txt | 5 + tests/basic/test-database.cpp | 139 ++++++++++ 7 files changed, 639 insertions(+), 1 deletion(-) create mode 100644 include/geode/basic/database.h create mode 100644 src/geode/basic/database.cpp create mode 100644 tests/basic/test-database.cpp diff --git a/include/geode/basic/database.h b/include/geode/basic/database.h new file mode 100644 index 000000000..2a7ad9e54 --- /dev/null +++ b/include/geode/basic/database.h @@ -0,0 +1,141 @@ +/* + * Copyright (c) 2019 - 2022 Geode-solutions + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +#pragma once + +#include +#include +#include + +#include +#include + +namespace geode +{ + class Identifier; + struct uuid; +} // namespace geode + +namespace geode +{ + /*! + * Stores any classes inherited from Identifier. It owns every data + * registered. Data is also stored on disk and offload from the memory after + * some unused time to save memory and performances. + */ + class opengeode_basic_api Database + { + OPENGEODE_DISABLE_COPY( Database ); + struct Storage; + + public: + using serializer_function = std::function< void( PContext& ) >; + + /*! + * Classe holding a const reference of a data. + * @warning Do not destroy this Data class before the const reference + * obtained using its get() method is no longer in used + */ + class opengeode_basic_api Data + { + public: + Data( std::shared_ptr< Storage > storage ); + ~Data(); + Data( Data&& other ); + + template < typename DataType > + const DataType& get() + { + const auto* typed_data = + dynamic_cast< const DataType* >( &data() ); + OPENGEODE_EXCEPTION( + typed_data, "[Data::get] Cannot cast data into DataType" ); + return *typed_data; + } + + private: + const Identifier& data() const; + + private: + IMPLEMENTATION_MEMBER( impl_ ); + }; + + public: + Database( absl::string_view directory ); + ~Database(); + + index_t nb_data() const; + + template < typename DataType > + const uuid& register_data( DataType&& data ) + { + static_assert( std::is_base_of< Identifier, DataType >::value, + "[Database::register_data] Data is not a subclass of " + "Identifier" ); + return register_unique_data( + absl::make_unique< DataType >( std::move( data ) ) ); + } + + template < typename DataType > + const uuid& register_data( std::unique_ptr< DataType >&& data ) + { + static_assert( std::is_base_of< Identifier, DataType >::value, + "[Database::register_data] Data is not a subclass of " + "Identifier" ); + return register_unique_data( std::move( data ) ); + } + + void delete_data( const uuid& id ); + + /*! + * Retrieve a read only reference to the data corresponding to the given + * uuid. + */ + Data get_data( const uuid& id ) const; + + template < typename DataType > + std::unique_ptr< DataType > take_data( const uuid& id ) + { + get_data( id ).get< DataType >(); + auto* data = + dynamic_cast< DataType* >( steal_data( id ).release() ); + return std::unique_ptr< DataType >{ data }; + } + + /*! + * Use this method to register custom serializer functions to allow + * saving any custom Object on disk + */ + void register_serializer_functions( + serializer_function serializer, serializer_function deserializer ); + + private: + const uuid& register_unique_data( + std::unique_ptr< Identifier >&& data ); + + std::unique_ptr< Identifier > steal_data( const uuid& id ); + + private: + IMPLEMENTATION_MEMBER( impl_ ); + }; +} // namespace geode \ No newline at end of file diff --git a/include/geode/basic/identifier.h b/include/geode/basic/identifier.h index 14d5ac2c8..fc00689a8 100644 --- a/include/geode/basic/identifier.h +++ b/include/geode/basic/identifier.h @@ -44,7 +44,7 @@ namespace geode static constexpr auto DEFAULT_NAME = "default_name"; Identifier( Identifier&& ); - ~Identifier(); + virtual ~Identifier(); const uuid& id() const; diff --git a/src/geode/basic/CMakeLists.txt b/src/geode/basic/CMakeLists.txt index 58f4ea5bd..a217735c5 100644 --- a/src/geode/basic/CMakeLists.txt +++ b/src/geode/basic/CMakeLists.txt @@ -29,6 +29,7 @@ add_geode_library( "common.cpp" "console_logger_client.cpp" "console_progress_logger_client.cpp" + "database.cpp" "filename.cpp" "identifier.cpp" "identifier_builder.cpp" @@ -52,6 +53,7 @@ add_geode_library( "common.h" "console_logger_client.h" "console_progress_logger_client.h" + "database.h" "factory.h" "filename.h" "identifier.h" diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp new file mode 100644 index 000000000..044510f93 --- /dev/null +++ b/src/geode/basic/database.cpp @@ -0,0 +1,345 @@ +/* + * Copyright (c) 2019 - 2022 Geode-solutions + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ +#include + +#include + +#include + +#include +#include +#include + +#include +#include +#include + +namespace +{ + constexpr auto DATA_EXPIRATION = absl::Minutes( 5 ); +} // namespace + +namespace geode +{ + class Database::Storage + { + public: + Storage( std::unique_ptr< geode::Identifier >&& data ) + : data_{ std::move( data ) } + { + } + + ~Storage() + { + absl::MutexLock locking{ &lock_ }; + terminate_ = true; + lock_.Await( absl::Condition( + +[]( index_t* nb_calls ) { + return *nb_calls == 0; + }, + &nb_calls_ ) ); + } + + bool expired() const + { + return !data_; + } + + bool unused() const + { + return counter_ == 0; + } + + void new_data_reference() + { + absl::MutexLock locking{ &lock_ }; + counter_++; + } + + void delete_data_reference() + { + absl::MutexLock locking{ &lock_ }; + OPENGEODE_ASSERT( + counter_ > 0, "[Database::Storage] Cannot decrement" ); + counter_--; + if( unused() ) + { + wait_for_memory_release(); + } + } + + void set_data( std::unique_ptr< geode::Identifier >&& data ) + { + data_ = std::move( data ); + } + + const std::unique_ptr< geode::Identifier >& data() const + { + return data_; + } + + geode::Identifier* steal_data() + { + return data_.release(); + } + + private: + void wait_for_memory_release() + { + last_used_ = absl::Now(); + async::spawn( [this] { + absl::MutexLock locking{ &lock_ }; + nb_calls_++; + if( !lock_.AwaitWithTimeout( absl::Condition( + +[]( bool* terminate ) { + return *terminate; + }, + &terminate_ ), + DATA_EXPIRATION + absl::Seconds( 1 ) ) ) + { + if( !terminate_ && unused() + && absl::Now() - last_used_ > DATA_EXPIRATION ) + { + data_.reset(); + } + } + nb_calls_--; + } ); + } + + private: + std::unique_ptr< Identifier > data_; + bool terminate_{ false }; + index_t nb_calls_{ 0 }; + index_t counter_{ 0 }; + absl::Time last_used_; + absl::Mutex lock_; + }; + + class Database::Impl + { + public: + Impl( absl::string_view directory ) + : directory_{ to_string( directory ) } + { + if( !ghc::filesystem::exists( directory_ ) ) + { + ghc::filesystem::create_directory( directory_ ); + } + } + + index_t nb_data() const + { + return storage_.size(); + } + + const uuid& register_unique_data( std::unique_ptr< Identifier >&& data ) + { + const auto& registered_data = register_data( std::move( data ) ); + save_data( registered_data ); + return registered_data->id(); + } + + std::shared_ptr< Storage > data( const uuid& id ) const + { + auto& storage = storage_.at( id ); + if( !storage || storage->expired() ) + { + storage = std::make_shared< Storage >( load_data( id ) ); + } + return storage; + } + + std::unique_ptr< Identifier > steal_data( const uuid& id ) + { + auto& storage = storage_.at( id ); + if( storage && storage->unused() && !storage->expired() ) + { + auto* data = storage->steal_data(); + storage.reset(); + return std::unique_ptr< Identifier >{ data }; + } + return load_data( id ); + } + + void delete_data( const uuid& id ) + { + storage_.erase( id ); + } + + void register_serializer_functions( + serializer_function serializer, serializer_function deserializer ) + { + serializers_.push_back( serializer ); + deserializers_.push_back( deserializer ); + } + + private: + const std::unique_ptr< Identifier >& register_data( + std::unique_ptr< Identifier >&& data ) + { + const auto& id = data->id(); + const auto it = storage_.find( id ); + if( it != storage_.end() ) + { + if( it->second->unused() ) + { + it->second->set_data( std::move( data ) ); + return it->second->data(); + } + delete_data( id ); + return do_register_data( std::move( data ) ); + } + return do_register_data( std::move( data ) ); + } + + const std::unique_ptr< Identifier >& do_register_data( + std::unique_ptr< Identifier >&& data ) + { + const auto id = data->id(); + auto new_storage = std::make_shared< Storage >( std::move( data ) ); + return storage_.emplace( id, std::move( new_storage ) ) + .first->second->data(); + } + + void save_data( const std::unique_ptr< Identifier >& data ) const + { + const auto filename = + absl::StrCat( directory_, "/", data->id().string() ); + std::ofstream file{ filename, std::ofstream::binary }; + TContext context; + for( const auto& serializer : serializers_ ) + { + serializer( std::get< 0 >( context ) ); + } + Serializer archive{ context, file }; + archive.ext( data, bitsery::ext::StdSmartPtr{} ); + archive.adapter().flush(); + OPENGEODE_EXCEPTION( std::get< 1 >( context ).isValid(), + "[Database::save_data] Error while writing file: ", filename ); + } + + std::unique_ptr< Identifier > load_data( const uuid& id ) const + { + const auto filename = absl::StrCat( directory_, "/", id.string() ); + std::ifstream file{ filename, std::ifstream::binary }; + OPENGEODE_EXCEPTION( + file, "[Database::load_data] Failed to open file: ", filename ); + TContext context{}; + for( const auto& deserializer : deserializers_ ) + { + deserializer( std::get< 0 >( context ) ); + } + Deserializer archive{ context, file }; + std::unique_ptr< Identifier > data; + archive.ext( data, bitsery::ext::StdSmartPtr{} ); + const auto& adapter = archive.adapter(); + OPENGEODE_EXCEPTION( + adapter.error() == bitsery::ReaderError::NoError + && adapter.isCompletedSuccessfully() + && std::get< 1 >( context ).isValid(), + "[Database::load_data] Error while reading file: ", filename ); + return data; + } + + private: + std::string directory_; + mutable absl::flat_hash_map< uuid, std::shared_ptr< Storage > > + storage_; + std::vector< serializer_function > serializers_; + std::vector< serializer_function > deserializers_; + }; + + class Database::Data::Impl + { + public: + Impl( std::shared_ptr< Storage > storage ) + : storage_{ std::move( storage ) } + { + storage_->new_data_reference(); + } + + ~Impl() + { + storage_->delete_data_reference(); + } + + const Identifier& data() const + { + return *storage_->data(); + } + + private: + std::shared_ptr< Storage > storage_; + }; + + Database::Data::Data( std::shared_ptr< Storage > storage ) + : impl_{ std::move( storage ) } + { + } + + Database::Data::~Data() {} // NOLINT + + Database::Data::Data( Data&& other ) : impl_{ std::move( other.impl_ ) } {} + + const Identifier& Database::Data::data() const + { + return impl_->data(); + } + + Database::Database( absl::string_view directory ) : impl_{ directory } {} + + Database::~Database() {} // NOLINT + + index_t Database::nb_data() const + { + return impl_->nb_data(); + } + + const uuid& Database::register_unique_data( + std::unique_ptr< Identifier >&& data ) + { + return impl_->register_unique_data( std::move( data ) ); + } + + Database::Data Database::get_data( const uuid& id ) const + { + return { impl_->data( id ) }; + } + + std::unique_ptr< Identifier > Database::steal_data( const uuid& id ) + { + return impl_->steal_data( id ); + } + + void Database::delete_data( const uuid& id ) + { + impl_->delete_data( id ); + } + + void Database::register_serializer_functions( + serializer_function serializer, serializer_function deserializer ) + { + impl_->register_serializer_functions( serializer, deserializer ); + } +} // namespace geode \ No newline at end of file diff --git a/src/geode/mesh/core/bitsery_archive.cpp b/src/geode/mesh/core/bitsery_archive.cpp index 3bb75d512..b39969ff9 100644 --- a/src/geode/mesh/core/bitsery_archive.cpp +++ b/src/geode/mesh/core/bitsery_archive.cpp @@ -47,6 +47,12 @@ namespace bitsery { namespace ext { + template <> + struct PolymorphicBaseClass< geode::Identifier > + : PolymorphicDerivedClasses< geode::VertexSet > + { + }; + template <> struct PolymorphicBaseClass< geode::VertexSet > : PolymorphicDerivedClasses< geode::Graph, diff --git a/tests/basic/CMakeLists.txt b/tests/basic/CMakeLists.txt index 549b3ec82..9dccc948f 100644 --- a/tests/basic/CMakeLists.txt +++ b/tests/basic/CMakeLists.txt @@ -38,6 +38,11 @@ add_geode_test( DEPENDENCIES ${PROJECT_NAME}::basic ) +add_geode_test( + SOURCE "test-database.cpp" + DEPENDENCIES + ${PROJECT_NAME}::basic +) add_geode_test( SOURCE "test-factory.cpp" DEPENDENCIES diff --git a/tests/basic/test-database.cpp b/tests/basic/test-database.cpp new file mode 100644 index 000000000..93560a7f5 --- /dev/null +++ b/tests/basic/test-database.cpp @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2019 - 2022 Geode-solutions + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +#include +#include +#include +#include +#include + +#include + +struct Foo : public geode::Identifier +{ + Foo() = default; + Foo( double value ) : value_( value ) {} + + template < typename Archive > + void serialize( Archive& archive ) + { + archive.ext( *this, bitsery::ext::BaseClass< geode::Identifier >{} ); + archive.value8b( value_ ); + } + + double value_{ 0 }; +}; + +void register_foo_serializer( geode::PContext& context ) +{ + context + .registerSingleBaseBranch< geode::Serializer, geode::Identifier, Foo >( + "Foo" ); +} + +void register_foo_deserializer( geode::PContext& context ) +{ + context.registerSingleBaseBranch< geode::Deserializer, geode::Identifier, + Foo >( "Foo" ); +} + +void test_take_data( geode::Database& database, const geode::uuid& id ) +{ + auto stolen_foo = database.take_data< Foo >( id ); + OPENGEODE_EXCEPTION( + stolen_foo->value_ == 42, "[Test] Wrong value after take data" ); + auto foo_data = database.get_data( id ); + const auto& foo = foo_data.get< Foo >(); + OPENGEODE_EXCEPTION( + foo.value_ == 42, "[Test] Wrong value after register data" ); + OPENGEODE_EXCEPTION( stolen_foo.get() != &foo, + "[Test] Objects adresses should be different" ); +} + +void test_take_wrong_data( geode::Database& database, const geode::uuid& id ) +{ + try + { + auto stolen_foo = database.take_data< geode::uuid >( id ); + throw geode::OpenGeodeException{ + "[Test] Should not be able to cast data into uuid" + }; + } + catch( ... ) + { + return; + } +} + +geode::uuid test_register_data( geode::Database& database ) +{ + auto foo = database.register_data( Foo{ 42 } ); + auto foo_data = database.get_data( foo ); + OPENGEODE_EXCEPTION( foo_data.get< Foo >().value_ == 42, + "[Test] Wrong value after register data" ); + return foo; +} + +geode::uuid test_register_unique_data( geode::Database& database ) +{ + auto foo = database.register_data( absl::make_unique< Foo >( 22 ) ); + auto foo_data = database.get_data( foo ); + OPENGEODE_EXCEPTION( foo_data.get< Foo >().value_ == 22, + "[Test] Wrong value after register unique data" ); + return foo; +} + +void test_modify_data( geode::Database& database, const geode::uuid& id ) +{ + auto foo_data = database.get_data( id ); + const auto& foo = foo_data.get< Foo >(); + auto taken_foo = database.take_data< Foo >( id ); + taken_foo->value_ = 12; + OPENGEODE_EXCEPTION( + taken_foo->value_ == 12, "[Test] Wrong value after modify taken data" ); + OPENGEODE_EXCEPTION( + foo.value_ == 42, "[Test] Wrong value after register data" ); + database.register_data( std::move( taken_foo ) ); + auto foo_data2 = database.get_data( id ); + const auto& foo2 = foo_data2.get< Foo >(); + OPENGEODE_EXCEPTION( + foo2.value_ == 12, "[Test] Wrong value after register data" ); + OPENGEODE_EXCEPTION( + foo.value_ == 42, "[Test] Wrong value after register data" ); +} + +void test() +{ + geode::Database database( "temp" ); + database.register_serializer_functions( + register_foo_serializer, register_foo_deserializer ); + auto foo0 = test_register_data( database ); + test_register_unique_data( database ); + test_take_data( database, foo0 ); + test_modify_data( database, foo0 ); + test_take_wrong_data( database, foo0 ); + OPENGEODE_EXCEPTION( + database.nb_data() == 2, "[Test] Database incomplete" ); +} + +OPENGEODE_TEST( "filename" ) \ No newline at end of file From a8fc1e3df2b490ee553c5c4980b850b9503fdad5 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 7 Dec 2022 16:02:33 +0100 Subject: [PATCH 02/18] test --- include/geode/basic/database.h | 2 ++ src/geode/basic/database.cpp | 58 ++++++++++++++++++++++------------ tests/basic/test-database.cpp | 10 ++++++ 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/include/geode/basic/database.h b/include/geode/basic/database.h index 2a7ad9e54..62979befa 100644 --- a/include/geode/basic/database.h +++ b/include/geode/basic/database.h @@ -117,8 +117,10 @@ namespace geode std::unique_ptr< DataType > take_data( const uuid& id ) { get_data( id ).get< DataType >(); + DEBUG( "get" ); auto* data = dynamic_cast< DataType* >( steal_data( id ).release() ); + DEBUG( "steal" ); return std::unique_ptr< DataType >{ data }; } diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 044510f93..70b73964d 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -22,12 +22,15 @@ */ #include +#include +#include +#include + #include #include #include -#include #include #include @@ -36,7 +39,7 @@ namespace { - constexpr auto DATA_EXPIRATION = absl::Minutes( 5 ); + constexpr auto DATA_EXPIRATION = std::chrono::minutes( 5 ); } // namespace namespace geode @@ -51,13 +54,15 @@ namespace geode ~Storage() { - absl::MutexLock locking{ &lock_ }; + DEBUG( "~Storage" ); + std::unique_lock< std::mutex > locking{ lock_ }; terminate_ = true; - lock_.Await( absl::Condition( - +[]( index_t* nb_calls ) { - return *nb_calls == 0; - }, - &nb_calls_ ) ); + DEBUG( nb_calls_ ); + condition_.notify_all(); + condition_.wait( locking, [this] { + return nb_calls_ == 0; + } ); + DEBUG( "~Storage end" ); } bool expired() const @@ -72,13 +77,13 @@ namespace geode void new_data_reference() { - absl::MutexLock locking{ &lock_ }; + const std::lock_guard< std::mutex > locking{ lock_ }; counter_++; } void delete_data_reference() { - absl::MutexLock locking{ &lock_ }; + const std::lock_guard< std::mutex > locking{ lock_ }; OPENGEODE_ASSERT( counter_ > 0, "[Database::Storage] Cannot decrement" ); counter_--; @@ -106,24 +111,30 @@ namespace geode private: void wait_for_memory_release() { - last_used_ = absl::Now(); + last_used_ = std::chrono::system_clock::now(); async::spawn( [this] { - absl::MutexLock locking{ &lock_ }; + DEBUG( "wait" ); + std::unique_lock< std::mutex > locking{ lock_ }; nb_calls_++; - if( !lock_.AwaitWithTimeout( absl::Condition( - +[]( bool* terminate ) { - return *terminate; - }, - &terminate_ ), - DATA_EXPIRATION + absl::Seconds( 1 ) ) ) + DEBUG( nb_calls_ ); + DEBUG( "wait 2" ); + if( !condition_.wait_for( locking, + DATA_EXPIRATION + std::chrono::seconds( 1 ), [this] { + return terminate_; + } ) ) { + DEBUG( "wait in" ); if( !terminate_ && unused() - && absl::Now() - last_used_ > DATA_EXPIRATION ) + && std::chrono::system_clock::now() - last_used_ + > DATA_EXPIRATION ) { + DEBUG( "wait reset" ); data_.reset(); } } + DEBUG( "wait out" ); nb_calls_--; + condition_.notify_all(); } ); } @@ -132,8 +143,9 @@ namespace geode bool terminate_{ false }; index_t nb_calls_{ 0 }; index_t counter_{ 0 }; - absl::Time last_used_; - absl::Mutex lock_; + std::chrono::time_point< std::chrono::system_clock > last_used_; + std::mutex lock_; + std::condition_variable condition_; }; class Database::Impl @@ -175,10 +187,14 @@ namespace geode auto& storage = storage_.at( id ); if( storage && storage->unused() && !storage->expired() ) { + DEBUG( "in" ); auto* data = storage->steal_data(); + DEBUG( "steal" ); storage.reset(); + DEBUG( "reset" ); return std::unique_ptr< Identifier >{ data }; } + DEBUG( "load" ); return load_data( id ); } diff --git a/tests/basic/test-database.cpp b/tests/basic/test-database.cpp index 93560a7f5..b1a0a658a 100644 --- a/tests/basic/test-database.cpp +++ b/tests/basic/test-database.cpp @@ -60,14 +60,18 @@ void register_foo_deserializer( geode::PContext& context ) void test_take_data( geode::Database& database, const geode::uuid& id ) { auto stolen_foo = database.take_data< Foo >( id ); + DEBUG( "take 1" ); OPENGEODE_EXCEPTION( stolen_foo->value_ == 42, "[Test] Wrong value after take data" ); auto foo_data = database.get_data( id ); + DEBUG( "take 2" ); const auto& foo = foo_data.get< Foo >(); + DEBUG( "take 3" ); OPENGEODE_EXCEPTION( foo.value_ == 42, "[Test] Wrong value after register data" ); OPENGEODE_EXCEPTION( stolen_foo.get() != &foo, "[Test] Objects adresses should be different" ); + DEBUG( "take 4" ); } void test_take_wrong_data( geode::Database& database, const geode::uuid& id ) @@ -127,11 +131,17 @@ void test() geode::Database database( "temp" ); database.register_serializer_functions( register_foo_serializer, register_foo_deserializer ); + DEBUG( "functions" ); auto foo0 = test_register_data( database ); + DEBUG( "register" ); test_register_unique_data( database ); + DEBUG( "register unique" ); test_take_data( database, foo0 ); + DEBUG( "take" ); test_modify_data( database, foo0 ); + DEBUG( "modify" ); test_take_wrong_data( database, foo0 ); + DEBUG( "wrong" ); OPENGEODE_EXCEPTION( database.nb_data() == 2, "[Test] Database incomplete" ); } From 83a1e8415be29573d076f03241984d9eb7cb1369 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 7 Dec 2022 16:40:54 +0100 Subject: [PATCH 03/18] test --- src/geode/basic/database.cpp | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 70b73964d..d6d4fe6c9 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -39,7 +39,8 @@ namespace { - constexpr auto DATA_EXPIRATION = std::chrono::minutes( 5 ); + constexpr auto DATA_EXPIRATION = std::chrono::minutes( 1 ); + int count{ 0 }; } // namespace namespace geode @@ -48,21 +49,21 @@ namespace geode { public: Storage( std::unique_ptr< geode::Identifier >&& data ) - : data_{ std::move( data ) } + : data_{ std::move( data ) }, count_{ count++ } { } ~Storage() { - DEBUG( "~Storage" ); + Logger::debug( count, " -> ", "~Storage" ); std::unique_lock< std::mutex > locking{ lock_ }; terminate_ = true; - DEBUG( nb_calls_ ); condition_.notify_all(); condition_.wait( locking, [this] { + Logger::debug( count, " -> ", nb_calls_ ); return nb_calls_ == 0; } ); - DEBUG( "~Storage end" ); + Logger::debug( count, " -> ", "~Storage end" ); } bool expired() const @@ -113,26 +114,26 @@ namespace geode { last_used_ = std::chrono::system_clock::now(); async::spawn( [this] { - DEBUG( "wait" ); + Logger::debug( count, " -> ", "wait" ); std::unique_lock< std::mutex > locking{ lock_ }; nb_calls_++; - DEBUG( nb_calls_ ); - DEBUG( "wait 2" ); + Logger::debug( count, " -> ", "wait 2" ); if( !condition_.wait_for( locking, DATA_EXPIRATION + std::chrono::seconds( 1 ), [this] { + Logger::debug( count, " -> ", terminate_ ); return terminate_; } ) ) { - DEBUG( "wait in" ); + Logger::debug( count, " -> ", "wait in" ); if( !terminate_ && unused() && std::chrono::system_clock::now() - last_used_ > DATA_EXPIRATION ) { - DEBUG( "wait reset" ); + Logger::debug( count, " -> ", "wait reset" ); data_.reset(); } } - DEBUG( "wait out" ); + Logger::debug( count, " -> ", "wait out" ); nb_calls_--; condition_.notify_all(); } ); @@ -146,6 +147,7 @@ namespace geode std::chrono::time_point< std::chrono::system_clock > last_used_; std::mutex lock_; std::condition_variable condition_; + int count_; }; class Database::Impl From cf97c83de90fc82177f20db6388621c05d0ffd36 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 8 Dec 2022 13:28:41 +0100 Subject: [PATCH 04/18] test --- src/geode/basic/database.cpp | 113 +++++++++++++++++++++++++---------- 1 file changed, 81 insertions(+), 32 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index d6d4fe6c9..c810fde93 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -25,6 +25,9 @@ #include #include #include +#include +#include +#include #include @@ -51,19 +54,21 @@ namespace geode Storage( std::unique_ptr< geode::Identifier >&& data ) : data_{ std::move( data ) }, count_{ count++ } { + Logger::debug( count_, " -> ", "Storage" ); } ~Storage() { - Logger::debug( count, " -> ", "~Storage" ); + Logger::debug( count_, " -> ", "~Storage" ); + terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - terminate_ = true; - condition_.notify_all(); - condition_.wait( locking, [this] { - Logger::debug( count, " -> ", nb_calls_ ); - return nb_calls_ == 0; + condition_.wait( locking, [&] { + Logger::debug( count_, " -> ", "~calls ", queue_.size() ); + clean_queue(); + Logger::debug( count_, " -> ", "~calls2 ", queue_.size() ); + return queue_.empty(); } ); - Logger::debug( count, " -> ", "~Storage end" ); + Logger::debug( count_, " -> ", "~Storage end" ); } bool expired() const @@ -80,6 +85,9 @@ namespace geode { const std::lock_guard< std::mutex > locking{ lock_ }; counter_++; + std::ostringstream oss; + oss << std::this_thread::get_id() << " " << this; + Logger::debug( count_, " -> ", "new ", counter_, " ", oss.str() ); } void delete_data_reference() @@ -88,16 +96,26 @@ namespace geode OPENGEODE_ASSERT( counter_ > 0, "[Database::Storage] Cannot decrement" ); counter_--; + std::ostringstream oss; + oss << std::this_thread::get_id() << " " << this; + Logger::debug( + count_, " -> ", "delete ", counter_, " ", oss.str() ); if( unused() ) { + clean_queue(); wait_for_memory_release(); } } - void set_data( std::unique_ptr< geode::Identifier >&& data ) - { - data_ = std::move( data ); - } + // void set_data( std::unique_ptr< geode::Identifier >&& data ) + // { + // const std::lock_guard< std::mutex > locking{ lock_ }; + // terminate_ = false; + // counter_ = 0; + // count_ = count++; + // Logger::debug( count_, " -> ", "set_data " ); + // data_ = std::move( data ); + // } const std::unique_ptr< geode::Identifier >& data() const { @@ -110,44 +128,75 @@ namespace geode } private: + void terminate_storage() + { + std::ostringstream oss; + oss << std::this_thread::get_id() << " " << this; + Logger::debug( count_, " -> ", "begin terminate_storage" ); + terminate_ = true; + Logger::debug( + count_, " -> ", "calls ", queue_.size(), " ", oss.str() ); + condition_.notify_all(); + Logger::debug( count_, " -> ", "end terminate_storage" ); + } + + void clean_queue() + { + while( !queue_.empty() ) + { + if( !queue_.front().ready() ) + { + return; + } + queue_.pop(); + } + } + void wait_for_memory_release() { - last_used_ = std::chrono::system_clock::now(); - async::spawn( [this] { - Logger::debug( count, " -> ", "wait" ); + queue_.emplace( async::spawn( [this] { + std::ostringstream oss; + oss << std::this_thread::get_id() << " " << this; + Logger::debug( count_, " -> ", "wait start ", oss.str() ); + Logger::debug( count_, " -> ", "wait start 2 ", oss.str() ); std::unique_lock< std::mutex > locking{ lock_ }; - nb_calls_++; - Logger::debug( count, " -> ", "wait 2" ); + last_used_ = std::chrono::system_clock::now(); + Logger::debug( count_, " -> ", "wait 2 + ", oss.str() ); if( !condition_.wait_for( locking, - DATA_EXPIRATION + std::chrono::seconds( 1 ), [this] { - Logger::debug( count, " -> ", terminate_ ); - return terminate_; + DATA_EXPIRATION + std::chrono::seconds( 1 ), + [this, &oss] { + Logger::debug( count_, " -> ", "terminate ", + terminate_.load(), " ", oss.str() ); + return terminate_.load(); } ) ) { - Logger::debug( count, " -> ", "wait in" ); + Logger::debug( count_, " -> ", "wait in", " ", oss.str() ); if( !terminate_ && unused() && std::chrono::system_clock::now() - last_used_ > DATA_EXPIRATION ) { - Logger::debug( count, " -> ", "wait reset" ); + Logger::debug( count_, " -> ", "wait reset", " ", + oss.str(), " ", + reinterpret_cast< size_t >( this ) ); data_.reset(); } } - Logger::debug( count, " -> ", "wait out" ); - nb_calls_--; + Logger::debug( count_, " -> ", "wait out + ", queue_.size(), + " ", oss.str() ); + locking.unlock(); condition_.notify_all(); - } ); + } ) ); } private: std::unique_ptr< Identifier > data_; - bool terminate_{ false }; - index_t nb_calls_{ 0 }; + std::atomic< bool > terminate_{ false }; index_t counter_{ 0 }; std::chrono::time_point< std::chrono::system_clock > last_used_; std::mutex lock_; std::condition_variable condition_; int count_; + std::queue< async::task< void > > queue_; }; class Database::Impl @@ -220,13 +269,13 @@ namespace geode const auto it = storage_.find( id ); if( it != storage_.end() ) { - if( it->second->unused() ) - { - it->second->set_data( std::move( data ) ); - return it->second->data(); - } + // if( it->second->unused() ) + // { + // it->second->set_data( std::move( data ) ); + // return it->second->data(); + // } delete_data( id ); - return do_register_data( std::move( data ) ); + // return do_register_data( std::move( data ) ); } return do_register_data( std::move( data ) ); } From 8e58c43d42a6eb3c09f0c97d271c888fadb919dc Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 8 Dec 2022 13:55:14 +0100 Subject: [PATCH 05/18] test --- src/geode/basic/database.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index c810fde93..aba7261a2 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -62,12 +62,13 @@ namespace geode Logger::debug( count_, " -> ", "~Storage" ); terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - condition_.wait( locking, [&] { - Logger::debug( count_, " -> ", "~calls ", queue_.size() ); + while( !queue_.empty() ) + { + Logger::debug( count_, " -> ", "Q ", queue_.size() ); clean_queue(); - Logger::debug( count_, " -> ", "~calls2 ", queue_.size() ); - return queue_.empty(); - } ); + Logger::debug( count_, " -> ", "Q2 ", queue_.size() ); + condition_.wait( locking ); + } Logger::debug( count_, " -> ", "~Storage end" ); } From 73456692b849763740e8953c3e5b3a6a4a225e4b Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 8 Dec 2022 14:19:12 +0100 Subject: [PATCH 06/18] test --- src/geode/basic/database.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index aba7261a2..92f1f309b 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -62,13 +62,14 @@ namespace geode Logger::debug( count_, " -> ", "~Storage" ); terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - while( !queue_.empty() ) + do { - Logger::debug( count_, " -> ", "Q ", queue_.size() ); clean_queue(); - Logger::debug( count_, " -> ", "Q2 ", queue_.size() ); - condition_.wait( locking ); - } + + } while( !condition_.wait_for( + locking, std::chrono::milliseconds( 1 ), [this] { + return queue_.empty(); + } ) ); Logger::debug( count_, " -> ", "~Storage end" ); } From b2d02f23e6de7850c215bc01f899ac3383debbc8 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 8 Dec 2022 15:02:06 +0100 Subject: [PATCH 07/18] test --- src/geode/basic/database.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 92f1f309b..646d28f7f 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -62,14 +62,10 @@ namespace geode Logger::debug( count_, " -> ", "~Storage" ); terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - do - { + condition_.wait( locking, [this] { clean_queue(); - - } while( !condition_.wait_for( - locking, std::chrono::milliseconds( 1 ), [this] { - return queue_.empty(); - } ) ); + return queue_.empty(); + } ); Logger::debug( count_, " -> ", "~Storage end" ); } @@ -185,8 +181,8 @@ namespace geode } Logger::debug( count_, " -> ", "wait out + ", queue_.size(), " ", oss.str() ); - locking.unlock(); condition_.notify_all(); + locking.unlock(); } ) ); } From 723463fa269e40912097657c5c067b24e74f3b63 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 8 Dec 2022 15:44:11 +0100 Subject: [PATCH 08/18] test --- src/geode/basic/database.cpp | 57 +++++++++++++++++------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 646d28f7f..a39b39100 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -62,10 +62,13 @@ namespace geode Logger::debug( count_, " -> ", "~Storage" ); terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - condition_.wait( locking, [this] { + do + { clean_queue(); - return queue_.empty(); - } ); + } while( !condition_.wait_for( + locking, std::chrono::microseconds( 10 ), [this] { + return queue_.empty(); + } ) ); Logger::debug( count_, " -> ", "~Storage end" ); } @@ -83,9 +86,10 @@ namespace geode { const std::lock_guard< std::mutex > locking{ lock_ }; counter_++; + last_++; std::ostringstream oss; oss << std::this_thread::get_id() << " " << this; - Logger::debug( count_, " -> ", "new ", counter_, " ", oss.str() ); + Logger::debug( count_, " -> ", "new ", counter_, " " ); } void delete_data_reference() @@ -96,8 +100,7 @@ namespace geode counter_--; std::ostringstream oss; oss << std::this_thread::get_id() << " " << this; - Logger::debug( - count_, " -> ", "delete ", counter_, " ", oss.str() ); + Logger::debug( count_, " -> ", "delete ", counter_, " " ); if( unused() ) { clean_queue(); @@ -132,8 +135,7 @@ namespace geode oss << std::this_thread::get_id() << " " << this; Logger::debug( count_, " -> ", "begin terminate_storage" ); terminate_ = true; - Logger::debug( - count_, " -> ", "calls ", queue_.size(), " ", oss.str() ); + Logger::debug( count_, " -> ", "calls ", queue_.size(), " " ); condition_.notify_all(); Logger::debug( count_, " -> ", "end terminate_storage" ); } @@ -152,37 +154,32 @@ namespace geode void wait_for_memory_release() { - queue_.emplace( async::spawn( [this] { - std::ostringstream oss; - oss << std::this_thread::get_id() << " " << this; - Logger::debug( count_, " -> ", "wait start ", oss.str() ); - Logger::debug( count_, " -> ", "wait start 2 ", oss.str() ); + const auto last = last_; + queue_.emplace( async::spawn( [this, last] { + Logger::debug( count_, " -> ", "wait start " ); + Logger::debug( count_, " -> ", "wait start 2 " ); std::unique_lock< std::mutex > locking{ lock_ }; - last_used_ = std::chrono::system_clock::now(); - Logger::debug( count_, " -> ", "wait 2 + ", oss.str() ); - if( !condition_.wait_for( locking, - DATA_EXPIRATION + std::chrono::seconds( 1 ), - [this, &oss] { + Logger::debug( count_, " -> ", "wait 2 + " ); + Logger::debug( count_, " -> ", "last ", last, " ", last_ ); + if( !condition_.wait_for( + locking, DATA_EXPIRATION, [this, last] { Logger::debug( count_, " -> ", "terminate ", - terminate_.load(), " ", oss.str() ); + terminate_.load(), " " ); return terminate_.load(); } ) ) { - Logger::debug( count_, " -> ", "wait in", " ", oss.str() ); - if( !terminate_ && unused() - && std::chrono::system_clock::now() - last_used_ - > DATA_EXPIRATION ) + Logger::debug( + count_, " -> ", "wait in", " ", last, " ", last_ ); + if( last == last_ ) { - Logger::debug( count_, " -> ", "wait reset", " ", - oss.str(), " ", - reinterpret_cast< size_t >( this ) ); + Logger::debug( count_, " -> ", "wait reset", " " ); data_.reset(); } } - Logger::debug( count_, " -> ", "wait out + ", queue_.size(), - " ", oss.str() ); - condition_.notify_all(); + Logger::debug( + count_, " -> ", "wait out + ", queue_.size(), " " ); locking.unlock(); + condition_.notify_all(); } ) ); } @@ -190,9 +187,9 @@ namespace geode std::unique_ptr< Identifier > data_; std::atomic< bool > terminate_{ false }; index_t counter_{ 0 }; - std::chrono::time_point< std::chrono::system_clock > last_used_; std::mutex lock_; std::condition_variable condition_; + index_t last_; int count_; std::queue< async::task< void > > queue_; }; From 0cabea58cd1c3fb31fcaa2cd2c284aa80b9e0e73 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 10:16:01 +0100 Subject: [PATCH 09/18] done --- include/geode/basic/database.h | 37 ++++++++--- src/geode/basic/database.cpp | 108 +++++++++++---------------------- tests/basic/test-database.cpp | 16 +---- 3 files changed, 68 insertions(+), 93 deletions(-) diff --git a/include/geode/basic/database.h b/include/geode/basic/database.h index 62979befa..b8f10ce06 100644 --- a/include/geode/basic/database.h +++ b/include/geode/basic/database.h @@ -87,22 +87,45 @@ namespace geode index_t nb_data() const; template < typename DataType > - const uuid& register_data( DataType&& data ) + uuid register_new_data( DataType&& data ) + { + static_assert( std::is_base_of< Identifier, DataType >::value, + "[Database::register_data] Data is not a subclass of " + "Identifier" ); + uuid new_id; + register_unique_data( + new_id, absl::make_unique< DataType >( std::move( data ) ) ); + return new_id; + } + + template < typename DataType > + uuid register_new_data( std::unique_ptr< DataType >&& data ) + { + static_assert( std::is_base_of< Identifier, DataType >::value, + "[Database::register_data] Data is not a subclass of " + "Identifier" ); + uuid new_id; + register_unique_data( new_id, std::move( data ) ); + return new_id; + } + + template < typename DataType > + void update_data( const uuid& id, DataType&& data ) { static_assert( std::is_base_of< Identifier, DataType >::value, "[Database::register_data] Data is not a subclass of " "Identifier" ); return register_unique_data( - absl::make_unique< DataType >( std::move( data ) ) ); + id, absl::make_unique< DataType >( std::move( data ) ) ); } template < typename DataType > - const uuid& register_data( std::unique_ptr< DataType >&& data ) + void update_data( const uuid& id, std::unique_ptr< DataType >&& data ) { static_assert( std::is_base_of< Identifier, DataType >::value, "[Database::register_data] Data is not a subclass of " "Identifier" ); - return register_unique_data( std::move( data ) ); + return register_unique_data( id, std::move( data ) ); } void delete_data( const uuid& id ); @@ -117,10 +140,8 @@ namespace geode std::unique_ptr< DataType > take_data( const uuid& id ) { get_data( id ).get< DataType >(); - DEBUG( "get" ); auto* data = dynamic_cast< DataType* >( steal_data( id ).release() ); - DEBUG( "steal" ); return std::unique_ptr< DataType >{ data }; } @@ -132,8 +153,8 @@ namespace geode serializer_function serializer, serializer_function deserializer ); private: - const uuid& register_unique_data( - std::unique_ptr< Identifier >&& data ); + void register_unique_data( + const uuid& id, std::unique_ptr< Identifier >&& data ); std::unique_ptr< Identifier > steal_data( const uuid& id ); diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index a39b39100..f156e46f4 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -26,15 +26,12 @@ #include #include #include -#include -#include #include #include #include -#include #include #include @@ -42,8 +39,7 @@ namespace { - constexpr auto DATA_EXPIRATION = std::chrono::minutes( 1 ); - int count{ 0 }; + constexpr auto DATA_EXPIRATION = std::chrono::minutes( 3 ); } // namespace namespace geode @@ -52,24 +48,24 @@ namespace geode { public: Storage( std::unique_ptr< geode::Identifier >&& data ) - : data_{ std::move( data ) }, count_{ count++ } + : data_{ std::move( data ) } { - Logger::debug( count_, " -> ", "Storage" ); } ~Storage() { - Logger::debug( count_, " -> ", "~Storage" ); terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - do + clean_queue(); + while( !condition_.wait_for( + locking, std::chrono::milliseconds( 10 ), [this] { + return queue_.empty(); + } ) ) + ; { + condition_.notify_all(); clean_queue(); - } while( !condition_.wait_for( - locking, std::chrono::microseconds( 10 ), [this] { - return queue_.empty(); - } ) ); - Logger::debug( count_, " -> ", "~Storage end" ); + } } bool expired() const @@ -87,9 +83,6 @@ namespace geode const std::lock_guard< std::mutex > locking{ lock_ }; counter_++; last_++; - std::ostringstream oss; - oss << std::this_thread::get_id() << " " << this; - Logger::debug( count_, " -> ", "new ", counter_, " " ); } void delete_data_reference() @@ -98,9 +91,6 @@ namespace geode OPENGEODE_ASSERT( counter_ > 0, "[Database::Storage] Cannot decrement" ); counter_--; - std::ostringstream oss; - oss << std::this_thread::get_id() << " " << this; - Logger::debug( count_, " -> ", "delete ", counter_, " " ); if( unused() ) { clean_queue(); @@ -108,15 +98,12 @@ namespace geode } } - // void set_data( std::unique_ptr< geode::Identifier >&& data ) - // { - // const std::lock_guard< std::mutex > locking{ lock_ }; - // terminate_ = false; - // counter_ = 0; - // count_ = count++; - // Logger::debug( count_, " -> ", "set_data " ); - // data_ = std::move( data ); - // } + void set_data( std::unique_ptr< geode::Identifier >&& data ) + { + const std::lock_guard< std::mutex > locking{ lock_ }; + counter_ = 0; + data_ = std::move( data ); + } const std::unique_ptr< geode::Identifier >& data() const { @@ -131,13 +118,8 @@ namespace geode private: void terminate_storage() { - std::ostringstream oss; - oss << std::this_thread::get_id() << " " << this; - Logger::debug( count_, " -> ", "begin terminate_storage" ); terminate_ = true; - Logger::debug( count_, " -> ", "calls ", queue_.size(), " " ); condition_.notify_all(); - Logger::debug( count_, " -> ", "end terminate_storage" ); } void clean_queue() @@ -156,28 +138,17 @@ namespace geode { const auto last = last_; queue_.emplace( async::spawn( [this, last] { - Logger::debug( count_, " -> ", "wait start " ); - Logger::debug( count_, " -> ", "wait start 2 " ); std::unique_lock< std::mutex > locking{ lock_ }; - Logger::debug( count_, " -> ", "wait 2 + " ); - Logger::debug( count_, " -> ", "last ", last, " ", last_ ); if( !condition_.wait_for( locking, DATA_EXPIRATION, [this, last] { - Logger::debug( count_, " -> ", "terminate ", - terminate_.load(), " " ); return terminate_.load(); } ) ) { - Logger::debug( - count_, " -> ", "wait in", " ", last, " ", last_ ); if( last == last_ ) { - Logger::debug( count_, " -> ", "wait reset", " " ); data_.reset(); } } - Logger::debug( - count_, " -> ", "wait out + ", queue_.size(), " " ); locking.unlock(); condition_.notify_all(); } ) ); @@ -189,8 +160,7 @@ namespace geode index_t counter_{ 0 }; std::mutex lock_; std::condition_variable condition_; - index_t last_; - int count_; + index_t last_{ 0 }; std::queue< async::task< void > > queue_; }; @@ -211,11 +181,11 @@ namespace geode return storage_.size(); } - const uuid& register_unique_data( std::unique_ptr< Identifier >&& data ) + void register_unique_data( + const uuid& id, std::unique_ptr< Identifier >&& data ) { - const auto& registered_data = register_data( std::move( data ) ); - save_data( registered_data ); - return registered_data->id(); + save_data( id, data ); + register_data( id, std::move( data ) ); } std::shared_ptr< Storage > data( const uuid& id ) const @@ -233,14 +203,10 @@ namespace geode auto& storage = storage_.at( id ); if( storage && storage->unused() && !storage->expired() ) { - DEBUG( "in" ); auto* data = storage->steal_data(); - DEBUG( "steal" ); storage.reset(); - DEBUG( "reset" ); return std::unique_ptr< Identifier >{ data }; } - DEBUG( "load" ); return load_data( id ); } @@ -258,36 +224,34 @@ namespace geode private: const std::unique_ptr< Identifier >& register_data( - std::unique_ptr< Identifier >&& data ) + const uuid& id, std::unique_ptr< Identifier >&& data ) { - const auto& id = data->id(); const auto it = storage_.find( id ); if( it != storage_.end() ) { - // if( it->second->unused() ) - // { - // it->second->set_data( std::move( data ) ); - // return it->second->data(); - // } + if( it->second->unused() ) + { + it->second->set_data( std::move( data ) ); + return it->second->data(); + } delete_data( id ); - // return do_register_data( std::move( data ) ); + return do_register_data( id, std::move( data ) ); } - return do_register_data( std::move( data ) ); + return do_register_data( id, std::move( data ) ); } const std::unique_ptr< Identifier >& do_register_data( - std::unique_ptr< Identifier >&& data ) + const uuid& id, std::unique_ptr< Identifier >&& data ) { - const auto id = data->id(); auto new_storage = std::make_shared< Storage >( std::move( data ) ); return storage_.emplace( id, std::move( new_storage ) ) .first->second->data(); } - void save_data( const std::unique_ptr< Identifier >& data ) const + void save_data( + const uuid& id, const std::unique_ptr< Identifier >& data ) const { - const auto filename = - absl::StrCat( directory_, "/", data->id().string() ); + const auto filename = absl::StrCat( directory_, "/", id.string() ); std::ofstream file{ filename, std::ofstream::binary }; TContext context; for( const auto& serializer : serializers_ ) @@ -378,10 +342,10 @@ namespace geode return impl_->nb_data(); } - const uuid& Database::register_unique_data( - std::unique_ptr< Identifier >&& data ) + void Database::register_unique_data( + const uuid& id, std::unique_ptr< Identifier >&& data ) { - return impl_->register_unique_data( std::move( data ) ); + impl_->register_unique_data( id, std::move( data ) ); } Database::Data Database::get_data( const uuid& id ) const diff --git a/tests/basic/test-database.cpp b/tests/basic/test-database.cpp index b1a0a658a..311316d66 100644 --- a/tests/basic/test-database.cpp +++ b/tests/basic/test-database.cpp @@ -60,18 +60,14 @@ void register_foo_deserializer( geode::PContext& context ) void test_take_data( geode::Database& database, const geode::uuid& id ) { auto stolen_foo = database.take_data< Foo >( id ); - DEBUG( "take 1" ); OPENGEODE_EXCEPTION( stolen_foo->value_ == 42, "[Test] Wrong value after take data" ); auto foo_data = database.get_data( id ); - DEBUG( "take 2" ); const auto& foo = foo_data.get< Foo >(); - DEBUG( "take 3" ); OPENGEODE_EXCEPTION( foo.value_ == 42, "[Test] Wrong value after register data" ); OPENGEODE_EXCEPTION( stolen_foo.get() != &foo, "[Test] Objects adresses should be different" ); - DEBUG( "take 4" ); } void test_take_wrong_data( geode::Database& database, const geode::uuid& id ) @@ -91,7 +87,7 @@ void test_take_wrong_data( geode::Database& database, const geode::uuid& id ) geode::uuid test_register_data( geode::Database& database ) { - auto foo = database.register_data( Foo{ 42 } ); + auto foo = database.register_new_data( Foo{ 42 } ); auto foo_data = database.get_data( foo ); OPENGEODE_EXCEPTION( foo_data.get< Foo >().value_ == 42, "[Test] Wrong value after register data" ); @@ -100,7 +96,7 @@ geode::uuid test_register_data( geode::Database& database ) geode::uuid test_register_unique_data( geode::Database& database ) { - auto foo = database.register_data( absl::make_unique< Foo >( 22 ) ); + auto foo = database.register_new_data( absl::make_unique< Foo >( 22 ) ); auto foo_data = database.get_data( foo ); OPENGEODE_EXCEPTION( foo_data.get< Foo >().value_ == 22, "[Test] Wrong value after register unique data" ); @@ -117,7 +113,7 @@ void test_modify_data( geode::Database& database, const geode::uuid& id ) taken_foo->value_ == 12, "[Test] Wrong value after modify taken data" ); OPENGEODE_EXCEPTION( foo.value_ == 42, "[Test] Wrong value after register data" ); - database.register_data( std::move( taken_foo ) ); + database.update_data( id, std::move( taken_foo ) ); auto foo_data2 = database.get_data( id ); const auto& foo2 = foo_data2.get< Foo >(); OPENGEODE_EXCEPTION( @@ -131,17 +127,11 @@ void test() geode::Database database( "temp" ); database.register_serializer_functions( register_foo_serializer, register_foo_deserializer ); - DEBUG( "functions" ); auto foo0 = test_register_data( database ); - DEBUG( "register" ); test_register_unique_data( database ); - DEBUG( "register unique" ); test_take_data( database, foo0 ); - DEBUG( "take" ); test_modify_data( database, foo0 ); - DEBUG( "modify" ); test_take_wrong_data( database, foo0 ); - DEBUG( "wrong" ); OPENGEODE_EXCEPTION( database.nb_data() == 2, "[Test] Database incomplete" ); } From 98cf279f317756a4f7e0ec8de12108505f827237 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:05:06 +0100 Subject: [PATCH 10/18] test --- src/geode/basic/database.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index f156e46f4..d32170acc 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -56,16 +56,16 @@ namespace geode { terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - clean_queue(); - while( !condition_.wait_for( - locking, std::chrono::milliseconds( 10 ), [this] { - return queue_.empty(); - } ) ) - ; + index_t count{ 0 }; + do { - condition_.notify_all(); + DEBUG( count++ ); clean_queue(); - } + } while( !condition_.wait_for( + locking, std::chrono::milliseconds( 10 ), [this] { + return queue_.empty(); + } ) ); + DEBUG( "end" ); } bool expired() const From 67c3b52db5e4d59777f1b95de486fd3664216b3f Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:29:09 +0100 Subject: [PATCH 11/18] test --- src/geode/basic/database.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index d32170acc..0f1882962 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -56,16 +56,10 @@ namespace geode { terminate_storage(); std::unique_lock< std::mutex > locking{ lock_ }; - index_t count{ 0 }; - do - { - DEBUG( count++ ); + condition_.wait( locking, [this] { clean_queue(); - } while( !condition_.wait_for( - locking, std::chrono::milliseconds( 10 ), [this] { - return queue_.empty(); - } ) ); - DEBUG( "end" ); + return queue_.empty(); + } ); } bool expired() const @@ -128,10 +122,11 @@ namespace geode { if( !queue_.front().ready() ) { - return; + break; } queue_.pop(); } + condition_.notify_all(); } void wait_for_memory_release() From ac9e9e455dab7e6f5db5fe24c61701924db672cf Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:29:25 +0100 Subject: [PATCH 12/18] test --- src/bin/CMakeLists.txt | 34 +++++++++++++++++----------------- src/geode/CMakeLists.txt | 6 +++--- tests/CMakeLists.txt | 6 +++--- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/bin/CMakeLists.txt b/src/bin/CMakeLists.txt index 478e4b909..1f749b89b 100644 --- a/src/bin/CMakeLists.txt +++ b/src/bin/CMakeLists.txt @@ -18,20 +18,20 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -add_geode_binary( - SOURCE "geode-triangulate-brep.cpp" - DEPENDENCIES - absl::flags - absl::flags_parse - absl::flags_usage - ${PROJECT_NAME}::mesh - ${PROJECT_NAME}::model -) -add_geode_binary( - SOURCE "geode-triangulate-section.cpp" - DEPENDENCIES - absl::flags - absl::flags_parse - absl::flags_usage - ${PROJECT_NAME}::model -) \ No newline at end of file +# add_geode_binary( +# SOURCE "geode-triangulate-brep.cpp" +# DEPENDENCIES +# absl::flags +# absl::flags_parse +# absl::flags_usage +# ${PROJECT_NAME}::mesh +# ${PROJECT_NAME}::model +# ) +# add_geode_binary( +# SOURCE "geode-triangulate-section.cpp" +# DEPENDENCIES +# absl::flags +# absl::flags_parse +# absl::flags_usage +# ${PROJECT_NAME}::model +# ) \ No newline at end of file diff --git a/src/geode/CMakeLists.txt b/src/geode/CMakeLists.txt index a3eacec4e..389d22003 100644 --- a/src/geode/CMakeLists.txt +++ b/src/geode/CMakeLists.txt @@ -21,6 +21,6 @@ #------------------------------------------------------------------------------------------------ # Configure the OpenGeode libraries add_subdirectory(basic) -add_subdirectory(geometry) -add_subdirectory(mesh) -add_subdirectory(model) +# add_subdirectory(geometry) +# add_subdirectory(mesh) +# add_subdirectory(model) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5da2c9423..fd0b3ed03 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -34,6 +34,6 @@ configure_file(${test_common_file_in} ${test_common_file}) include_directories(${PROJECT_BINARY_DIR}) add_subdirectory(basic) -add_subdirectory(geometry) -add_subdirectory(mesh) -add_subdirectory(model) +# add_subdirectory(geometry) +# add_subdirectory(mesh) +# add_subdirectory(model) From 9eea35c881bd62a19839df4dba963f450414e033 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:52:47 +0100 Subject: [PATCH 13/18] test --- src/geode/basic/database.cpp | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 0f1882962..3a3bab0f3 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -54,12 +54,13 @@ namespace geode ~Storage() { - terminate_storage(); - std::unique_lock< std::mutex > locking{ lock_ }; - condition_.wait( locking, [this] { - clean_queue(); - return queue_.empty(); - } ); + terminate_ = true; + condition_.notify_all(); + while( !queue_.empty() ) + { + queue_.front().wait(); + queue_.pop(); + } } bool expired() const @@ -110,23 +111,16 @@ namespace geode } private: - void terminate_storage() - { - terminate_ = true; - condition_.notify_all(); - } - void clean_queue() { while( !queue_.empty() ) { if( !queue_.front().ready() ) { - break; + return; } queue_.pop(); } - condition_.notify_all(); } void wait_for_memory_release() @@ -145,7 +139,7 @@ namespace geode } } locking.unlock(); - condition_.notify_all(); + // condition_.notify_all(); } ) ); } From 9b9af2a86ebb562850048b04653481d425fd1949 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:53:15 +0100 Subject: [PATCH 14/18] test --- src/bin/CMakeLists.txt | 34 +++++++++++++++++----------------- src/geode/CMakeLists.txt | 6 +++--- tests/CMakeLists.txt | 6 +++--- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/bin/CMakeLists.txt b/src/bin/CMakeLists.txt index 1f749b89b..478e4b909 100644 --- a/src/bin/CMakeLists.txt +++ b/src/bin/CMakeLists.txt @@ -18,20 +18,20 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -# add_geode_binary( -# SOURCE "geode-triangulate-brep.cpp" -# DEPENDENCIES -# absl::flags -# absl::flags_parse -# absl::flags_usage -# ${PROJECT_NAME}::mesh -# ${PROJECT_NAME}::model -# ) -# add_geode_binary( -# SOURCE "geode-triangulate-section.cpp" -# DEPENDENCIES -# absl::flags -# absl::flags_parse -# absl::flags_usage -# ${PROJECT_NAME}::model -# ) \ No newline at end of file +add_geode_binary( + SOURCE "geode-triangulate-brep.cpp" + DEPENDENCIES + absl::flags + absl::flags_parse + absl::flags_usage + ${PROJECT_NAME}::mesh + ${PROJECT_NAME}::model +) +add_geode_binary( + SOURCE "geode-triangulate-section.cpp" + DEPENDENCIES + absl::flags + absl::flags_parse + absl::flags_usage + ${PROJECT_NAME}::model +) \ No newline at end of file diff --git a/src/geode/CMakeLists.txt b/src/geode/CMakeLists.txt index 389d22003..a3eacec4e 100644 --- a/src/geode/CMakeLists.txt +++ b/src/geode/CMakeLists.txt @@ -21,6 +21,6 @@ #------------------------------------------------------------------------------------------------ # Configure the OpenGeode libraries add_subdirectory(basic) -# add_subdirectory(geometry) -# add_subdirectory(mesh) -# add_subdirectory(model) +add_subdirectory(geometry) +add_subdirectory(mesh) +add_subdirectory(model) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fd0b3ed03..5da2c9423 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -34,6 +34,6 @@ configure_file(${test_common_file_in} ${test_common_file}) include_directories(${PROJECT_BINARY_DIR}) add_subdirectory(basic) -# add_subdirectory(geometry) -# add_subdirectory(mesh) -# add_subdirectory(model) +add_subdirectory(geometry) +add_subdirectory(mesh) +add_subdirectory(model) From 3af66ba3f0b1a838a1b7fbe4b6abb1437fb853e7 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:54:20 +0100 Subject: [PATCH 15/18] test --- src/geode/basic/database.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 3a3bab0f3..5a431b989 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -139,7 +139,6 @@ namespace geode } } locking.unlock(); - // condition_.notify_all(); } ) ); } From 133b4263cf92025f8e24df8734b66601b5a61629 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 9 Dec 2022 11:58:42 +0100 Subject: [PATCH 16/18] test --- include/geode/basic/database.h | 6 +++--- src/geode/basic/database.cpp | 10 ++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/geode/basic/database.h b/include/geode/basic/database.h index b8f10ce06..3c920f555 100644 --- a/include/geode/basic/database.h +++ b/include/geode/basic/database.h @@ -52,9 +52,9 @@ namespace geode using serializer_function = std::function< void( PContext& ) >; /*! - * Classe holding a const reference of a data. - * @warning Do not destroy this Data class before the const reference - * obtained using its get() method is no longer in used + * Class holding a const reference of data. + * @warning Do not destroy this Data class if the const reference + * obtained using its get() method is still in used. */ class opengeode_basic_api Data { diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 5a431b989..40128f940 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -75,19 +75,18 @@ namespace geode void new_data_reference() { - const std::lock_guard< std::mutex > locking{ lock_ }; counter_++; last_++; } void delete_data_reference() { - const std::lock_guard< std::mutex > locking{ lock_ }; OPENGEODE_ASSERT( counter_ > 0, "[Database::Storage] Cannot decrement" ); counter_--; if( unused() ) { + const std::lock_guard< std::mutex > locking{ lock_ }; clean_queue(); wait_for_memory_release(); } @@ -125,7 +124,7 @@ namespace geode void wait_for_memory_release() { - const auto last = last_; + const auto last = last_.load(); queue_.emplace( async::spawn( [this, last] { std::unique_lock< std::mutex > locking{ lock_ }; if( !condition_.wait_for( @@ -138,17 +137,16 @@ namespace geode data_.reset(); } } - locking.unlock(); } ) ); } private: std::unique_ptr< Identifier > data_; std::atomic< bool > terminate_{ false }; - index_t counter_{ 0 }; + std::atomic< index_t > counter_{ 0 }; std::mutex lock_; std::condition_variable condition_; - index_t last_{ 0 }; + std::atomic< index_t > last_{ 0 }; std::queue< async::task< void > > queue_; }; From 996f46dfdc0abb06349a17ee5e09b51409c8271a Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Mon, 12 Dec 2022 13:08:28 +0100 Subject: [PATCH 17/18] test --- src/geode/basic/database.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 40128f940..3cbd315e5 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -55,9 +55,9 @@ namespace geode ~Storage() { terminate_ = true; - condition_.notify_all(); while( !queue_.empty() ) { + condition_.notify_all(); queue_.front().wait(); queue_.pop(); } From cb89487acf95357afc801990f528119a956da6ed Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Mon, 12 Dec 2022 13:38:46 +0100 Subject: [PATCH 18/18] test --- src/geode/basic/database.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/geode/basic/database.cpp b/src/geode/basic/database.cpp index 3cbd315e5..0788b7a26 100644 --- a/src/geode/basic/database.cpp +++ b/src/geode/basic/database.cpp @@ -54,10 +54,9 @@ namespace geode ~Storage() { - terminate_ = true; + terminate_storage(); while( !queue_.empty() ) { - condition_.notify_all(); queue_.front().wait(); queue_.pop(); } @@ -75,18 +74,19 @@ namespace geode void new_data_reference() { + const std::lock_guard< std::mutex > locking{ lock_ }; counter_++; last_++; } void delete_data_reference() { + const std::lock_guard< std::mutex > locking{ lock_ }; OPENGEODE_ASSERT( counter_ > 0, "[Database::Storage] Cannot decrement" ); counter_--; if( unused() ) { - const std::lock_guard< std::mutex > locking{ lock_ }; clean_queue(); wait_for_memory_release(); } @@ -110,6 +110,13 @@ namespace geode } private: + void terminate_storage() + { + const std::lock_guard< std::mutex > locking{ lock_ }; + terminate_ = true; + condition_.notify_all(); + } + void clean_queue() { while( !queue_.empty() ) @@ -124,12 +131,12 @@ namespace geode void wait_for_memory_release() { - const auto last = last_.load(); + const auto last = last_; queue_.emplace( async::spawn( [this, last] { std::unique_lock< std::mutex > locking{ lock_ }; if( !condition_.wait_for( locking, DATA_EXPIRATION, [this, last] { - return terminate_.load(); + return terminate_; } ) ) { if( last == last_ ) @@ -142,11 +149,11 @@ namespace geode private: std::unique_ptr< Identifier > data_; - std::atomic< bool > terminate_{ false }; - std::atomic< index_t > counter_{ 0 }; + bool terminate_{ false }; + index_t counter_{ 0 }; std::mutex lock_; std::condition_variable condition_; - std::atomic< index_t > last_{ 0 }; + index_t last_{ 0 }; std::queue< async::task< void > > queue_; };