Do use merges, much better perf, instantiate inverses directly and generally, no longer copy map

This commit is contained in:
Thomas Krijnen
2025-03-14 16:16:25 +01:00
parent 8a3c979172
commit 8af4afa781
5 changed files with 105 additions and 45 deletions
+1 -1
View File
@@ -378,7 +378,7 @@ void rocks_db_attribute_storage::set(void* storage, const IfcParse::declaration*
std::string v;
impl::serialize(v, value);
rdb_storage->db->Put(
rocksdb::WriteOptions{},
rdb_storage->wopts,
(is_header ? "h|" : (decl->as_entity() ? "i|" : "t|")) +
(is_header ? decl->name() : std::to_string(identity)) + "|" +
std::to_string(index), v);
+11 -4
View File
@@ -383,8 +383,9 @@ namespace {
rocksdb::DB* init_db(const std::string& filepath) {
rocksdb::DB* db;
rocksdb::Options options;
// options.disable_auto_compactions = true;
options.create_if_missing = true;
// options.merge_operator.reset(new ConcatenateIdMergeOperator());
options.merge_operator.reset(new ConcatenateIdMergeOperator());
rocksdb::Status status = rocksdb::DB::Open(options, filepath, &db);
if (!status.ok()) {
throw std::runtime_error(status.ToString());
@@ -405,7 +406,9 @@ IfcParse::impl::rocks_db_file_storage::rocks_db_file_storage(const std::string&
, byref_excl_(db, "v|")
// @todo by_identity is probably not correct here, this mapping is Name -> Identity, so Fn should have access to full pair?
// , byidentity_(&byid_, [this](size_t v) { return assert_existance(v, by_identity); }, [](IfcUtil::IfcBaseClass* v) { return v->identity(); })
{}
{
// wopts.disableWAL = true;
}
IfcParse::impl::rocks_db_file_storage::~rocks_db_file_storage()
{
@@ -413,6 +416,10 @@ IfcParse::impl::rocks_db_file_storage::~rocks_db_file_storage()
flush_options.allow_write_stall = true;
flush_options.wait = true; // Wait until flush completes.
rocksdb::Status s = db->Flush(flush_options);
// compact entire db
db->CompactRange(rocksdb::CompactRangeOptions{}, nullptr, nullptr);
assert(s.ok());
db->Close();
@@ -445,7 +452,7 @@ void IfcParse::impl::rocks_db_file_storage::process_deletion_inverse(IfcUtil::If
rocksdb::WriteBatch batch;
batch.DeleteRange(prefix, it->key());
db->Write(rocksdb::WriteOptions{}, &batch);
db->Write(wopts, &batch);
}
// This is based on traversal which needs instances to still be contained in the map.
@@ -475,7 +482,7 @@ void IfcParse::impl::rocks_db_file_storage::process_deletion_inverse(IfcUtil::If
vals.erase(std::find(vals.begin(), vals.end(), (size_t)id));
s.resize(vals.size() * sizeof(size_t));
memcpy(s.data(), vals.data(), s.size());
db->Put(rocksdb::WriteOptions{}, it->key(), s);
db->Put(wopts, it->key(), s);
it->Next();
}
+21 -3
View File
@@ -39,18 +39,34 @@
#include <iterator>
#include <map>
// #include "rocksdb/merge_operator.h"
/*
#include "rocksdb/merge_operator.h"
namespace {
// @todo move to a proper place
class ConcatenateIdMergeOperator : public rocksdb::AssociativeMergeOperator {
public:
virtual bool FullMergeV2(const MergeOperator::MergeOperationInput& merge_in,
MergeOperator::MergeOperationOutput* merge_out) const {
// Log(InfoLogLevel::INFO_LEVEL, merge_in.logger, "FullMergeV2 new_value size:%ld", merge_out->new_value.size());
if (merge_in.existing_value) {
merge_out->new_value.append(merge_in.existing_value->data(), merge_in.existing_value->size());
}
for (auto& operand : merge_in.operand_list) {
merge_out->new_value.append(operand.data(), operand.size());
}
return true;
}
virtual bool Merge(const rocksdb::Slice&,
const rocksdb::Slice* existing_value,
const rocksdb::Slice& value,
std::string* new_value,
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());
@@ -58,6 +74,7 @@ namespace {
new_value->assign(value.data(), value.size());
}
return true;
*/
}
virtual const char* Name() const override {
@@ -65,7 +82,6 @@ namespace {
}
};
}
*/
namespace IfcParse {
@@ -331,6 +347,8 @@ namespace impl {
class rocks_db_file_storage {
public:
rocksdb::DB* db;
rocksdb::WriteOptions wopts;
rocksdb::ReadOptions ropts;
IfcParse::IfcFile* file;
enum instance_ref {
+61 -37
View File
@@ -789,24 +789,22 @@ namespace {
void IfcParse::impl::rocks_db_file_storage::register_inverse(unsigned id_from, const IfcParse::entity* from_entity, int inst_id, int attribute_index) {
static std::string s;
size_t v = id_from;
s.resize(sizeof(size_t));
memcpy(s.data(), &v, sizeof(size_t));
uint32_t v = id_from;
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);
db->Merge(wopts, key, s);
/*
// no merges yet, because python client doesn't support them
db->Merge(
rocksdb::WriteOptions{},
,
s);
*/
// Python client does not support merges
// @todo turn this into a setting
{
std::string current;
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);
db->Get(rocksdb::ReadOptions{}, key, &current);
auto new_val = current + s;
db->Put(rocksdb::WriteOptions{}, key, new_val);
}
db->Put(wopts, key, new_val);
}*/
}
void IfcParse::impl::rocks_db_file_storage::unregister_inverse(unsigned id_from, const IfcParse::entity* from_entity, IfcUtil::IfcBaseClass* inst, int attribute_index) {
@@ -814,12 +812,12 @@ void IfcParse::impl::rocks_db_file_storage::unregister_inverse(unsigned id_from,
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);
if (db->Get(rocksdb::ReadOptions{}, key, &s).ok()) {
std::vector<size_t> vals(s.size() / sizeof(size_t));
std::vector<uint32_t> vals(s.size() / sizeof(uint32_t));
memcpy(vals.data(), s.data(), s.size());
vals.erase(std::find(vals.begin(), vals.end(), (size_t)id_from));
s.resize(vals.size() * sizeof(size_t));
vals.erase(std::find(vals.begin(), vals.end(), (uint32_t)id_from));
s.resize(vals.size() * sizeof(uint32_t));
memcpy(s.data(), vals.data(), s.size());
db->Put(rocksdb::WriteOptions{}, key, s);
db->Put(wopts, key, s);
}
}
@@ -833,21 +831,22 @@ void IfcParse::impl::rocks_db_file_storage::add_type_ref(IfcUtil::IfcBaseClass*
memcpy(s.data(), &v, sizeof(size_t));
// no merges yet, because the python client doesn't support them
// db->Merge(rocksdb::WriteOptions{}, "t|" + std::to_string(new_entity->declaration().index_in_schema()), s);
{
db->Merge(rocksdb::WriteOptions{}, "t|" + std::to_string(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());
db->Get(rocksdb::ReadOptions{}, key, &current);
auto new_val = current + s;
db->Put(rocksdb::WriteOptions{}, key, new_val);
}
db->Put(wopts, key, new_val);
}*/
}
// not only mapping also register type
v = new_entity->declaration().index_in_schema();
memcpy(s.data(), &v, sizeof(size_t));
db->Put(rocksdb::WriteOptions{}, (new_entity->declaration().as_entity() ? "i|" : "t|") + std::to_string(new_entity->id() ? new_entity->id() : new_entity->identity()) + "|_", s);
db->Put(wopts, (new_entity->declaration().as_entity() ? "i|" : "t|") + std::to_string(new_entity->id() ? new_entity->id() : new_entity->identity()) + "|_", s);
}
void IfcParse::impl::rocks_db_file_storage::remove_type_ref(IfcUtil::IfcBaseClass* new_entity)
@@ -861,11 +860,11 @@ void IfcParse::impl::rocks_db_file_storage::remove_type_ref(IfcUtil::IfcBaseClas
vals.erase(std::find(vals.begin(), vals.end(), (size_t)new_entity->id()));
s.resize(vals.size() * sizeof(size_t));
memcpy(s.data(), vals.data(), s.size());
db->Put(rocksdb::WriteOptions{}, key, s);
db->Put(wopts, key, s);
}
}
db->Delete(rocksdb::WriteOptions{}, (new_entity->declaration().as_entity() ? "i|" : "t|") + std::to_string(new_entity->id() ? new_entity->id() : new_entity->identity()) + "|_");
db->Delete(wopts, (new_entity->declaration().as_entity() ? "i|" : "t|") + std::to_string(new_entity->id() ? new_entity->id() : new_entity->identity()) + "|_");
}
namespace {
@@ -1175,24 +1174,32 @@ class apply_individual_instance_visitor {
template <typename T>
void apply_attribute_(T& t, const AttributeValue& attr, int index) const {
if (attr.type() == IfcUtil::Argument_ENTITY_INSTANCE) {
switch (attr.type()) {
case IfcUtil::Argument_ENTITY_INSTANCE: {
IfcUtil::IfcBaseClass* inst = attr;
t(inst, index);
} else if (attr.type() == IfcUtil::Argument_AGGREGATE_OF_ENTITY_INSTANCE) {
break;
}
case IfcUtil::Argument_AGGREGATE_OF_ENTITY_INSTANCE: {
aggregate_of_instance::ptr entity_list_attribute = attr;
for (aggregate_of_instance::it it = entity_list_attribute->begin(); it != entity_list_attribute->end(); ++it) {
t(*it, index);
}
} else if (attr.type() == IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_ENTITY_INSTANCE) {
break;
}
case IfcUtil::Argument_AGGREGATE_OF_AGGREGATE_OF_ENTITY_INSTANCE: {
aggregate_of_aggregate_of_instance::ptr entity_list_attribute = attr;
for (aggregate_of_aggregate_of_instance::outer_it it = entity_list_attribute->begin(); it != entity_list_attribute->end(); ++it) {
for (aggregate_of_aggregate_of_instance::inner_it jt = it->begin(); jt != it->end(); ++jt) {
t(*jt, index);
}
}
break;
}
};
default:
break;
}
}
public:
apply_individual_instance_visitor(const AttributeValue& attribute, int idx)
: attribute_(attribute)
@@ -1238,9 +1245,11 @@ void IfcUtil::IfcBaseClass::set_attribute_value(size_t i, const T& t) {
}
}
// Deregister inverse indices in file
unregister_inverse_visitor visitor(*file_, this);
apply_individual_instance_visitor(current_attribute, (int) i).apply(visitor);
if constexpr (std::is_same_v<T, IfcUtil::IfcBaseClass*> || std::is_same_v<T, aggregate_of_instance::ptr> || std::is_same_v<T, aggregate_of_aggregate_of_instance::ptr> || std::is_same_v<T, Blank>) {
// Deregister inverse indices in file
unregister_inverse_visitor visitor(*file_, this);
apply_individual_instance_visitor(current_attribute, (int)i).apply(visitor);
}
}
{
void* const storage = file_ ? std::visit([](const auto& m) { return (void*)&m; }, file_->storage_) : nullptr;
@@ -1258,9 +1267,10 @@ void IfcUtil::IfcBaseClass::set_attribute_value(size_t i, const T& t) {
if (file_ != nullptr) {
// Register inverse indices in file
// @todo verify no longer necessary?
// register_inverse_visitor visitor(*file_, this);
// apply_individual_instance_visitor(new_attribute, (int) i).apply(visitor);
if constexpr (std::is_same_v<T, IfcUtil::IfcBaseClass*> || std::is_same_v<T, aggregate_of_instance::ptr> || std::is_same_v<T, aggregate_of_aggregate_of_instance::ptr>) {
register_inverse_visitor visitor(*file_, this);
apply_individual_instance_visitor(new_attribute, (int)i).apply(visitor);
}
// Register new attribute guid in guid map
if (i == 0 && (file_->ifcroot_type() != nullptr) && this->declaration().is(*file_->ifcroot_type())) {
@@ -1820,7 +1830,8 @@ IfcUtil::IfcBaseClass* IfcFile::addEntity(IfcUtil::IfcBaseClass* entity, int id)
void* own_storage = std::visit([](const auto& m) { return (void*)&m; }, storage_);
void* other_storage = std::visit([](const auto& m) { return (void*)&m; }, other_file->storage_);
for (size_t i = 0; i < (entity->declaration().as_entity() ? entity->declaration().as_entity()->attribute_count() : 1); ++i) {
auto num_attributes = (entity->declaration().as_entity() ? entity->declaration().as_entity()->attribute_count() : 1);
for (size_t i = 0; i < num_attributes; ++i) {
entity->data().apply_visitor(other_storage, decl, entity->id() ? entity->id() : entity->identity(), [this, i, decl, new_entity, own_storage](const auto& v) {
using U = std::decay_t<decltype(v)>;
// only need to copy non-instance attribute values, others are assigned below after mapping
@@ -1970,18 +1981,20 @@ IfcUtil::IfcBaseClass* IfcFile::addEntity(IfcUtil::IfcBaseClass* entity, int id)
new_id = new_entity->id();
}
/*
if (byid_.find(new_id) != byid_.end()) {
// This should not happen
std::stringstream ss;
ss << "Overwriting entity with id " << new_id;
Logger::Message(Logger::LOG_WARNING, ss.str());
}
*/
// rocksdb instances are assumed to be create with file.create();
std::visit([new_entity](auto& m) {
if constexpr (std::is_same_v<std::decay_t<decltype(m)>, impl::in_memory_file_storage>) {
// @todo not freed yet
m.byid_.insert({ new_entity->identity(), new_entity });
m.byid_.insert({ new_entity->id(), new_entity });
}
}, storage_);
} else if (new_entity->file_ == nullptr) {
@@ -2246,7 +2259,18 @@ aggregate_of_instance::ptr IfcFile::instances_by_reference(int t) {
}
}
} else if constexpr (std::is_same_v<std::decay_t<decltype(x)>, impl::rocks_db_file_storage>) {
// @todo
// @todo no lower/upper_bounds() implemented yet
auto prefix = "v|" + std::to_string(t) + "|";
auto it = x.db->NewIterator(rocksdb::ReadOptions());
it->Seek(prefix);
while (it->Valid() && it->key().starts_with(prefix)) {
std::vector<uint32_t> vals(it->value().size() / sizeof(uint32_t));
memcpy(vals.data(), it->value().data(), it->value().size());
for (auto& v : vals) {
ret->push(instance_by_id(v));
}
it->Next();
}
} else {
throw std::runtime_error("Storage not initialized");
}
+11
View File
@@ -74,18 +74,29 @@ void RocksDbSerializer::finalize()
}
// Add them in topological order, so that add() never recurses into something not previously visited
size_t n = 0;
for (auto& i : deps_topo_order) {
if (((n++) % 1000) == 0) {
std::wcout << n * 100 / deps_topo_order.size() << "%";
}
output_file_->addEntity(file_->instance_by_id(i), i);
}
// Copy inverses
/*
// These are now back to being added in addEntity() / set_attribute_value()
std::visit([this](const auto& m) {
if constexpr (std::is_same_v<std::decay_t<decltype(m)>, IfcParse::impl::in_memory_file_storage>) {
for (auto& p : m.byref_excl_) {
// This is much slower than need be, because:
// - insert() first checks for existance [we know it does not] because insert() should not overwrite
// - insert() returns an pair<iterator, bool> [which is not used] which requires an expensive seek after the put.
// This is left as-is for now, because anyway we want to build a streaming converter
std::get<IfcParse::impl::rocks_db_file_storage>(output_file_->storage_).byref_excl_.insert(p);
}
}
}, file_->storage_);
*/
delete output_file_;
}