From 425de7b857ce55cf2b22e6006c4cee12397b79f9 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Tue, 18 Mar 2025 10:41:03 +0100 Subject: [PATCH] Further small memory optimizations --- src/ifcparse/IfcEntityInstanceData.cpp | 1 + src/ifcparse/IfcEntityInstanceData.h | 66 +++++++++----------------- src/ifcparse/IfcFile.cpp | 26 ++++++++-- src/ifcparse/IfcFile.h | 20 +++----- src/ifcparse/IfcParse.cpp | 14 ++---- 5 files changed, 55 insertions(+), 72 deletions(-) diff --git a/src/ifcparse/IfcEntityInstanceData.cpp b/src/ifcparse/IfcEntityInstanceData.cpp index 40e47bf141..b4d28d8718 100644 --- a/src/ifcparse/IfcEntityInstanceData.cpp +++ b/src/ifcparse/IfcEntityInstanceData.cpp @@ -264,6 +264,7 @@ bool impl::serialize(std::string& val, const aggregate_of_instance::ptr& t) bool impl::serialize(std::string& val, const aggregate_of_aggregate_of_instance::ptr& t) { + // @todo return false; } diff --git a/src/ifcparse/IfcEntityInstanceData.h b/src/ifcparse/IfcEntityInstanceData.h index cfcb466af5..7117212e08 100644 --- a/src/ifcparse/IfcEntityInstanceData.h +++ b/src/ifcparse/IfcEntityInstanceData.h @@ -386,11 +386,6 @@ struct AttributeValue { struct rocks_db_attribute_storage { public: - size_t size(void*, const IfcParse::declaration*, std::size_t identity) const { - // @todo is this actually needed? - return 8; - } - // @todo void* is obviously very ugly here template void set(void* storage, const IfcParse::declaration*, std::size_t identity, std::size_t index, const T& value); @@ -407,20 +402,19 @@ public: class IFC_PARSE_API IfcEntityInstanceData { public: - // @todo since rocks_db_attribute_storage has no members anymore, change to in_memory_attribute_storage*? - // 24 -> 8 bytes... - std::variant storage_; + // Since rocks_db_attribute_storage has no members this is not a variant but in_memory*, where nullptr means a rocks_db_attribute_storage is constructed on the fly given the context from instance data. + in_memory_attribute_storage* storage_; IfcEntityInstanceData(in_memory_attribute_storage&& storage) - : storage_(std::move(storage)) + : storage_(new in_memory_attribute_storage(std::move(storage))) {} - IfcEntityInstanceData(rocks_db_attribute_storage&& storage) - : storage_(std::move(storage)) + IfcEntityInstanceData(rocks_db_attribute_storage&&) + : storage_(nullptr) {} IfcEntityInstanceData(IfcEntityInstanceData&& other) noexcept - : storage_(std::move(other.storage_)) + : storage_(other.storage_) {} // No copy-constructor anymore because we need the instance for storage model context @@ -428,7 +422,7 @@ class IFC_PARSE_API IfcEntityInstanceData { IfcEntityInstanceData& operator=(IfcEntityInstanceData&& other) { if (this != &other) { - storage_ = std::move(other.storage_); + storage_ = other.storage_; } return *this; } @@ -437,45 +431,29 @@ class IFC_PARSE_API IfcEntityInstanceData { template void set_attribute_value(void* storage, const IfcParse::declaration* decl, std::size_t identity, std::size_t index, T&& value) { - std::visit([&index, &value, storage, decl, identity](auto& x) { - if constexpr (std::is_same_v, in_memory_attribute_storage>) { - return x.set(index, value); - } else { - return x.set(storage, decl, identity, index, value); - } - }, storage_); + if (storage_) { + storage_->set(index, value); + } else { + rocks_db_attribute_storage{}.set(storage, decl, identity, index, value); + } } template bool has_attribute_value(void* storage, const IfcParse::declaration* decl, std::size_t identity, std::size_t index) const { - return std::visit([&index, storage, decl, identity](const auto& x) { - if constexpr (std::is_same_v, in_memory_attribute_storage>) { - return x.has(index); - } else { - return x.has(storage, decl, identity, index); - } - }, storage_); + if (storage_) { + return storage_->has(index); + } else { + return rocks_db_attribute_storage{}.has(storage, decl, identity, index); + } } template auto apply_visitor(void* storage, const IfcParse::declaration* decl, std::size_t identity, Visitor&& visitor, std::size_t index) const { - return std::visit([&index, &visitor, storage, decl, identity](const auto& x) { - if constexpr (std::is_same_v, in_memory_attribute_storage>) { - return x.apply_visitor(std::forward(visitor), index); - } else { - return x.apply_visitor(storage, decl, identity, index, std::forward(visitor)); - } - }, storage_); - } - - size_t size(void* storage, const IfcParse::declaration* decl, std::size_t identity) const { - return std::visit([storage, decl, identity](const auto& x) { - if constexpr (std::is_same_v, in_memory_attribute_storage>) { - return x.size(); - } else { - return x.size(storage, decl, identity); - } - }, storage_); + if (storage_) { + return storage_->apply_visitor(std::forward(visitor), index); + } else { + return rocks_db_attribute_storage{}.apply_visitor(storage, decl, identity, index, std::forward(visitor)); + } } void toString(void* storage, const IfcParse::declaration*, std::size_t identity, std::ostream&, bool upper = false) const; diff --git a/src/ifcparse/IfcFile.cpp b/src/ifcparse/IfcFile.cpp index f1c3ea4c1a..f0e6b328d6 100644 --- a/src/ifcparse/IfcFile.cpp +++ b/src/ifcparse/IfcFile.cpp @@ -349,9 +349,16 @@ IfcParse::impl::rocks_db_file_storage::rocksdb_types_iterator::value_type const& } IfcUtil::IfcBaseClass* IfcParse::impl::rocks_db_file_storage::assert_existance(size_t number, instance_ref r) { - decltype(instance_cache_)::const_iterator it = instance_cache_.find({ r, number }); - if (it != instance_cache_.end()) { - return it->second; + if (r == IfcParse::impl::rocks_db_file_storage::entityinstance_ref) { + auto it = instance_cache_.find(number); + if (it != instance_cache_.end()) { + return it->second; + } + } else { + auto it = type_instance_cache_.find(number); + if (it != type_instance_cache_.end()) { + return it->second; + } } std::string v; @@ -369,10 +376,19 @@ IfcUtil::IfcBaseClass* IfcParse::impl::rocks_db_file_storage::assert_existance(s throw std::runtime_error("Incorrect reference"); } IfcEntityInstanceData data(rocks_db_attribute_storage{}); - auto inst = file->schema()->instantiate(decl, std::move(data)); + IfcUtil::IfcBaseClass* inst; + if (file->instantiate_typed_instances) { + inst = file->schema()->instantiate(decl, std::move(data)); + } else { + inst = new IfcUtil::IfcLateBoundEntity(decl, std::move(data)); + } inst->id_ = number; inst->file_ = file; - instance_cache_.insert({ {r, number}, inst }); + if (r == IfcParse::impl::rocks_db_file_storage::entityinstance_ref) { + instance_cache_.insert({ number, inst }); + } else { + type_instance_cache_.insert({ number, inst }); + } return inst; } else { throw IfcException("Instance #" + boost::lexical_cast(number) + " not found"); diff --git a/src/ifcparse/IfcFile.h b/src/ifcparse/IfcFile.h index 4189049e54..1a03e78784 100644 --- a/src/ifcparse/IfcFile.h +++ b/src/ifcparse/IfcFile.h @@ -60,21 +60,12 @@ namespace { virtual bool Merge(const rocksdb::Slice&, - const rocksdb::Slice* existing_value, - const rocksdb::Slice& value, - std::string* new_value, + const rocksdb::Slice*, + const rocksdb::Slice&, + std::string*, rocksdb::Logger*) const override { return false; - /* - if (existing_value) { - new_value->assign(existing_value->data(), existing_value->size()); - new_value->append(value.data(), value.size()); - } else { - new_value->assign(value.data(), value.size()); - } - return true; - */ } virtual const char* Name() const override { @@ -359,8 +350,8 @@ namespace impl { // to make sure that instance pointer are constant during file lifetime // cache instances because we want stable pointers // @todo this is silly, but we cannot have the same type, this should be just a pointer then on the IfcFile side? - typedef std::map, IfcUtil::IfcBaseClass*> entity_by_iden_cache_t; - entity_by_iden_cache_t instance_cache_; + typedef std::map entity_by_iden_cache_t; + entity_by_iden_cache_t instance_cache_, type_instance_cache_; // @todo all these size_ts should probably be uint32_t for consistency with in-mem storage @@ -619,6 +610,7 @@ public: bool check_existance_before_adding = true; bool calculate_unit_factors = true; + bool instantiate_typed_instances = true; // @todo temporarily public for header storage_t storage_; diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index 1b86235f3e..b8242d5caf 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -2694,15 +2694,11 @@ IfcEntityInstanceData::IfcEntityInstanceData(const IfcEntityInstanceData& data) AttributeValue IfcEntityInstanceData::get_attribute_value(void* storage, const IfcParse::declaration* decl, std::size_t identity, size_t index) const { - return std::visit([this, storage, decl, identity, index](const auto& x) { - if constexpr (std::is_same_v, in_memory_attribute_storage>) { - return AttributeValue(&x, (uint8_t)index); - } else if constexpr (std::is_same_v, rocks_db_attribute_storage>) { - return AttributeValue((IfcParse::impl::rocks_db_file_storage*) storage, identity, decl->as_entity() ? 1 : 0, index); - } else { - return AttributeValue{}; - } - }, storage_); + if (storage_) { + return AttributeValue(storage_, (uint8_t)index); + } else { + return AttributeValue((IfcParse::impl::rocks_db_file_storage*)storage, identity, decl->as_entity() ? 1 : 0, index); + } } bool IfcParse::impl::rocks_db_file_storage::read_schema(const IfcParse::schema_definition*& schema) {