From d94215360f55a322d62936da58f7ddfa54dd71c2 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Thu, 17 Sep 2026 20:47:33 +1000 Subject: [PATCH] [AI-generated, unverified] ifcparse: make deleting and creating instances work on RocksDB-backed files (#9508) * ifcparse: make deleting and creating instances work on RocksDB-backed files file.remove() on a RocksDB-backed file segfaulted. Reducing it turned up five gaps that each made editing such a file crash or silently do nothing: - process_deletion_inverse() decoded the v| inverse-record values as size_t while the serializer, register_inverse(), unregister_inverse() and instances_by_reference() use uint32_t, so std::find failed and vals.erase(end()) was undefined behaviour. It also took the DeleteRange end from an iterator that is invalid when the instance has no inverse records. Decode as uint32_t, remove every occurrence guarded on "found", and derive the range end from the prefix itself. - attribute_value::size() ignored storage_model_, so every aggregate assignment on a RocksDB instance threw "Invalid variant index" from set_attribute_value(). Branch on the storage model like the sibling accessors and count the deserialized aggregate. - rocks_db_file_storage::create() was a stub returning an empty handle, which anything creating an instance then dereferenced. Implement it after in_memory_file_storage::create(). - max_id_ is only initialised by the in-memory parse, so a RocksDB file would have handed out ids that overwrite existing instances. Implement the recalculate_id_counter() stub per backend and run it once before the first fresh_id() on RocksDB. - byid_.erase() was a no-op: set_to_map_transformer::erase() and rocksdb_set_view::erase() were stubs. The deleted instance's attribute keys and cached handle survived, entity_names() still listed it and reopening the database threw. Erase deletes every key under the instance's prefix; the transformer forwards to it and takes an on-erase hook the storage uses to drop the cached handle. root.remove_product on the first 200 products of a 61 MB model now leaves the same surviving ids and inverse counts whether the file was opened from SPF or converted to RocksDB. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH * ifcparse: map argument_type to its stored type, size() as size_t Review: argument_type enumerates the members of type_variant_parameter_pack in order, so express that once as argument_storage_type_t (pinned by static_asserts) and let attribute_value::size() on RocksDB go through a single aggregate_size_() helper instead of spelling each vector type out in the switch. size() now returns size_t; its only caller already took size_t. Also build the RocksDB DeleteRange upper bounds as prefix + ('|' + 1) rather than a literal '}', which read as the {id} placeholder notation. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH * ifcparse: fixed-width hex ids in RocksDB keys, batched instance erasure Review: recalculate_id_counter() scanned every i||_ record because ids were written as decimal text, which doesn't sort numerically. Make every numeric key segment fixed-width 16-digit lowercase hex, produced and parsed by key_to_string()/key_from_string() in rocksdb_map_adapter.h, with named builders (rocksdb_key::attribute, header_attribute, type_record, inverse, inverse_prefix, type_list, upper_bound) that the storage, entity_instance_data.cpp, read_schema() and the serializer use instead of assembling "i|" + std::to_string(id) + ... by hand. Keys now sort by id, an instance's records are contiguous in id order, and the largest id is the last key under i|, which recalculate_id_counter() seeks to. The two dormant to_string_fixed_width() helpers are gone. This changes the on-disk layout; databases converted before this commit have to be re-converted. Also review: instance_cache_ eviction went through an on-erase hook on set_to_map_transformer, a std::function call under a mutex per deleted instance. Drop the hook; file::remove_entity() and unbatch() call file::erase_instances_(ids), which on RocksDB is rocks_db_file_storage::erase_instances(): one WriteBatch of DeleteRanges and one lock for the whole batch. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH * ifcopenshell-python: test_rocks queries the fixed-width hex keys The test looked up raw keys in the decimal layout ("i|139|5", "t||0"); numeric key segments are fixed-width hex now. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH --------- Co-authored-by: Claude Fable 5.1 --- ...st_streaming_rocksdb_and_simpletyperefs.py | 6 +- src/ifcparse/entity_instance_data.cpp | 74 +++++++---- src/ifcparse/file.cpp | 117 ++++++++++++------ src/ifcparse/file.h | 1 + src/ifcparse/instance_data.h | 21 +++- src/ifcparse/parse.cpp | 84 ++++++++----- src/ifcparse/rocksdb_map_adapter.h | 60 ++++++++- src/ifcparse/rocksdb_set_view.h | 26 +++- src/ifcparse/set_to_map_transformer.h | 4 +- src/ifcparse/storage.h | 7 ++ src/serializers/rocks_db_serializer.cpp | 24 +--- 11 files changed, 294 insertions(+), 130 deletions(-) diff --git a/src/ifcopenshell-python/test/test_streaming_rocksdb_and_simpletyperefs.py b/src/ifcopenshell-python/test/test_streaming_rocksdb_and_simpletyperefs.py index a10a6ee5d4..d7d2e7f440 100644 --- a/src/ifcopenshell-python/test/test_streaming_rocksdb_and_simpletyperefs.py +++ b/src/ifcopenshell-python/test/test_streaming_rocksdb_and_simpletyperefs.py @@ -111,9 +111,11 @@ def test_rocks(): assert f[139].RelatingPropertyDefinition.is_a("IfcPropertySetDefinitionSet") assert {x.id() for x in f[139].RelatingPropertyDefinition[0]} == {136, 138} - b = f.key_value_store_query("i|139|5")[2:] + # Numeric key segments are fixed-width hex: i||, + # t||. See rocksdb_map_adapter.h. + b = f.key_value_store_query(f"i|{139:016x}|{5:016x}")[2:] iden = struct.unpack("Q", b)[0] - b = f.key_value_store_query(f"t|{iden}|0")[1:] + b = f.key_value_store_query(f"t|{iden:016x}|{0:016x}")[1:] assert set(struct.unpack("Q", b[i : i + 8])[0] for i in range(1, len(b), 9)) == {136, 138} g = ifcopenshell.open(fn) diff --git a/src/ifcparse/entity_instance_data.cpp b/src/ifcparse/entity_instance_data.cpp index b5d38d4280..fa9d61f894 100644 --- a/src/ifcparse/entity_instance_data.cpp +++ b/src/ifcparse/entity_instance_data.cpp @@ -65,9 +65,7 @@ namespace { { std::string str; array_.db_ptr->db->Get(rocksdb::ReadOptions{}, - (is_header ? "h|" : (entity_or_type->as_entity() ? "i|" : "t|")) + - (is_header ? entity_or_type->name() : std::to_string(instance_name_)) + "|" + - std::to_string(index_), &str); + (is_header ? rocksdb_key::header_attribute(entity_or_type->name(), index_) : rocksdb_key::attribute(entity_or_type->as_entity() != nullptr, instance_name_, index_)), &str); ::impl::deserialize(array_.db_ptr, str, val); } else { static_assert( @@ -97,9 +95,7 @@ namespace { std::string str; const bool is_header = entity_or_type->schema() == &Header_section_schema::get_schema(); array_.db_ptr->db->Get(rocksdb::ReadOptions{}, - (is_header ? "h|" : (entity_or_type->as_entity() ? "i|" : "t|")) + - (is_header ? entity_or_type->name() : std::to_string(instance_name_)) + "|" + - std::to_string(index_), &str); + (is_header ? rocksdb_key::header_attribute(entity_or_type->name(), index_) : rocksdb_key::attribute(entity_or_type->as_entity() != nullptr, instance_name_, index_)), &str); if constexpr (std::is_same_v) { if (str.size() == 0) { return true; @@ -125,9 +121,7 @@ namespace { std::string str; const bool is_header = entity_or_type->schema() == &Header_section_schema::get_schema(); if (!array_.db_ptr->db->Get(rocksdb::ReadOptions{}, - (is_header ? "h|" : (entity_or_type->as_entity() ? "i|" : "t|")) + - (is_header ? entity_or_type->name() : std::to_string(instance_name_)) + "|" + - std::to_string(index_), &str).ok()) { + (is_header ? rocksdb_key::header_attribute(entity_or_type->name(), index_) : rocksdb_key::attribute(entity_or_type->as_entity() != nullptr, instance_name_, index_)), &str).ok()) { return type_encoder::encode_type() - 'A'; } return (size_t) str[0] - 'A'; @@ -135,6 +129,12 @@ namespace { #endif throw std::logic_error("RocksDB storage is unavailable"); } + + template + inline size_t aggregate_size_(attribute_value::pointer_type array_, uint8_t storage_model_, size_t instance_name_, const ifcopenshell::declaration* entity_or_type, uint8_t index_) + { + return dispatch_get_>(array_, storage_model_, instance_name_, entity_or_type, index_).size(); + } } attribute_value::operator int64_t() const @@ -173,9 +173,7 @@ attribute_value::operator std::string() const std::string str; const bool is_header = entity_or_type_->schema() == &Header_section_schema::get_schema(); array_.db_ptr->db->Get(rocksdb::ReadOptions{}, - (is_header ? "h|" : (entity_or_type_->as_entity() ? "i|" : "t|")) + - (is_header ? entity_or_type_->name() : std::to_string(instance_name_)) + "|" + - std::to_string(index_), &str); + (is_header ? rocksdb_key::header_attribute(entity_or_type_->name(), index_) : rocksdb_key::attribute(entity_or_type_->as_entity() != nullptr, instance_name_, index_)), &str); size_t v; memcpy(&v, str.data() + 1, sizeof(size_t)); auto decl = array_.db_ptr->file->schema()->declarations()[v]->as_enumeration_type(); @@ -197,9 +195,7 @@ attribute_value::operator enumeration_reference() const std::string str; const bool is_header = entity_or_type_->schema() == &Header_section_schema::get_schema(); array_.db_ptr->db->Get(rocksdb::ReadOptions{}, - (is_header ? "h|" : (entity_or_type_->as_entity() ? "i|" : "t|")) + - (is_header ? entity_or_type_->name() : std::to_string(instance_name_)) + "|" + - std::to_string(index_), &str); + (is_header ? rocksdb_key::header_attribute(entity_or_type_->name(), index_) : rocksdb_key::attribute(entity_or_type_->as_entity() != nullptr, instance_name_, index_)), &str); size_t v; memcpy(&v, str.data() + 1, sizeof(size_t)); auto decl = array_.db_ptr->file->schema()->declarations()[v]->as_enumeration_type(); @@ -225,9 +221,7 @@ attribute_value::operator express::base () const std::string str; const bool is_header = entity_or_type_->schema() == &Header_section_schema::get_schema(); array_.db_ptr->db->Get(rocksdb::ReadOptions{}, - (is_header ? "h|" : (entity_or_type_->as_entity() ? "i|" : "t|")) + - (is_header ? entity_or_type_->name() : std::to_string(instance_name_)) + "|" + - std::to_string(index_), &str); + (is_header ? rocksdb_key::header_attribute(entity_or_type_->name(), index_) : rocksdb_key::attribute(entity_or_type_->as_entity() != nullptr, instance_name_, index_)), &str); size_t v; memcpy(&v, str.data() + 2, sizeof(size_t)); if (str.size() > 1 && str[1] == 'i') { @@ -289,10 +283,40 @@ bool attribute_value::isNull() const return dispatch_has_(array_, storage_model_, instance_name_, entity_or_type_, index_); } -unsigned int attribute_value::size() const +size_t attribute_value::size() const { - // @todo - return array_.storage_ptr->apply_visitor(size_visitor{}, index_); + if (storage_model_ == 0) { + return (size_t)array_.storage_ptr->apply_visitor(size_visitor{}, index_); + } +#ifdef IFOPSH_WITH_ROCKSDB + else { + // Same answers as size_visitor: element count for aggregates, -1 otherwise. + switch (type()) { + case Argument_EMPTY_AGGREGATE: + case Argument_AGGREGATE_OF_EMPTY_AGGREGATE: + return 0; + case Argument_AGGREGATE_OF_INT: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_DOUBLE: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_STRING: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_BINARY: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_ENTITY_INSTANCE: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_AGGREGATE_OF_INT: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_AGGREGATE_OF_DOUBLE: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + case Argument_AGGREGATE_OF_AGGREGATE_OF_ENTITY_INSTANCE: + return aggregate_size_(array_, storage_model_, instance_name_, entity_or_type_, index_); + default: + return (size_t)-1; + } + } +#endif + throw std::logic_error("RocksDB storage is unavailable"); } ifcopenshell::argument_type attribute_value::type() const @@ -512,9 +536,7 @@ bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declarat std::string v; auto success = rdb_storage->db->Get( rocksdb::ReadOptions{}, - (is_header ? "h|" : (decl->as_entity() ? "i|" : "t|")) + - (is_header ? decl->name() : std::to_string(identity)) + "|" + - std::to_string(index), &v); + (is_header ? rocksdb_key::header_attribute(decl->name(), index) : rocksdb_key::attribute(decl->as_entity() != nullptr, identity, index)), &v); if constexpr (std::is_same_v, blank>) { if (!success.ok()) { return true; @@ -532,9 +554,7 @@ void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declarat ::impl::serialize(v, value); rdb_storage->db->Put( rdb_storage->wopts, - (is_header ? "h|" : (decl->as_entity() ? "i|" : "t|")) + - (is_header ? decl->name() : std::to_string(identity)) + "|" + - std::to_string(index), v); + (is_header ? rocksdb_key::header_attribute(decl->name(), index) : rocksdb_key::attribute(decl->as_entity() != nullptr, identity, index)), v); } template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const blank& value); diff --git a/src/ifcparse/file.cpp b/src/ifcparse/file.cpp index 8c805e52c2..09b0ad91a6 100644 --- a/src/ifcparse/file.cpp +++ b/src/ifcparse/file.cpp @@ -55,7 +55,7 @@ express::base ifcopenshell::impl::rocks_db_file_storage::assert_existance(size_t std::string v; - rocksdb::Status s = db->Get(rocksdb::ReadOptions{}, (r == entityinstance_ref ? "i|" : "t|") + std::to_string(number) + "|_", &v); + rocksdb::Status s = db->Get(rocksdb::ReadOptions{}, rocksdb_key::type_record(r == entityinstance_ref, number), &v); if (s.ok()) { size_t s; memcpy(&s, v.data(), sizeof(size_t)); @@ -207,22 +207,20 @@ void ifcopenshell::impl::rocks_db_file_storage::process_deletion_inverse(const e auto id = inst.id(); { - // compute next prefix that does not start with v|{id}| - auto prefix = "v|" + std::to_string(id) + "|"; - auto it = std::unique_ptr(db->NewIterator(rocksdb::ReadOptions())); - it->Seek(prefix); - while (it->Valid()) { - it->Next(); - if (!it->key().starts_with(prefix)) { - break; - } - } + // Delete every record referencing inst: all keys under v||. The + // exclusive upper bound is the same prefix with its separator + // incremented, so no iterator is needed to find the range end. + const auto prefix = rocksdb_key::inverse_prefix(id); + const auto upper_bound = rocksdb_key::upper_bound(prefix); rocksdb::WriteBatch batch; - batch.DeleteRange(prefix, it->key()); + batch.DeleteRange(prefix, upper_bound); db->Write(wopts, &batch); } + // Delete the records inst contributed through its own attributes: drop + // its id from the value lists of every instance it references. The values + // are uint32_t ids, as written by the serializer and register_inverse(). // This is based on traversal which needs instances to still be contained in the map. // another option would be to keep byid intact for the remainder of this loop auto entity_attributes = traverse(inst, 1); @@ -233,32 +231,51 @@ void ifcopenshell::impl::rocks_db_file_storage::process_deletion_inverse(const e const unsigned int name = entity_attribute.id(); // Do not update inverses for simple types (which have id()==0 in IfcOpenShell). if (name != 0) { - // Find instances entity -> other - // and update inverses from entity into other + auto prefix = rocksdb_key::inverse_prefix(name); + auto it = std::unique_ptr(db->NewIterator(rocksdb::ReadOptions())); + it->Seek(prefix); + while (it->Valid() && it->key().starts_with(prefix)) { + std::string s = it->value().ToString(); - { - auto prefix = "v|" + std::to_string(name) + "|"; - auto it = std::unique_ptr(db->NewIterator(rocksdb::ReadOptions())); - it->Seek(prefix); - while (it->Valid() && it->key().starts_with(prefix)) { - std::string s = it->value().ToString(); - - // Iterator are snapshotted? So don't get invalidated? - std::vector vals(s.size() / sizeof(size_t)); - memcpy(vals.data(), s.data(), s.size()); - vals.erase(std::find(vals.begin(), vals.end(), (size_t)id)); - s.resize(vals.size() * sizeof(size_t)); + // Iterator are snapshotted? So don't get invalidated? + std::vector vals(s.size() / sizeof(uint32_t)); + memcpy(vals.data(), s.data(), s.size()); + auto removed = std::remove(vals.begin(), vals.end(), (uint32_t)id); + if (removed != vals.end()) { + vals.erase(removed, vals.end()); + s.resize(vals.size() * sizeof(uint32_t)); memcpy(s.data(), vals.data(), s.size()); db->Put(wopts, it->key(), s); - - it->Next(); } + + it->Next(); } } } #endif } +void ifcopenshell::impl::rocks_db_file_storage::erase_instances(const std::vector& ids) +{ +#ifndef IFOPSH_WITH_ROCKSDB + (void)ids; +#endif +#ifdef IFOPSH_WITH_ROCKSDB + // One write for every instance's keys, one lock for their cached handles. + rocksdb::WriteBatch batch; + for (auto id : ids) { + const auto prefix = rocksdb_key::instance(true, id); + batch.DeleteRange(prefix, rocksdb_key::upper_bound(prefix)); + } + db->Write(wopts, &batch); + + std::lock_guard lock(instance_cache_mutex_); + for (auto id : ids) { + instance_cache_.erase(id); + } +#endif +} + express::base ifcopenshell::impl::in_memory_file_storage::instance_by_id(int id) { auto it = byid_.find(id); @@ -314,19 +331,47 @@ ifcopenshell::filetype ifcopenshell::guess_file_type(const std::string& fn) { } express::base ifcopenshell::impl::rocks_db_file_storage::create(const ifcopenshell::declaration* decl, int id) { +#ifndef IFOPSH_WITH_ROCKSDB (void)decl; (void)id; - return express::base{}; - /* - if (decl->as_entity() || decl->as_type_declaration()) { - auto* inst = file->schema()->instantiate(decl, rocks_db_attribute_storage{}); - // @todo maybe this needs to be set to file? In order to have a context (ie. rocksdb::db*) to write to? - inst->file_ = nullptr; - return file->add_entity(inst); + throw exception("RocksDB support not compiled in"); +#else + // Mirrors in_memory_file_storage::create(). The instance's attributes + // live in the database (written by set_attribute_value(), read back on + // access), so the cache only has to keep the identity of the handle + // stable: assert_existance() can reload it from the type record that + // add_type_ref() writes. + uint32_t instance_name; + if (decl->as_entity() != nullptr) { + if (id == -1) { + if (!id_counter_recalculated_) { + file->recalculate_id_counter(); + id_counter_recalculated_ = true; + } + instance_name = file->fresh_id(); + } else { + instance_name = id; + } + } else if (decl->as_type_declaration() != nullptr) { + instance_name = 0; } else { throw std::runtime_error("Requires and entity or type declaration"); } - */ + auto data = ifcopenshell::make_pointer_type(file, decl, instance_name, rocks_db_attribute_storage{}); + { + std::lock_guard lock(instance_cache_mutex_); + if (instance_name) { + instance_cache_.insert({instance_name, data}); + } else { + type_instance_cache_.insert({data->identity(), data}); + } + } + + express::base inst(data); + add_type_ref(inst); + + return inst; +#endif } express::base ifcopenshell::impl::in_memory_file_storage::create(const ifcopenshell::declaration* decl, int id) { diff --git a/src/ifcparse/file.h b/src/ifcparse/file.h index 852b74f28b..76b3e1c1b3 100644 --- a/src/ifcparse/file.h +++ b/src/ifcparse/file.h @@ -234,6 +234,7 @@ public: batch_deletion_ids_t batch_deletion_ids_; bool batch_mode_ = false; void process_deletion_(const express::base& entity); + void erase_instances_(const std::vector& ids); public: #ifdef USE_MMAP diff --git a/src/ifcparse/instance_data.h b/src/ifcparse/instance_data.h index 7d4061d567..ad41e09c11 100644 --- a/src/ifcparse/instance_data.h +++ b/src/ifcparse/instance_data.h @@ -32,6 +32,7 @@ #undef Handle #include +#include #pragma pop_macro("Handle") @@ -226,6 +227,24 @@ struct pack_to_variant_array> { using in_memory_attribute_storage = pack_to_variant_array::type; +// argument_type enumerates the members of type_variant_parameter_pack in +// order, so a member maps back to the type stored for it. +template +struct pack_element; + +template +struct pack_element> { + template + using type = std::tuple_element_t>; +}; + +template +using argument_storage_type_t = typename pack_element::template type; + +static_assert(std::is_same_v, int64_t>, "argument_type must enumerate type_variant_parameter_pack in order"); +static_assert(std::is_same_v, std::vector>, "argument_type must enumerate type_variant_parameter_pack in order"); +static_assert(std::is_same_v, std::vector>>, "argument_type must enumerate type_variant_parameter_pack in order"); + template struct type_encoder_impl; @@ -427,7 +446,7 @@ public: operator enumeration_reference() const; bool isNull() const; - unsigned int size() const; + size_t size() const; ifcopenshell::argument_type type() const; diff --git a/src/ifcparse/parse.cpp b/src/ifcparse/parse.cpp index 8ab7fe12e5..c5beb66494 100644 --- a/src/ifcparse/parse.cpp +++ b/src/ifcparse/parse.cpp @@ -1099,16 +1099,6 @@ void ifcopenshell::impl::in_memory_file_storage::unregister_inverse(unsigned id_ } } -namespace { - template - std::string to_string_fixed_width(const T& t, size_t) { - // @todo currently inactive - std::ostringstream oss; - oss << /*std::setfill('0') << std::setw(w) <<*/ t; - return oss.str(); - } -} - void ifcopenshell::impl::rocks_db_file_storage::register_inverse(unsigned id_from, const ifcopenshell::entity* from_entity, int inst_id, int attribute_index) { #ifndef IFOPSH_WITH_ROCKSDB (void)id_from; @@ -1122,7 +1112,7 @@ void ifcopenshell::impl::rocks_db_file_storage::register_inverse(unsigned id_fro s.resize(sizeof(uint32_t)); memcpy(s.data(), &v, sizeof(uint32_t)); - auto key = "v|" + to_string_fixed_width(inst_id, 10) + "|" + to_string_fixed_width(from_entity->index_in_schema(), 4) + "|" + to_string_fixed_width(attribute_index, 2); + auto key = rocksdb_key::inverse(inst_id, from_entity->index_in_schema(), attribute_index); db->Merge(wopts, key, s); /* @@ -1147,7 +1137,7 @@ void ifcopenshell::impl::rocks_db_file_storage::unregister_inverse(unsigned id_f #ifdef IFOPSH_WITH_ROCKSDB static std::string s; auto inst_id = inst.id(); - auto key = "v|" + to_string_fixed_width(inst_id, 10) + "|" + to_string_fixed_width(from_entity->index_in_schema(), 4) + "|" + to_string_fixed_width(attribute_index, 2); + auto key = rocksdb_key::inverse(inst_id, from_entity->index_in_schema(), attribute_index); if (db->Get(rocksdb::ReadOptions{}, key, &s).ok()) { std::vector vals(s.size() / sizeof(uint32_t)); memcpy(vals.data(), s.data(), s.size()); @@ -1178,12 +1168,12 @@ void ifcopenshell::impl::rocks_db_file_storage::add_type_ref(const express::base memcpy(s.data(), &v, sizeof(size_t)); // no merges yet, because the python client doesn't support them - db->Merge(wopts, "t|" + std::to_string(new_entity.declaration().index_in_schema()), s); + db->Merge(wopts, rocksdb_key::type_list(new_entity.declaration().index_in_schema()), s); /*{ std::string current; // @todo this uses the same key-namespace as typedecl instances, not a direct conflict, but also not very clear - auto key = "t|" + std::to_string(new_entity.declaration().index_in_schema()); + auto key = rocksdb_key::type_list(new_entity.declaration().index_in_schema()); db->Get(rocksdb::ReadOptions{}, key, ¤t); auto new_val = current + s; db->Put(wopts, key, new_val); @@ -1193,7 +1183,7 @@ void ifcopenshell::impl::rocks_db_file_storage::add_type_ref(const express::base // not only mapping also register type v = new_entity.declaration().index_in_schema(); memcpy(s.data(), &v, sizeof(size_t)); - db->Put(wopts, (new_entity.declaration().as_entity() ? "i|" : "t|") + std::to_string(new_entity.id() ? new_entity.id() : new_entity.identity()) + "|_", s); + db->Put(wopts, rocksdb_key::type_record(new_entity.declaration().as_entity() != nullptr, new_entity.id() ? new_entity.id() : new_entity.identity()), s); #endif } @@ -1205,7 +1195,7 @@ void ifcopenshell::impl::rocks_db_file_storage::remove_type_ref(const express::b #ifdef IFOPSH_WITH_ROCKSDB if (new_entity.declaration().as_entity()) { std::string s; - auto key = "t|" + std::to_string(new_entity.declaration().index_in_schema()); + auto key = rocksdb_key::type_list(new_entity.declaration().index_in_schema()); if (db->Get(rocksdb::ReadOptions{}, key, &s).ok()) { std::vector vals(s.size() / sizeof(size_t)); memcpy(vals.data(), s.data(), s.size()); @@ -1216,7 +1206,7 @@ void ifcopenshell::impl::rocks_db_file_storage::remove_type_ref(const express::b } } - db->Delete(wopts, (new_entity.declaration().as_entity() ? "i|" : "t|") + std::to_string(new_entity.id() ? new_entity.id() : new_entity.identity()) + "|_"); + db->Delete(wopts, rocksdb_key::type_record(new_entity.declaration().as_entity() != nullptr, new_entity.id() ? new_entity.id() : new_entity.identity())); #endif } @@ -2636,16 +2626,30 @@ template void ifcopenshell::impl::in_memory_file_storage::read_from_stream(file_ #endif void file::recalculate_id_counter() { - /* - // @todo - entity_by_id::key_type k = 0; - for (auto& p : byid_) { - if (p.first > k) { - k = p.first; + unsigned int k = 0; + std::visit([&k](auto& x) { + if constexpr (std::is_same_v, impl::in_memory_file_storage>) { + for (const auto& p : x.byid_) { + k = std::max(k, (unsigned int)p.first); + } } - } - max_id_ = (unsigned int)k; - */ +#ifdef IFOPSH_WITH_ROCKSDB + else if constexpr (std::is_same_v, impl::rocks_db_file_storage>) { + // Ids are fixed-width hex in the keys, so the largest id owns the + // last key under the entity prefix. + const std::string prefix = "i|"; + auto it = std::unique_ptr(x.db->NewIterator(rocksdb::ReadOptions())); + it->SeekForPrev(rocksdb_key::upper_bound(prefix)); + if (it->Valid() && it->key().starts_with(prefix)) { + k = (unsigned int)parse_hex_key(it->key().ToString().substr(prefix.size(), 16)); + } + } +#endif + else { + throw std::runtime_error("Storage not initialized"); + } + }, storage_); + max_id_ = k; } class traversal_recorder { @@ -2940,10 +2944,24 @@ void file::remove_entity(const express::base& entity) { batch_deletion_ids_.push_back(id); } else { process_deletion_(entity); - byid_.erase(entity.id()); + erase_instances_({(uint32_t)id}); } } +void file::erase_instances_(const std::vector& ids) { + std::visit([this, &ids](auto& x) { + if constexpr (std::is_same_v, impl::in_memory_file_storage>) { + for (auto id : ids) { + byid_.erase(id); + } + } else if constexpr (std::is_same_v, impl::rocks_db_file_storage>) { + x.erase_instances(ids); + } else { + throw std::runtime_error("Storage not initialized"); + } + }, storage_); +} + void file::process_deletion_(const express::base& entity) { auto references = instances_by_reference(entity.id()); @@ -3127,7 +3145,7 @@ std::vector file::instances_by_reference(int t) { #ifdef IFOPSH_WITH_ROCKSDB else if constexpr (std::is_same_v, impl::rocks_db_file_storage>) { // @todo no lower/upper_bounds() implemented yet - auto prefix = "v|" + std::to_string(t) + "|"; + auto prefix = rocksdb_key::inverse_prefix(t); auto it = std::unique_ptr(x.db->NewIterator(rocksdb::ReadOptions())); it->Seek(prefix); while (it->Valid() && it->key().starts_with(prefix)) { @@ -3285,7 +3303,7 @@ std::vector file::get_inverse_indices_by_id(int instance_id) { } else if constexpr (std::is_same_v, impl::rocks_db_file_storage>) { #ifdef IFOPSH_WITH_ROCKSDB // @todo no lower/upper_bounds() implemented yet - auto prefix = "v|" + std::to_string(instance_id) + "|"; + auto prefix = rocksdb_key::inverse_prefix(instance_id); auto it = std::unique_ptr(x.db->NewIterator(rocksdb::ReadOptions())); it->Seek(prefix); while (it->Valid() && it->key().starts_with(prefix)) { @@ -3358,7 +3376,7 @@ std::vector file::get_inverse(int instance_id, const ifcopenshe visit_subtypes(type->as_entity(), [this, attribute_index, instance_id, &return_value, &x](const ifcopenshell::declaration* ent) { if (attribute_index == -1) { // @todo no lower/upper_bounds() implemented yet - auto prefix = "v|" + std::to_string(instance_id) + "|" + std::to_string(ent->index_in_schema()) + "|"; + auto prefix = rocksdb_key::inverse_prefix(instance_id) + key_to_string(ent->index_in_schema()) + "|"; auto it = std::unique_ptr(x.db->NewIterator(rocksdb::ReadOptions())); it->Seek(prefix); while (it->Valid() && it->key().starts_with(prefix)) { @@ -3507,9 +3525,7 @@ void ifcopenshell::file::unbatch() { process_deletion_(instance_by_id(id)); } // keep in memory until all deletions are processed - for (auto& id : batch_deletion_ids_) { - byid_.erase(id); - } + erase_instances_(std::vector(batch_deletion_ids_.begin(), batch_deletion_ids_.end())); batch_mode_ = false; batch_deletion_ids_.clear(); } @@ -3613,7 +3629,7 @@ bool ifcopenshell::impl::rocks_db_file_storage::read_schema(const ifcopenshell:: #endif #ifdef IFOPSH_WITH_ROCKSDB std::string value; - auto key = "h|file_schema|0"; + const auto key = rocksdb_key::header_attribute("file_schema", 0); db->Get(rocksdb::ReadOptions{}, key, &value); std::vector strings; if (::impl::deserialize(this, value, strings) && strings.size() == 1) { diff --git a/src/ifcparse/rocksdb_map_adapter.h b/src/ifcparse/rocksdb_map_adapter.h index 189f092c94..fd83db23dd 100644 --- a/src/ifcparse/rocksdb_map_adapter.h +++ b/src/ifcparse/rocksdb_map_adapter.h @@ -105,12 +105,64 @@ struct DefaultCodec { } }; +// Numeric key segments are fixed-width lowercase hex, so keys sort as +// numbers: an instance's records are laid out in id order and the largest +// id under a prefix is its last key. +inline std::string hex_key(uint64_t value) { + static constexpr char digits[] = "0123456789abcdef"; + std::string s(16, '0'); + for (int i = 15; i >= 0; --i) { + s[(size_t)i] = digits[value & 0xf]; + value >>= 4; + } + return s; +} + +inline uint64_t parse_hex_key(const std::string& s) { + return std::stoull(s, nullptr, 16); +} + template std::string key_to_string(const KeyT& key) { if constexpr (std::is_same_v) { return key; } else { - return std::to_string(key); + static_assert(std::is_integral_v, "key_to_string expects a string or an integral key"); + return hex_key((uint64_t)key); + } +} + +// The keys the file storage and the serializer agree on: +// i|| t|| h|| +// i||_ t||_ type record +// v||| inverse record +// t| instances of a type +namespace rocksdb_key { + // The exclusive end of everything under prefix. + inline std::string upper_bound(std::string prefix) { + prefix.back() = (char)(prefix.back() + 1); + return prefix; + } + inline std::string instance(bool is_entity, uint64_t id) { + return (is_entity ? "i|" : "t|") + key_to_string(id) + "|"; + } + inline std::string attribute(bool is_entity, uint64_t id, uint64_t index) { + return instance(is_entity, id) + key_to_string(index); + } + inline std::string header_attribute(const std::string& name, uint64_t index) { + return "h|" + name + "|" + key_to_string(index); + } + inline std::string type_record(bool is_entity, uint64_t id) { + return instance(is_entity, id) + "_"; + } + inline std::string inverse_prefix(uint64_t referenced_id) { + return "v|" + key_to_string(referenced_id) + "|"; + } + inline std::string inverse(uint64_t referenced_id, uint64_t entity_index, uint64_t attribute_index) { + return inverse_prefix(referenced_id) + key_to_string(entity_index) + "|" + key_to_string(attribute_index); + } + inline std::string type_list(uint64_t declaration_index) { + return "t|" + key_to_string(declaration_index); } } @@ -121,7 +173,7 @@ KeyT key_from_string(const std::string& key_string) { if constexpr (std::is_same_v) { return key_string; } else if constexpr (std::is_integral_v) { - return static_cast(std::stoll(key_string)); + return static_cast(parse_hex_key(key_string)); } else { static_assert(sizeof(KeyT) == 0, "key_from_string not implemented for this type"); } @@ -132,7 +184,7 @@ std::string tuple_to_string_impl(const Tuple& tuple_value, std::index_sequence(indices); std::ostringstream oss; // Unpack the tuple; add a pipe before each element except the first. - ((oss << (Is == 0 ? "" : "|") << std::to_string(std::get(tuple_value))), ...); + ((oss << (Is == 0 ? "" : "|") << key_to_string(std::get(tuple_value))), ...); return oss.str(); } @@ -145,7 +197,7 @@ std::string key_to_string(const std::tuple& key) { template T convert_string(const std::string& token) { if constexpr (std::is_integral_v) { - return static_cast(std::stoll(token)); + return static_cast(parse_hex_key(token)); } else if constexpr (std::is_floating_point_v) { return static_cast(std::stod(token)); } else { diff --git a/src/ifcparse/rocksdb_set_view.h b/src/ifcparse/rocksdb_set_view.h index 13e0f8e66e..0dcab8cf74 100644 --- a/src/ifcparse/rocksdb_set_view.h +++ b/src/ifcparse/rocksdb_set_view.h @@ -23,6 +23,7 @@ #ifdef IFOPSH_WITH_ROCKSDB #include #include +#include #endif #include @@ -196,6 +197,26 @@ public: return iterator(); } + // Removes the element: every key under prefix + key + "|". The exclusive + // upper bound is the same prefix with its separator incremented. Returns + // 1 if the element existed, 0 otherwise. + size_t erase(const key_type& key) { +#ifdef IFOPSH_WITH_ROCKSDB + if (find(key) == end()) { + return 0; + } + const std::string lower_bound = prefix_ + key_to_string(key) + "|"; + const std::string upper_bound = prefix_ + key_to_string(key) + std::string(1, '|' + 1); + rocksdb::WriteBatch batch; + batch.DeleteRange(lower_bound, upper_bound); + db_->Write(rocksdb::WriteOptions{}, &batch); + return 1; +#else + static_cast(key); + return 0; +#endif + } + // Read-only find: returns an iterator to the element with the given key if it exists. iterator find(const key_type& key) const { #ifdef IFOPSH_WITH_ROCKSDB @@ -216,11 +237,6 @@ public: return end(); } - size_t erase(const key_type& key) { - static_cast(key); - // @todo - return 0; - } }; #endif diff --git a/src/ifcparse/set_to_map_transformer.h b/src/ifcparse/set_to_map_transformer.h index 3ec284cc34..cea70ebd13 100644 --- a/src/ifcparse/set_to_map_transformer.h +++ b/src/ifcparse/set_to_map_transformer.h @@ -107,8 +107,6 @@ public: } size_t erase(const key_type& key) { - static_cast(key); - // @todo - return 0; + return base_map_->erase(key); } }; diff --git a/src/ifcparse/storage.h b/src/ifcparse/storage.h index 2322bb449b..02edcac3c3 100644 --- a/src/ifcparse/storage.h +++ b/src/ifcparse/storage.h @@ -678,6 +678,13 @@ namespace ifcopenshell { typedef std::map entity_by_iden_cache; entity_by_iden_cache instance_cache_, type_instance_cache_; std::mutex instance_cache_mutex_; + // Opening a database doesn't visit every instance, so the file's + // id counter is recalculated on the first create(). + bool id_counter_recalculated_ = false; + + // Deletes every key of the given instances and drops their cached + // handles, in one write and under one lock. + void erase_instances(const std::vector& ids); // @todo all these size_ts should probably be uint32_t for consistency with in-mem storage diff --git a/src/serializers/rocks_db_serializer.cpp b/src/serializers/rocks_db_serializer.cpp index b593c41805..e125db9dfd 100644 --- a/src/serializers/rocks_db_serializer.cpp +++ b/src/serializers/rocks_db_serializer.cpp @@ -98,16 +98,6 @@ namespace { } } -namespace { - template - std::string to_string_fixed_width(const T& t, size_t w) { - // @todo currently inactive - std::ostringstream oss; - oss << /*std::setfill('0') << std::setw(w) <<*/ t; - return oss.str(); - } -} - void RocksDbSerializer::write_streaming_() { ifcopenshell::impl::rocks_db_file_storage storage(rocksdb_filename_, nullptr); @@ -170,15 +160,13 @@ void RocksDbSerializer::write_streaming_() { // @nb cast to int in order not be interpreted as a char when appending to string int index = p.first.index_; - auto key = (is_header ? "h|" : (decl->as_entity() ? "i|" : "t|")) + - (is_header ? decl->name() : std::to_string(p.first.name_)) + "|" + - std::to_string(index); + auto key = (is_header ? rocksdb_key::header_attribute(decl->name(), index) : rocksdb_key::attribute(decl->as_entity() != nullptr, p.first.name_, index)); if (storage.db->Get(storage.ropts, key, &tmp) == rocksdb::Status::OK() && tmp.size() == (sizeof(size_t) + 2) && tmp[0] == ifcopenshell::type_encoder::encode_type() && tmp[1] == 't') { size_t iden; memcpy(&iden, tmp.data() + 2, sizeof(size_t)); - key = "t|" + std::to_string(iden) + "|0"; + key = rocksdb_key::attribute(false, iden, 0); type_identities_wrote_as_refs.insert(iden); } @@ -215,7 +203,7 @@ void RocksDbSerializer::write_streaming_() { auto write_inverse = [&](const ifcopenshell::reference_or_simple_type& v) { if (auto* ref = std::get_if(&v)) { - auto key = "v|" + to_string_fixed_width(*ref, 10) + "|" + to_string_fixed_width(decl->index_in_schema(), 4) + "|" + to_string_fixed_width(index, 2); + auto key = rocksdb_key::inverse(*ref, decl->index_in_schema(), index); static std::string s; uint32_t vv = name; s.resize(sizeof(uint32_t)); @@ -247,7 +235,7 @@ void RocksDbSerializer::write_streaming_() { storage.db->Put( storage.wopts, - (inst.declaration().as_entity() ? "i|" : "t|") + std::to_string(inst.identity()) + "|_", s); + rocksdb_key::type_record(inst.declaration().as_entity() != nullptr, inst.identity()), s); if (type_identities_wrote_as_refs.find(inst.identity()) != type_identities_wrote_as_refs.end()) { // already written as reference, skip @@ -275,13 +263,13 @@ void RocksDbSerializer::write_streaming_() { memcpy(s.data(), &v, sizeof(size_t)); storage.db->Put( storage.wopts, - (decl->as_entity() ? "i|" : "t|") + std::to_string(name) + "|_", s); + rocksdb_key::type_record(decl->as_entity() != nullptr, name), s); { size_t v = name; std::string s(sizeof(size_t), ' '); memcpy(s.data(), &v, sizeof(size_t)); - storage.db->Merge(storage.wopts, "t|" + std::to_string(decl->index_in_schema()), s); + storage.db->Merge(storage.wopts, rocksdb_key::type_list(decl->index_in_schema()), s); } // GlobalId as numeric ref to instance name, so that the guid map in