From fc5f8e5a649fe81293fbc4dfe85f9ba23f1c4bb3 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Fri, 19 May 2023 19:07:14 -0700 Subject: [PATCH 01/28] Use more C++ features - Use C++ style for structs, including constructors, destructors - Use C++ new/delete instead of malloc()/free() - Use C++ initializer list for constructors - Use templates to avoid some copy&paste code - Atom and functor static constants moved inside functions that use them - Add [[nodiscard]]s - Class PlSlice is encpasulates rocksdb::Slice instead of subclassing it - Remove some inappropriate uses of PlCheckFail() --- Makefile | 11 +- cpp/rocksdb4pl.cpp | 645 ++++++++++++++++++++++----------------------- 2 files changed, 324 insertions(+), 332 deletions(-) diff --git a/Makefile b/Makefile index 2cc920b..ce3fdbb 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,9 @@ #COFLAGS=-gdwarf-2 -g3 -# The following flags are the same as rocksdb uses, except that rocksdb also has -Werror -# and doesn't have the conversion flags in ADDED_CPPFLAGS -# TODO: Add -Werror -# TODO: Options that work with clang -ADDED_CPPFLAGS=-Wconversion -Warith-conversion -Wsign-conversion -Wfloat-conversion -Wno-unused-parameter -CPPFLAGS=-Wall -Wextra -Wsign-compare -Wshadow -Wunused-parameter -Woverloaded-virtual -Wnon-virtual-dtor -Wno-missing-field-initializers -Wno-invalid-offsetof $(ADDED_CPPFLAGS) -std=c++17 -O2 $(CFLAGS) $(COFLAGS) $(LDSOFLAGS) -Irocksdb/include + +# For development, specify the following: +# ADDED_CPP_FLAGS= -Wsign-compare -Wshadow -Wunused-parameter -Woverloaded-virtual -Wnon-virtual-dtor -Wno-invalid-offsetof -Wconversion -Warith-conversion -Wsign-conversion -Wfloat-conversion -Wno-unused-parameter -Wno-missing-field-initializers + +CPPFLAGS=-Wall $(ADDED_CPPFLAGS) -std=c++17 -O2 $(CFLAGS) $(COFLAGS) $(LDSOFLAGS) -Irocksdb/include LIBROCKSDB=rocksdb/librocksdb.a ROCKSENV=ROCKSDB_DISABLE_JEMALLOC=1 ROCKSDB_DISABLE_TCMALLOC=1 # DEBUG_LEVEL=0 implies -O2 without assertions and debug code diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index c51cdc7..4ac369d 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -45,8 +45,7 @@ #include #include -#include // TODO: this should possibly be separate - +#include // TODO: this should possibly be separate /******************************* * SYMBOL * @@ -72,8 +71,20 @@ typedef enum MERGE_SET } merger_t; -typedef struct dbref -{ rocksdb::DB *db; /* DB handle */ +struct dbref +{ + dbref() + : db( nullptr), + symbol( PlAtom(PlAtom::null)), + name( PlAtom(PlAtom::null)), + flags( 0), + builtin_merger( MERGE_NONE), + merger( PlRecord(PlRecord::null)), + type( { .key = BLOB_ATOM, + .value = BLOB_ATOM}) + { } + + rocksdb::DB *db; /* DB handle */ PlAtom symbol; /* associated symbol */ PlAtom name; /* alias name */ int flags; /* flags */ @@ -83,31 +94,21 @@ typedef struct dbref { blob_type key; blob_type value; } type; -} dbref; - -static dbref null_dbref = -{ nullptr, // rocksdb::DB *db; - PlAtom(PlAtom::null), // PlAtom symbol; - PlAtom(PlAtom::null), // PlAtom name; - 0, // int flags; - MERGE_NONE, // merger_t builtin_merger; - PlRecord(PlRecord::null), // PlRexordRaw merger; - { BLOB_ATOM, // blob_type key; - BLOB_ATOM // blob_type value; - } }; - /******************************* * ALIAS * *******************************/ -typedef struct alias_cell -{ PlAtom name; - PlAtom symbol; - struct alias_cell *next; -} alias_cell; +struct alias_cell +{ + alias_cell(PlAtom _name, PlAtom _symbol, alias_cell *_next) + : name(_name), symbol(_symbol), next(_next) { } + PlAtom name; + PlAtom symbol; + alias_cell *next; +}; #define ALIAS_HASH_SIZE 64 @@ -115,16 +116,18 @@ std::mutex alias_lock; static unsigned int alias_size = ALIAS_HASH_SIZE; static alias_cell *alias_entries[ALIAS_HASH_SIZE]; +[[nodiscard]] static unsigned int atom_hash(PlAtom a) { return static_cast(a.C_>>7) % alias_size; } +[[nodiscard]] static PlAtom rocks_get_alias(PlAtom name) -{ for(alias_cell *c = alias_entries[atom_hash(name)]; +{ for ( alias_cell *c = alias_entries[atom_hash(name)]; c; - c = c->next) + c = c->next ) { if ( c->name == name ) return c->symbol; } @@ -138,11 +141,7 @@ rocks_alias(PlAtom name, PlAtom symbol) alias_lock.lock(); if ( rocks_get_alias(name).is_null() ) - { alias_cell *c = static_cast(malloc(sizeof *c)); - - c->name = name; - c->symbol = symbol; - c->next = alias_entries[key]; + { auto c = new alias_cell(name, symbol, alias_entries[key]); alias_entries[key] = c; c->name.register_ref(); c->symbol.register_ref(); @@ -159,7 +158,7 @@ rocks_unalias(PlAtom name) alias_cell *c, *prev=nullptr; alias_lock.lock(); - for(c = alias_entries[key]; c; prev=c, c = c->next) + for ( c = alias_entries[key]; c; prev=c, c = c->next ) { if ( c->name == name ) { if ( prev ) prev->next = c->next; @@ -167,7 +166,7 @@ rocks_unalias(PlAtom name) alias_entries[key] = c->next; c->name.unregister_ref(); c->symbol.unregister_ref(); - free(c); + delete c; break; } @@ -180,6 +179,7 @@ rocks_unalias(PlAtom name) * SYMBOL REFERENCES * *******************************/ +[[nodiscard]] static bool write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) { auto refp = static_cast(eref.blob_data(nullptr, nullptr)); @@ -191,6 +191,7 @@ write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) return true; } +[[nodiscard]] static int // TODO: bool write_rocks_ref(IOSTREAM *s, atom_t eref, int flags) { return write_rocks_ref_(s, PlAtom(eref), flags); @@ -198,9 +199,10 @@ write_rocks_ref(IOSTREAM *s, atom_t eref, int flags) /* - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -GC an rocks from the atom garbage collector. +GC a rocks dbref blob from the atom garbage collector. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */ +[[nodiscard]] static bool release_rocks_ref_(PlAtom aref) { auto refp = static_cast(aref.blob_data(nullptr, nullptr)); @@ -215,16 +217,18 @@ release_rocks_ref_(PlAtom aref) } } ref->merger.erase(); - Plx_free(ref); + delete ref; return true; } +[[nodiscard]] static int // TODO: bool release_rocks_ref(atom_t aref) { return release_rocks_ref_(PlAtom(aref)); } +[[nodiscard]] static bool save_rocks_(PlAtom aref, IOSTREAM *fd) { auto refp = static_cast(aref.blob_data(nullptr, nullptr)); @@ -234,11 +238,13 @@ save_rocks_(PlAtom aref, IOSTREAM *fd) return PL_warning("Cannot save reference to (%p)", ref); } +[[nodiscard]] static int // TODO: bool save_rocks(atom_t aref, IOSTREAM *fd) { return save_rocks_(PlAtom(aref), fd); } +[[nodiscard]] static PlAtom load_rocks_(IOSTREAM *fd) { (void)fd; @@ -252,18 +258,19 @@ load_rocks(IOSTREAM *fd) } static PL_blob_t rocks_blob = -{ PL_BLOB_MAGIC, - PL_BLOB_UNIQUE, - (const char *)"rocksdb", - release_rocks_ref, - nullptr, - write_rocks_ref, - nullptr, - save_rocks, // TODO: implement - load_rocks // TODO: implement +{ .magic = PL_BLOB_MAGIC, + .flags = PL_BLOB_UNIQUE, + .name = "rocksdb", + .release = release_rocks_ref, + .compare = nullptr, + .write = write_rocks_ref, + .acquire = nullptr, + .save = save_rocks, // TODO: implement + .load = load_rocks // TODO: implement }; +[[nodiscard]] static bool unify_rocks(PlTerm t, dbref *ref) { if ( ref->name.not_null() ) @@ -281,7 +288,8 @@ unify_rocks(PlTerm t, dbref *ref) } -static dbref* +[[nodiscard]] +static dbref * symbol_dbref(PlAtom symbol) { void *data; size_t len; @@ -298,15 +306,15 @@ symbol_dbref(PlAtom symbol) [[nodiscard]] static dbref * -get_rocks(PlTerm t,bool warn=true) +get_rocks(PlTerm t, bool warn=true) { PlAtom a(PlAtom::null); if ( warn ) a = t.as_atom(); else t.get_atom_ex(&a); - if ( a.not_null() ) - { for(int i=0; i<2 && a.not_null(); i++) + if ( t.not_null() ) + { for ( int i=0; i<2 && a.not_null(); i++ ) { dbref *ref; if ( (ref=symbol_dbref(a)) ) @@ -331,17 +339,10 @@ get_rocks(PlTerm t,bool warn=true) * UTIL * *******************************/ -class RocksError : public PlException -{ -public: - RocksError(const rocksdb::Status &status) : - PlException(PlCompound("error", - PlTermv(PlCompound("rocks_error", - PlTermv(PlTerm_atom(status.ToString()))), - PlTerm_var()))) - { - } -}; +PlException RocksError(const rocksdb::Status &status) +{ return PlGeneralError(PlCompound("rocks_error", + PlTermv(PlTerm_atom(status.ToString())))); +} static bool @@ -353,7 +354,12 @@ ok(const rocksdb::Status &status) throw RocksError(status); } -class PlSlice : public rocksdb::Slice +static void +ok_or_throw_fail(const rocksdb::Status &status) +{ PlCheckFail(ok(status)); +} + +class PlSlice { public: explicit PlSlice() @@ -411,7 +417,7 @@ class PlSliceStr : public PlSlice [[nodiscard]] static std::unique_ptr get_slice(PlTerm t, blob_type type) -{ switch(type) +{ switch ( type ) { case BLOB_ATOM: case BLOB_STRING: return std::make_unique(t.get_nchars(CVT_IN|CVT_EXCEPTION|REP_UTF8)); @@ -441,46 +447,38 @@ get_slice(PlTerm t, blob_type type) } -static const PlAtom ATOM_(""); - - +[[nodiscard]] static bool unify(PlTerm t, const rocksdb::Slice &s, blob_type type) -{ switch(type) +{ switch ( type ) { case BLOB_ATOM: return t.unify_chars(PL_ATOM|REP_UTF8, s.size_, s.data_); case BLOB_STRING: - return t.unify_chars(PL_STRING|REP_UTF8, s.size_, s.data_); case BLOB_BINARY: return t.unify_chars(PL_STRING|REP_ISO_LATIN_1, s.size_, s.data_); case BLOB_INT32: { int i; - memcpy(&i, s.data_, sizeof i); // Unaligned i=*reinterpret_cast(s.data_) return t.unify_integer(i); } case BLOB_INT64: { int64_t i; - memcpy(&i, s.data_, sizeof i); return t.unify_integer(i); } case BLOB_FLOAT32: { float f; - memcpy(&f, s.data_, sizeof f); return t.unify_float(f); } case BLOB_FLOAT64: { double f; - memcpy(&f, s.data_, sizeof f); return t.unify_float(f); } case BLOB_TERM: { PlTerm_var tmp; - Plx_recorded_external(s.data_, tmp.C_); return tmp.unify_term(t); } @@ -490,95 +488,99 @@ unify(PlTerm t, const rocksdb::Slice &s, blob_type type) } } +[[nodiscard]] static bool unify(PlTerm t, const rocksdb::Slice *s, blob_type type) -{ if ( s == static_cast(nullptr) ) - { switch(type) - { case BLOB_ATOM: - return t.unify_atom(ATOM_); - case BLOB_STRING: - case BLOB_BINARY: - return t.unify_chars(PL_STRING, 0, ""); - case BLOB_INT32: - case BLOB_INT64: - return t.unify_integer(0); - case BLOB_FLOAT32: - case BLOB_FLOAT64: - return t.unify_float(0.0); - case BLOB_TERM: - return t.unify_nil(); - default: - assert(0); - return false; +{ if ( s ) + return unify(t, *s, type); + + switch ( type ) + { case BLOB_ATOM: + { static const PlAtom ATOM_(""); + return t.unify_atom(ATOM_); } + case BLOB_STRING: + case BLOB_BINARY: + return t.unify_chars(PL_STRING, 0, ""); + case BLOB_INT32: + case BLOB_INT64: + return t.unify_integer(0); + case BLOB_FLOAT32: + case BLOB_FLOAT64: + return t.unify_float(0.0); + case BLOB_TERM: + return t.unify_nil(); + default: + assert(0); + return false; } - - return unify(t, *s, type); } +[[nodiscard]] static bool unify(PlTerm t, const std::string &s, blob_type type) -{ rocksdb::Slice sl(s.data(), s.length()); +{ rocksdb::Slice sl(s); return unify(t, sl, type); } +[[nodiscard]] static bool unify_value(PlTerm t, const rocksdb::Slice &s, merger_t merge, blob_type type) { if ( merge == MERGE_NONE ) - { return unify(t, s, type); - } else - { PlTerm_tail list(t); - PlTerm_var tmp; - const char *data = s.data(); - const char *end = data+s.size(); - - while ( data < end ) - { switch ( type ) - { case BLOB_INT32: - { int i; - memcpy(&i, data, sizeof i); - data += sizeof i; - Plx_put_integer(tmp.C_, i); - } - break; - case BLOB_INT64: - { int64_t i; - memcpy(&i, data, sizeof i); - data += sizeof i; - Plx_put_int64(tmp.C_, i); - } - break; - case BLOB_FLOAT32: - { float i; - memcpy(&i, data, sizeof i); - data += sizeof i; - Plx_put_float(tmp.C_, i); - } - break; - case BLOB_FLOAT64: - { double i; - memcpy(&i, data, sizeof i); - data += sizeof i; - Plx_put_float(tmp.C_, i); - } - break; - default: - assert(0); - return false; - } + return unify(t, s, type); - if ( !list.append(tmp) ) + PlTerm_tail list(t); + PlTerm_var tmp; + const char *data = s.data(); + const char *end = data+s.size(); + + while ( data < end ) + { switch ( type ) + { case BLOB_INT32: + { int i; + memcpy(&i, data, sizeof i); + data += sizeof i; + Plx_put_integer(tmp.C_, i); + } + break; + case BLOB_INT64: + { int64_t i; + memcpy(&i, data, sizeof i); + data += sizeof i; + Plx_put_int64(tmp.C_, i); + } + break; + case BLOB_FLOAT32: + { float i; + memcpy(&i, data, sizeof i); + data += sizeof i; + Plx_put_float(tmp.C_, i); + } + break; + case BLOB_FLOAT64: + { double i; + memcpy(&i, data, sizeof i); + data += sizeof i; + Plx_put_float(tmp.C_, i); + } + break; + default: + assert(0); return false; } - return list.close(); + if ( !list.append(tmp) ) + return false; } + + return list.close(); } +[[nodiscard]] static bool unify_value(PlTerm t, const std::string &s, merger_t merge, blob_type type) -{ rocksdb::Slice sl(s.data(), s.length()); +{ rocksdb::Slice sl(s); return unify_value(t, sl, merge, type); } @@ -588,19 +590,19 @@ unify_value(PlTerm t, const std::string &s, merger_t merge, blob_type type) * MERGER * *******************************/ -static const PlAtom ATOM_partial("partial"); -static const PlAtom ATOM_full("full"); - +[[nodiscard]] static bool log_exception(rocksdb::Logger* logger) { PlTerm_term_t ex(Plx_exception(0)); Log(logger, "%s", ex.as_string(PlEncoding::UTF8).c_str()); - return false; + return false; // For convenience, allowing: return log_exception(logger); } class engine -{ int tid = 0; +{ +private: + int tid = 0; public: engine() @@ -616,18 +618,16 @@ class engine } ~engine() - { if ( tid ) + { if ( tid > 0 ) Plx_thread_destroy_engine(); } }; +[[nodiscard]] static bool call_merger(const dbref *ref, PlTermv av, std::string* new_value, rocksdb::Logger* logger) -{ static PlPredicate pred_call6(PlPredicate::null); - - if ( pred_call6.is_null() ) - pred_call6 = PlPredicate(PlFunctor("call", 6), PlModule("system")); +{ static PlPredicate pred_call6(PlFunctor("call", 6), PlModule("system")); try { PlQuery q(pred_call6, av); @@ -640,18 +640,20 @@ call_merger(const dbref *ref, PlTermv av, std::string* new_value, return false; } } catch(PlException &ex) - { Log(logger, "%s", ex.as_string(PlEncoding::UTF8).c_str()); + { Log(logger, "%s", ex.as_string(PlEncoding::UTF8).c_str()); return false; } } class PrologMergeOperator : public rocksdb::MergeOperator -{ const dbref *ref; +{ +private: + const dbref *ref; + public: - PrologMergeOperator(const dbref *reference) : MergeOperator() - { ref = reference; - } + explicit PrologMergeOperator(const dbref *reference) + : ref(reference) { } virtual bool FullMerge(const rocksdb::Slice& key, @@ -663,13 +665,16 @@ class PrologMergeOperator : public rocksdb::MergeOperator PlTermv av(6); PlTerm_tail list(av[4]); PlTerm_var tmp; + static const PlAtom ATOM_full("full"); for (const auto& value : operand_list) { Plx_put_variable(tmp.C_); - unify(tmp, value, ref->type.value); - PlCheckFail(list.append(tmp)); + if ( !unify(tmp, value, ref->type.value) || + !list.append(tmp) ) + return false; } - PlCheckFail(list.close()); + if ( !list.close() ) + return false; if ( av[0].unify_term(ref->merger.term()) && av[1].unify_atom(ATOM_full) && @@ -688,6 +693,7 @@ class PrologMergeOperator : public rocksdb::MergeOperator rocksdb::Logger* logger) const override { engine e; PlTermv av(6); + static const PlAtom ATOM_partial("partial"); if ( av[0].unify_term(ref->merger.term()) && av[1].unify_atom(ATOM_partial) && @@ -705,96 +711,68 @@ class PrologMergeOperator : public rocksdb::MergeOperator } }; +template +[[nodiscard]] static int -cmp_int32(const void *v1, const void *v2) -{ auto i1 = static_cast(v1); - auto i2 = static_cast(v2); +cmp_number(const void *v1, const void *v2) +{ auto i1 = static_cast(v1); + auto i2 = static_cast(v2); return *i1 > *i2 ? 1 : *i1 < *i2 ? -1 : 0; } +template +static void +sort_numbers(std::string *str) +{ auto s = const_cast(str->data()); + auto len = str->length(); + if ( len == 0 ) + return; + auto ip = reinterpret_cast(s); + auto op = ip+1; + auto ep = reinterpret_cast(s+len); + Number_t cv; + + qsort(s, len / sizeof ip, sizeof ip, cmp_number); + cv = *ip; + for ( ip++; ip < ep; ip++ ) + { if ( *ip != cv ) + *op++ = cv = *ip; + } + str->resize(static_cast(reinterpret_cast(op) - s)); +} + void sort(std::string *str, blob_type type) -{ auto s = const_cast(str->c_str()); - auto len = str->length(); - - if ( len > 0 ) - { switch ( type ) - { case BLOB_INT32: - { auto ip = reinterpret_cast(s); - auto op = ip+1; - auto ep = reinterpret_cast(s+len); - int cv; - - qsort(s, len/sizeof(int), sizeof(int), cmp_int32); - cv = *ip; - for(ip++; ip < ep; ip++) - { if ( *ip != cv ) - *op++ = cv = *ip; - } - str->resize(static_cast((reinterpret_cast(op) - s))); - break; - } - case BLOB_INT64: - { auto ip = reinterpret_cast(s); - auto op = ip+1; - auto ep = reinterpret_cast(s+len); - int64_t cv; - - qsort(s, len/sizeof(int64_t), sizeof(int64_t), cmp_int32); - cv = *ip; - for(ip++; ip < ep; ip++) - { if ( *ip != cv ) - *op++ = cv = *ip; - } - str->resize(static_cast(reinterpret_cast(op) - s)); - break; - } - case BLOB_FLOAT32: - { auto ip = reinterpret_cast(s); - auto op = ip+1; - auto ep = reinterpret_cast(s+len); - float cv; - - qsort(s, len/sizeof(float), sizeof(float), cmp_int32); - cv = *ip; - for(ip++; ip < ep; ip++) - { if ( *ip != cv ) - *op++ = cv = *ip; - } - str->resize(static_cast(reinterpret_cast(op) - s)); - break; - } - case BLOB_FLOAT64: - { auto ip = reinterpret_cast(s); - auto op = ip+1; - auto ep = reinterpret_cast(s+len); - double cv; - - qsort(s, len/sizeof(double), sizeof(double), cmp_int32); - cv = *ip; - for(ip++; ip < ep; ip++) - { if ( *ip != cv ) - *op++ = cv = *ip; - } - str->resize(static_cast(reinterpret_cast(op) - s)); - break; - } - default: - assert(0); - return; - } +{ switch ( type ) + { case BLOB_INT32: + sort_numbers(str); + break; + case BLOB_INT64: + sort_numbers(str); + break; + case BLOB_FLOAT32: + sort_numbers(str); + break; + case BLOB_FLOAT64: + sort_numbers(str); + break; + default: + assert(0); + break; } } class ListMergeOperator : public rocksdb::MergeOperator -{ const dbref *ref; +{ +private: + const dbref *ref; + public: - ListMergeOperator(const dbref *reference) : MergeOperator() - { ref = reference; - } + explicit ListMergeOperator(const dbref *reference) + : ref(reference) { } virtual bool FullMerge(const rocksdb::Slice& key, @@ -804,10 +782,7 @@ class ListMergeOperator : public rocksdb::MergeOperator rocksdb::Logger* logger) const override { (void)key; (void)logger; - std::string s; - - if ( existing_value ) - s += existing_value->ToString(); + std::string s(existing_value ? existing_value->ToString() : ""); for (const auto& value : operand_list) { s += value; @@ -827,7 +802,7 @@ class ListMergeOperator : public rocksdb::MergeOperator rocksdb::Logger* logger) const override { (void)key; (void)logger; - std::string s = left_operand.ToString(); + std::string s(left_operand.ToString()); s += right_operand.ToString(); if ( ref->builtin_merger == MERGE_SET ) @@ -847,67 +822,42 @@ class ListMergeOperator : public rocksdb::MergeOperator * PREDICATES * *******************************/ -static const PlAtom ATOM_key("key"); -static const PlAtom ATOM_value("value"); -static const PlAtom ATOM_alias("alias"); -static const PlAtom ATOM_merge("merge"); - -static const PlAtom ATOM_atom("atom"); -static const PlAtom ATOM_string("string"); -static const PlAtom ATOM_binary("binary"); -static const PlAtom ATOM_int32("int32"); -static const PlAtom ATOM_int64("int64"); -static const PlAtom ATOM_float("float"); -static const PlAtom ATOM_double("double"); -static const PlAtom ATOM_term("term"); -static const PlAtom ATOM_open("open"); -static const PlAtom ATOM_once("once"); -static const PlAtom ATOM_mode("mode"); -static const PlAtom ATOM_read_only("read_only"); -static const PlAtom ATOM_read_write("read_write"); - -static const PlAtom ATOM_debug("debug"); -static const PlAtom ATOM_info("info"); -static const PlAtom ATOM_warn("warn"); -static const PlAtom ATOM_error("error"); -static const PlAtom ATOM_fatal("fatal"); -static const PlAtom ATOM_header("header"); - -static const PlFunctor FUNCTOR_list1("list", 1); -static const PlFunctor FUNCTOR_set1("set", 1); - static void get_blob_type(PlTerm t, blob_type *key_type, merger_t *m) { PlAtom a(PlAtom::null); - bool rc; + + static const PlFunctor FUNCTOR_list1("list", 1); + static const PlFunctor FUNCTOR_set1("set", 1); + static const PlAtom ATOM_atom("atom"); + static const PlAtom ATOM_string("string"); + static const PlAtom ATOM_binary("binary"); + static const PlAtom ATOM_term("term"); + static const PlAtom ATOM_int32("int32"); + static const PlAtom ATOM_int64("int64"); + static const PlAtom ATOM_float("float"); + static const PlAtom ATOM_double("double"); if ( m && t.is_functor(FUNCTOR_list1) ) { *m = MERGE_LIST; - rc = t[1].get_atom(&a); + t[1].get_atom_ex(&a); } else if ( m && t.is_functor(FUNCTOR_set1) ) { *m = MERGE_SET; - rc = t[1].get_atom(&a); + t[1].get_atom_ex(&a); } else - rc = t.get_atom(&a); - - if ( rc ) - { if ( !m || *m == MERGE_NONE ) - { if ( ATOM_atom == a ) { *key_type = BLOB_ATOM; return; } - else if ( ATOM_string == a ) { *key_type = BLOB_STRING; return; } - else if ( ATOM_binary == a ) { *key_type = BLOB_BINARY; return; } - else if ( ATOM_term == a ) { *key_type = BLOB_TERM; return; } - } - - if ( ATOM_int32 == a ) { *key_type = BLOB_INT32; return; } - else if ( ATOM_int64 == a ) { *key_type = BLOB_INT64; return; } - else if ( ATOM_float == a ) { *key_type = BLOB_FLOAT32; return; } - else if ( ATOM_double == a ) { *key_type = BLOB_FLOAT64; return; } - else throw PlDomainError("rocks_type", t); + t.get_atom_ex(&a); - return; + if ( !m || *m == MERGE_NONE ) + { if ( ATOM_atom == a ) { *key_type = BLOB_ATOM; return; } + else if ( ATOM_string == a ) { *key_type = BLOB_STRING; return; } + else if ( ATOM_binary == a ) { *key_type = BLOB_BINARY; return; } + else if ( ATOM_term == a ) { *key_type = BLOB_TERM; return; } } - throw PlTypeError("atom", t); + if ( ATOM_int32 == a ) { *key_type = BLOB_INT32; return; } + else if ( ATOM_int64 == a ) { *key_type = BLOB_INT64; return; } + else if ( ATOM_float == a ) { *key_type = BLOB_FLOAT32; return; } + else if ( ATOM_double == a ) { *key_type = BLOB_FLOAT64; return; } + else throw PlDomainError("rocks_type", t); } @@ -921,6 +871,9 @@ struct ReadOptdef #define RD_ODEF [](rocksdb::ReadOptions *options, PlTerm arg) +// TODO: move read_optdefs into read_options(), remove the PlAtom(PlAtom::null), +// replacing ReadOptdef::name with PlAtom ... C++ will initialize the +// "static" const structure on first use. static ReadOptdef read_optdefs[] = { // "snapshot" const Snapshot* // "iterate_lower_bound" const rocksdb::Slice* @@ -974,8 +927,8 @@ static void lookup_read_optdef_and_apply(rocksdb::ReadOptions *options, ReadOptdef opt_defs[], PlAtom name, PlTerm opt) -{ for(auto def=opt_defs; def->name; def++) - { if ( def->atom.is_null() ) // lazilly fill in atoms in lookup table +{ for ( auto def=opt_defs; def->name; def++ ) + { if ( def->atom.is_null() ) // lazily fill in atoms in lookup table def->atom = PlAtom(def->name); if ( def->atom == name ) { def->action(options, opt[1]); @@ -996,6 +949,9 @@ struct WriteOptdef #define WR_ODEF [](rocksdb::WriteOptions *options, PlTerm arg) +// TODO: move write_optdefs into write_options(), remove the PlAtom(PlAtom::null), +// replacing WriteOptdef::name with PlAtom ... C++ will initialize the +// "static" const structure on first use. static WriteOptdef write_optdefs[] = { { "sync", WR_ODEF { options->sync = arg.as_bool(); }, PlAtom(PlAtom::null) }, @@ -1019,7 +975,7 @@ static void lookup_write_optdef_and_apply(rocksdb::WriteOptions *options, WriteOptdef opt_defs[], PlAtom name, PlTerm opt) -{ for(auto def=opt_defs; def->name; def++) +{ for ( auto def=opt_defs; def->name; def++ ) { if ( def->atom.is_null() ) // lazilly fill in atoms in lookup table def->atom = PlAtom(def->name); if ( def->atom == name ) @@ -1033,6 +989,14 @@ lookup_write_optdef_and_apply(rocksdb::WriteOptions *options, static void options_set_InfoLogLevel(rocksdb::Options *options, PlTerm arg) { rocksdb::InfoLogLevel log_level; + + static const PlAtom ATOM_debug("debug"); + static const PlAtom ATOM_info("info"); + static const PlAtom ATOM_warn("warn"); + static const PlAtom ATOM_error("error"); + static const PlAtom ATOM_fatal("fatal"); + static const PlAtom ATOM_header("header"); + const auto arg_a = arg.as_atom(); if ( arg_a == ATOM_debug ) log_level = rocksdb::DEBUG_LEVEL; else if ( arg_a == ATOM_info ) log_level = rocksdb::INFO_LEVEL; @@ -1231,12 +1195,15 @@ static Optdef optdefs[] = { nullptr, nullptr, PlAtom(PlAtom::null) } }; +// TODO: move optdefs into options(), remove the PlAtom(PlAtom::null), +// replacing Optdef::name with PlAtom ... C++ will initialize the +// "static" const structure on first use. static void lookup_optdef_and_apply(rocksdb::Options *options, Optdef opt_defs[], PlAtom name, PlTerm opt) -{ for(auto def=opt_defs; def->name; def++) +{ for ( auto def=opt_defs; def->name; def++ ) { if ( def->atom.is_null() ) // lazilly fill in atoms in lookup table def->atom = PlAtom(def->name); if ( def->atom == name ) @@ -1260,8 +1227,17 @@ PREDICATE(rocks_open_, 3) int once = false; int read_only = false; - if ( !Plx_get_file_name(A1.C_, &fn, PL_FILE_OSPATH) ) - return false; + static const PlAtom ATOM_key("key"); + static const PlAtom ATOM_value("value"); + static const PlAtom ATOM_alias("alias"); + static const PlAtom ATOM_merge("merge"); + static const PlAtom ATOM_open("open"); + static const PlAtom ATOM_once("once"); + static const PlAtom ATOM_mode("mode"); + static const PlAtom ATOM_read_write("read_write"); + static const PlAtom ATOM_read_only("read_only"); + + PlCheckFail(Plx_get_file_name(A1.C_, &fn, PL_FILE_OSPATH)); PlTerm_tail tail(A3); PlTerm_var opt; while ( tail.next(opt) ) @@ -1309,8 +1285,7 @@ PREDICATE(rocks_open_, 3) } } - dbref *ref = static_cast(Plx_malloc(sizeof *ref)); - *ref = null_dbref; + auto ref = new dbref(); ref->merger = merger; ref->builtin_merger = builtin_merger; ref->type.key = key_type; @@ -1320,22 +1295,20 @@ PREDICATE(rocks_open_, 3) ref->flags |= DB_OPEN_ONCE; try - { rocksdb::Status status; - - if ( ref->merger.not_null() ) + { if ( ref->merger.not_null() ) options.merge_operator.reset(new PrologMergeOperator(ref)); else if ( builtin_merger != MERGE_NONE ) options.merge_operator.reset(new ListMergeOperator(ref)); if ( read_only ) - status = rocksdb::DB::OpenForReadOnly(options, fn, &ref->db); + ok_or_throw_fail(rocksdb::DB::OpenForReadOnly(options, fn, &ref->db)); else - status = rocksdb::DB::Open(options, fn, &ref->db); - ok(status); - return unify_rocks(A2, ref); + ok_or_throw_fail(rocksdb::DB::Open(options, fn, &ref->db)); + PlCheckFail(unify_rocks(A2, ref)); } catch(...) - { Plx_free(ref); + { delete ref; throw; } + return true; } @@ -1355,6 +1328,7 @@ PREDICATE(rocks_close, 1) } +[[nodiscard]] static rocksdb::WriteOptions write_options(PlTerm options_term) { rocksdb::WriteOptions options; @@ -1366,8 +1340,8 @@ write_options(PlTerm options_term) PlCheckFail(opt.name_arity(&name, &arity)); if ( arity == 1 ) - { lookup_write_optdef_and_apply(&options, write_optdefs, name, opt); - } else + lookup_write_optdef_and_apply(&options, write_optdefs, name, opt); + else throw PlTypeError("option", opt); } return options; @@ -1380,7 +1354,7 @@ PREDICATE(rocks_put, 4) if ( ref->builtin_merger == MERGE_NONE ) { auto value = get_slice(A3, ref->type.value); - ok(ref->db->Put(write_options(A4), key->slice(), value->slice())); + ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value->slice())); } else { PlTerm_tail list(A3); PlTerm_var tmp; @@ -1394,7 +1368,7 @@ PREDICATE(rocks_put, 4) if ( ref->builtin_merger == MERGE_SET ) sort(&value, ref->type.value); - ok(ref->db->Put(write_options(A4), key->slice(), value)); + ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value)); } return true; @@ -1405,14 +1379,15 @@ PREDICATE(rocks_merge, 4) if ( ref->merger.is_null() && ref->builtin_merger == MERGE_NONE ) throw PlPermissionError("merge", "rocksdb", A1); - auto key = get_slice(A2, ref->type.key); + auto key = get_slice(A2,ref->type.key); auto value = get_slice(A3, ref->type.value); - ok(ref->db->Merge(write_options(A4), key->slice(), value->slice())); + ok_or_throw_fail(ref->db->Merge(write_options(A4), key->slice(), value->slice())); return true; } +[[nodiscard]] static rocksdb::ReadOptions read_options(PlTerm options_term) { rocksdb::ReadOptions options; @@ -1423,8 +1398,8 @@ read_options(PlTerm options_term) size_t arity; PlCheckFail(opt.name_arity(&name, &arity)); if ( arity == 1 ) - { lookup_read_optdef_and_apply(&options, read_optdefs, name, opt); - } else + lookup_read_optdef_and_apply(&options, read_optdefs, name, opt); + else throw PlTypeError("option", opt); } return options; @@ -1435,41 +1410,53 @@ PREDICATE(rocks_get, 4) std::string value; auto key = get_slice(A2, ref->type.key); - return ( ok(ref->db->Get(read_options(A4), key->slice(), &value)) && - unify_value(A3, value, ref->builtin_merger, ref->type.value) ); + ok_or_throw_fail(ref->db->Get(read_options(A4), key->slice(), &value)); + return unify_value(A3, value, ref->builtin_merger, ref->type.value); } PREDICATE(rocks_delete, 3) { dbref *ref = get_rocks(A1); auto key = get_slice(A2, ref->type.key); - return ok(ref->db->Delete(write_options(A3), key->slice())); + ok_or_throw_fail(ref->db->Delete(write_options(A3), key->slice())); + return true; } typedef enum -{ ENUM_ALL, - ENUM_FROM, +{ ENUM_NOT_INITIALIZED, + ENUM_ALL, // TODO: not used? + ENUM_FROM, // TODO: not used? ENUM_PREFIX } enum_type; -typedef struct -{ rocksdb::Iterator *it; +struct enum_state +{ + rocksdb::Iterator *it; // TODO: use smart ptr to auto delete dbref *ref; enum_type type; struct { size_t length; char *string; - } prefix; - int saved; -} enum_state; + } prefix; // TODO: use std::string + bool saved; +}; + +static enum_state null_enum_state = +{ .it = nullptr, + .ref = nullptr, + .type = ENUM_NOT_INITIALIZED, + .prefix = { .length=0, .string=nullptr }, + .saved = false +}; +[[nodiscard]] static enum_state * save_enum_state(enum_state *state) { if ( !state->saved ) { auto copy = static_cast(malloc(sizeof (enum_state))); *copy = *state; if ( copy->prefix.string ) - { copy->prefix.string = static_cast(malloc(copy->prefix.length+1)); + { copy->prefix.string = new char[copy->prefix.length+1]; memcpy(copy->prefix.string, state->prefix.string, copy->prefix.length+1); } copy->saved = true; @@ -1479,17 +1466,18 @@ save_enum_state(enum_state *state) return state; } -static void +static void // TODO: change this to ~enum_state() free_enum_state(enum_state *state) { if ( state->saved ) { if ( state->it ) delete state->it; if ( state->prefix.string ) - free(state->prefix.string); + delete [] state->prefix.string; free(state); } } +[[nodiscard]] static bool unify_enum_key(PlTerm t, const enum_state *state) { if ( state->type == ENUM_PREFIX ) @@ -1509,6 +1497,7 @@ unify_enum_key(PlTerm t, const enum_state *state) } +[[nodiscard]] static bool enum_key_prefix(const enum_state *state) { if ( state->type == ENUM_PREFIX ) @@ -1520,11 +1509,14 @@ enum_key_prefix(const enum_state *state) } +[[nodiscard]] static foreign_t rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::ReadOptions options) -{ enum_state state_buf = {0}; +{ enum_state state_buf = null_enum_state; enum_state *state = &state_buf; + // TODO: PlForeignContextPtr ctxt(handle) + switch ( handle.foreign_control() ) { case PL_FIRST_CALL: state->ref = get_rocks(A1); @@ -1553,7 +1545,7 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea state = static_cast(handle.foreign_context_address()); next: { PlFrame fr; - for(; state->it->Valid(); state->it->Next()) + for ( ; state->it->Valid(); state->it->Next() ) { if ( unify_enum_key(A2, state) && unify_value(A3, state->it->value(), state->ref->builtin_merger, state->ref->type.value) ) @@ -1578,7 +1570,7 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea assert(0); return false; } - PL_fail; + return false; } PREDICATE_NONDET(rocks_enum, 4) @@ -1593,14 +1585,14 @@ PREDICATE_NONDET(rocks_enum_prefix, 5) { return rocks_enum(PL_av, 4, ENUM_PREFIX, handle, read_options(A5)); } -static PlAtom ATOM_delete("delete"); -static PlAtom ATOM_put("put"); - static void batch_operation(const dbref *ref, rocksdb::WriteBatch &batch, PlTerm e) { PlAtom name(PlAtom::null); size_t arity; + static PlAtom ATOM_delete("delete"); + static PlAtom ATOM_put("put"); + PlCheckFail(e.name_arity(&name, &arity)); if ( ATOM_delete == name && arity == 1 ) { auto key = get_slice(e[1], ref->type.key); @@ -1625,7 +1617,8 @@ PREDICATE(rocks_batch, 3) { batch_operation(ref, batch, e); } - return ok(ref->db->Write(write_options(A3), &batch)); + ok_or_throw_fail(ref->db->Write(write_options(A3), &batch)); + return true; } From 1b5fe270723d3557d05efbfa8b8fb02d0b6f75c9 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Fri, 19 May 2023 20:02:29 -0700 Subject: [PATCH 02/28] Use PlForeignContextPtr, unique_ptr for rocks_enum predicates --- cpp/rocksdb4pl.cpp | 92 +++++++++++--------------------------------- test/test_rocksdb.pl | 2 +- 2 files changed, 24 insertions(+), 70 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 4ac369d..869626d 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -1431,62 +1431,26 @@ typedef enum struct enum_state { - rocksdb::Iterator *it; // TODO: use smart ptr to auto delete + enum_state(dbref *_ref) + : ref(_ref), type(ENUM_NOT_INITIALIZED) + { } + std::unique_ptr it; dbref *ref; enum_type type; - struct - { size_t length; - char *string; - } prefix; // TODO: use std::string - bool saved; -}; - -static enum_state null_enum_state = -{ .it = nullptr, - .ref = nullptr, - .type = ENUM_NOT_INITIALIZED, - .prefix = { .length=0, .string=nullptr }, - .saved = false + std::string prefix; }; -[[nodiscard]] -static enum_state * -save_enum_state(enum_state *state) -{ if ( !state->saved ) - { auto copy = static_cast(malloc(sizeof (enum_state))); - *copy = *state; - if ( copy->prefix.string ) - { copy->prefix.string = new char[copy->prefix.length+1]; - memcpy(copy->prefix.string, state->prefix.string, copy->prefix.length+1); - } - copy->saved = true; - return copy; - } - - return state; -} - -static void // TODO: change this to ~enum_state() -free_enum_state(enum_state *state) -{ if ( state->saved ) - { if ( state->it ) - delete state->it; - if ( state->prefix.string ) - delete [] state->prefix.string; - free(state); - } -} - [[nodiscard]] static bool unify_enum_key(PlTerm t, const enum_state *state) { if ( state->type == ENUM_PREFIX ) { rocksdb::Slice k(state->it->key()); - if ( k.size_ >= state->prefix.length && - memcmp(k.data_, state->prefix.string, state->prefix.length) == 0 ) - { k.data_ += state->prefix.length; - k.size_ -= state->prefix.length; + // TODO: use std::string's compare + if ( k.size_ >= state->prefix.length() && + memcmp(k.data_, state->prefix.data(), state->prefix.length()) == 0 ) + { k.data_ += state->prefix.length(); + k.size_ -= state->prefix.length(); return unify(t, k, state->ref->type.key); } else @@ -1502,8 +1466,8 @@ static bool enum_key_prefix(const enum_state *state) { if ( state->type == ENUM_PREFIX ) { rocksdb::Slice k(state->it->key()); - return ( k.size_ >= state->prefix.length && - memcmp(k.data_, state->prefix.string, state->prefix.length) == 0 ); + return ( k.size_ >= state->prefix.length() && + memcmp(k.data_, state->prefix.data(), state->prefix.length()) == 0 ); } else return true; } @@ -1512,14 +1476,11 @@ enum_key_prefix(const enum_state *state) [[nodiscard]] static foreign_t rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::ReadOptions options) -{ enum_state state_buf = null_enum_state; - enum_state *state = &state_buf; - - // TODO: PlForeignContextPtr ctxt(handle) +{ PlForeignContextPtr state(handle); switch ( handle.foreign_control() ) { case PL_FIRST_CALL: - state->ref = get_rocks(A1); + state.set(new enum_state(get_rocks(A1))); if ( ac >= 4 ) { if ( !(state->ref->type.key == BLOB_ATOM || state->ref->type.key == BLOB_STRING || @@ -1529,42 +1490,35 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea std::string prefix = A4.get_nchars(REP_UTF8|CVT_IN|CVT_EXCEPTION); if ( type == ENUM_PREFIX ) - { state->prefix.length = prefix.size(); - state->prefix.string = prefix.data(); + { state->prefix = prefix; } - state->it = state->ref->db->NewIterator(options); + state->it.reset(state->ref->db->NewIterator(options)); state->it->Seek(prefix); } else - { state->it = state->ref->db->NewIterator(options); + { state->it.reset(state->ref->db->NewIterator(options)); state->it->SeekToFirst(); } state->type = type; - state->saved = false; - goto next; + [[fallthrough]]; case PL_REDO: - state = static_cast(handle.foreign_context_address()); - next: { PlFrame fr; for ( ; state->it->Valid(); state->it->Next() ) - { if ( unify_enum_key(A2, state) && + { if ( unify_enum_key(A2, state.get()) && unify_value(A3, state->it->value(), state->ref->builtin_merger, state->ref->type.value) ) { state->it->Next(); - if ( state->it->Valid() && enum_key_prefix(state) ) - { PL_retry_address(save_enum_state(state)); + if ( state->it->Valid() && enum_key_prefix(state.get()) ) + { state.keep(); + PL_retry_address(state.get()); } else - { free_enum_state(state); - return true; + { return true; } } fr.rewind(); } - free_enum_state(state); return false; } case PL_PRUNED: - state = static_cast(handle.foreign_context_address()); - free_enum_state(state); return true; default: assert(0); diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index 39ae643..ebf135d 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -331,7 +331,7 @@ :- begin_tests(enum, [cleanup(delete_db)]). test(enum, - [ Keys = ["aap", "aapje", "noot"], + [ Keys == ["aap", "aapje", "noot"], cleanup(delete_db) ]) :- test_db(Dir), From 9aa78dbe88c7929deb8020a8dcf8b458a370d142 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sat, 20 May 2023 08:49:59 -0700 Subject: [PATCH 03/28] Use RAII for mutexes --- cpp/rocksdb4pl.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 869626d..8c6f629 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -110,6 +110,8 @@ struct alias_cell alias_cell *next; }; +// TODO: use std::hash_map or similar for alias_entries + #define ALIAS_HASH_SIZE 64 std::mutex alias_lock; @@ -139,26 +141,24 @@ static void rocks_alias(PlAtom name, PlAtom symbol) { auto key = atom_hash(name); - alias_lock.lock(); + std::lock_guard alias_lock_(alias_lock); if ( rocks_get_alias(name).is_null() ) { auto c = new alias_cell(name, symbol, alias_entries[key]); alias_entries[key] = c; c->name.register_ref(); c->symbol.register_ref(); - alias_lock.unlock(); } else - { alias_lock.unlock(); - throw PlPermissionError("alias", "rocksdb", PlTerm_atom(name)); + { throw PlPermissionError("alias", "rocksdb", PlTerm_atom(name)); } } static void rocks_unalias(PlAtom name) { auto key = atom_hash(name); - alias_cell *c, *prev=nullptr; + alias_cell *prev=nullptr; - alias_lock.lock(); - for ( c = alias_entries[key]; c; prev=c, c = c->next ) + std::lock_guard alias_lock_(alias_lock); + for ( auto c = alias_entries[key]; c; prev=c, c = c->next ) { if ( c->name == name ) { if ( prev ) prev->next = c->next; @@ -171,7 +171,6 @@ rocks_unalias(PlAtom name) break; } } - alias_lock.unlock(); } @@ -315,9 +314,9 @@ get_rocks(PlTerm t, bool warn=true) t.get_atom_ex(&a); if ( t.not_null() ) { for ( int i=0; i<2 && a.not_null(); i++ ) - { dbref *ref; + { dbref *ref = symbol_dbref(a); - if ( (ref=symbol_dbref(a)) ) + if ( ref ) { if ( !(ref->flags & DB_DESTROYED) ) { return ref; } else if ( warn ) @@ -724,7 +723,7 @@ cmp_number(const void *v1, const void *v2) template static void sort_numbers(std::string *str) -{ auto s = const_cast(str->data()); +{ auto s = str->data(); auto len = str->length(); if ( len == 0 ) return; From 8c1b081197081a12a5b9fb07ae8246973804b571 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sat, 20 May 2023 09:42:39 -0700 Subject: [PATCH 04/28] Use std::map for alias_entries --- cpp/rocksdb4pl.cpp | 82 ++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 54 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 8c6f629..f12b883 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -101,52 +101,36 @@ struct dbref * ALIAS * *******************************/ -struct alias_cell -{ - alias_cell(PlAtom _name, PlAtom _symbol, alias_cell *_next) - : name(_name), symbol(_symbol), next(_next) { } - PlAtom name; - PlAtom symbol; - alias_cell *next; -}; - -// TODO: use std::hash_map or similar for alias_entries - -#define ALIAS_HASH_SIZE 64 - -std::mutex alias_lock; -static unsigned int alias_size = ALIAS_HASH_SIZE; -static alias_cell *alias_entries[ALIAS_HASH_SIZE]; +std::mutex rocksdb4pl_alias_lock; // global +// TODO: Define the necessary operators for PlAtom, so that it can be +// the key instead of atom_t. +static std::map alias_entries; +// rocks_get_alias() assumes that rocksdb4pl_alias_lock has been acquired [[nodiscard]] -static unsigned int -atom_hash(PlAtom a) -{ return static_cast(a.C_>>7) % alias_size; +static PlAtom +rocks_get_alias(PlAtom name) +{ auto lookup = alias_entries.find(name.C_); + if ( lookup == alias_entries.end() ) + return PlAtom(PlAtom::null); + else + return lookup->second; } [[nodiscard]] static PlAtom -rocks_get_alias(PlAtom name) -{ for ( alias_cell *c = alias_entries[atom_hash(name)]; - c; - c = c->next ) - { if ( c->name == name ) - return c->symbol; - } - - return PlAtom(PlAtom::null); +rocks_get_alias_locked(PlAtom name) +{ std::lock_guard alias_lock_(rocksdb4pl_alias_lock); + return rocks_get_alias(name); } static void rocks_alias(PlAtom name, PlAtom symbol) -{ auto key = atom_hash(name); - - std::lock_guard alias_lock_(alias_lock); +{ std::lock_guard alias_lock_(rocksdb4pl_alias_lock); if ( rocks_get_alias(name).is_null() ) - { auto c = new alias_cell(name, symbol, alias_entries[key]); - alias_entries[key] = c; - c->name.register_ref(); - c->symbol.register_ref(); + { alias_entries.insert(std::make_pair(name.C_, symbol)); + name.register_ref(); + symbol.register_ref(); } else { throw PlPermissionError("alias", "rocksdb", PlTerm_atom(name)); } @@ -154,23 +138,13 @@ rocks_alias(PlAtom name, PlAtom symbol) static void rocks_unalias(PlAtom name) -{ auto key = atom_hash(name); - alias_cell *prev=nullptr; - - std::lock_guard alias_lock_(alias_lock); - for ( auto c = alias_entries[key]; c; prev=c, c = c->next ) - { if ( c->name == name ) - { if ( prev ) - prev->next = c->next; - else - alias_entries[key] = c->next; - c->name.unregister_ref(); - c->symbol.unregister_ref(); - delete c; - - break; - } - } +{ std::lock_guard alias_lock_(rocksdb4pl_alias_lock); + auto lookup = alias_entries.find(name.C_); + if ( lookup == alias_entries.end() ) + return; + name.unregister_ref(); + lookup->second.unregister_ref(); // alias_entries[name].unregister_ref() + alias_entries.erase(lookup); } @@ -324,7 +298,7 @@ get_rocks(PlTerm t, bool warn=true) } } - a = rocks_get_alias(a); + a = rocks_get_alias_locked(a); } throw PlExistenceError("rocksdb", t); @@ -1275,7 +1249,7 @@ PREDICATE(rocks_open_, 3) } if ( alias.not_null() && once ) - { PlAtom existing = rocks_get_alias(alias); + { PlAtom existing = rocks_get_alias_locked(alias); if ( existing.not_null() ) { dbref *eref; if ( (eref=symbol_dbref(existing)) && From ab076695f2a0406aa78ee8da19ae94f76b42f088 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sat, 20 May 2023 10:29:51 -0700 Subject: [PATCH 05/28] C++ style of enum --- cpp/rocksdb4pl.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index f12b883..e14e70d 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -54,7 +54,7 @@ #define DB_DESTROYED 0x0001 /* Was destroyed by user */ #define DB_OPEN_ONCE 0x0002 /* open(once) option */ -typedef enum +enum blob_type { BLOB_ATOM = 0, /* UTF-8 string as atom */ BLOB_STRING, /* UTF-8 string as string */ BLOB_BINARY, /* Byte string as string */ @@ -63,13 +63,13 @@ typedef enum BLOB_FLOAT32, /* 32-bit IEEE float */ BLOB_FLOAT64, /* 64-bit IEEE double */ BLOB_TERM /* Arbitrary term */ -} blob_type; +}; -typedef enum +enum merger_t { MERGE_NONE = 0, MERGE_LIST, MERGE_SET -} merger_t; +}; struct dbref { @@ -1395,12 +1395,12 @@ PREDICATE(rocks_delete, 3) return true; } -typedef enum -{ ENUM_NOT_INITIALIZED, +enum enum_type +{ ENUM_NOT_INITIALIZED = 0, ENUM_ALL, // TODO: not used? ENUM_FROM, // TODO: not used? ENUM_PREFIX -} enum_type; +}; struct enum_state { From 65cbb41a957c036be63a518d105b7f276a5eb653 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sat, 20 May 2023 13:07:25 -0700 Subject: [PATCH 06/28] Add some debug info --- cpp/rocksdb4pl.cpp | 24 +++++++++++++++--------- test/test_rocksdb.pl | 29 ++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index e14e70d..5fc5386 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -33,7 +33,7 @@ POSSIBILITY OF SUCH DAMAGE. */ -#include +#include #include #include #include @@ -85,8 +85,9 @@ struct dbref { } rocksdb::DB *db; /* DB handle */ + std::string pathname; /* DB's file name (for debugging) */ PlAtom symbol; /* associated symbol */ - PlAtom name; /* alias name */ + PlAtom name; /* alias name (can be PlAtom::null) */ int flags; /* flags */ merger_t builtin_merger; /* C++ Merger */ PlRecord merger; /* merge option */ @@ -155,10 +156,15 @@ rocks_unalias(PlAtom name) [[nodiscard]] static bool write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) -{ auto refp = static_cast(eref.blob_data(nullptr, nullptr)); - auto ref = *refp; +{ auto ref = *static_cast(eref.blob_data(nullptr, nullptr)); - Sfprintf(s, "(%p)", ref); + PlStringBuffers _string_buffers; + Sfprintf(s, "(%p", ref); + Sfprintf(s, ",path=%s", ref->pathname.c_str()); + // TODO: ref->name.as_wstring() + if ( ref->name.not_null() ) + Sfprintf(s, ",alias=%Ws", Plx_atom_wchars(ref->name.C_, nullptr)); + Sfprintf(s, "%s", ")"); if ( flags&PL_WRT_NEWLINE ) Sfprintf(s, "\n"); return true; @@ -178,9 +184,9 @@ GC a rocks dbref blob from the atom garbage collector. [[nodiscard]] static bool release_rocks_ref_(PlAtom aref) -{ auto refp = static_cast(aref.blob_data(nullptr, nullptr)); - auto ref = *refp; +{ auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); + // TODO: remove this assertion? assert(ref->name.is_null()); { auto db = ref->db; @@ -204,8 +210,7 @@ release_rocks_ref(atom_t aref) [[nodiscard]] static bool save_rocks_(PlAtom aref, IOSTREAM *fd) -{ auto refp = static_cast(aref.blob_data(nullptr, nullptr)); - auto ref = *refp; +{ auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); (void)fd; return PL_warning("Cannot save reference to (%p)", ref); @@ -1264,6 +1269,7 @@ PREDICATE(rocks_open_, 3) ref->type.key = key_type; ref->type.value = value_type; ref->name = alias; + ref->pathname = fn; if ( once ) ref->flags |= DB_OPEN_ONCE; diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index ebf135d..41eff7b 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -55,14 +55,15 @@ merge, builtin_merge, properties, - enum + enum, + alias ]). :- begin_tests(rocks, [cleanup(delete_db)]). test(basic, [Noot == noot, setup(setup_db(Dir, RocksDB)), - cleanup(cleaup_db(Dir, RocksDB))]) :- + cleanup(cleanup_db(Dir, RocksDB))]) :- rocks_put(RocksDB, aap, noot), rocks_get(RocksDB, aap, Noot), rocks_delete(RocksDB, aap), @@ -70,7 +71,7 @@ test(basic, [Vs_get_expect == Vs_get_actual, setup(setup_db(Dir, RocksDB, [key(atom), value(term)])), - cleanup(cleaup_db(Dir, RocksDB))]) :- + cleanup(cleanup_db(Dir, RocksDB))]) :- % TODO: binary KVs = [atom - one, string - "two", @@ -95,7 +96,7 @@ test(basic, [Noot == noot, setup(setup_db(Dir, RocksDB)), - cleanup(cleaup_db(Dir, RocksDB))]) :- + cleanup(cleanup_db(Dir, RocksDB))]) :- rocks_put(RocksDB, aap, noot, [sync(true)]), rocks_close(RocksDB), rocks_open(Dir, RocksDB2, [mode(read_only)]), @@ -349,6 +350,24 @@ :- end_tests(enum). +:- begin_tests(alias, [cleanup(delete_db)]). + +test(basic, [blocked(assertion_error), + Noot == noot, + setup(setup_db(Dir, _, [alias(rocks_db)])), + cleanup(cleanup_db(Dir, rocks_db))]) :- + rocks_put(rocks_db, aap, noot), + rocks_get(rocks_db, aap, Noot), + rocks_delete(rocks_db, aap), + assertion(\+ rocks_get(rocks_db, aap, _)). + +test(basic2, [blocked(assertion_error), + error(permission_error(alias,rocksdb,rocks_db)), + setup(setup_db(Dir, _, [alias(rocks_db)]))]) :- + rocks_open(Dir, _, [alias(rocks_db)]). + +:- end_tests(alias). + /******************************* * UTIL * *******************************/ @@ -375,6 +394,6 @@ % cleanup_db/2 does its best to close the database because, if there % is an error in the test case and the database isn't closed, it will % cause spurious errors from all the subsequent tests. -cleaup_db(Dir, RocksDB) :- +cleanup_db(Dir, RocksDB) :- catch(ignore(rocks_close(RocksDB)), _, true), delete_db(Dir). From fcedebeba7930da66933367da36be5cf56d94d21 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sat, 20 May 2023 17:30:47 -0700 Subject: [PATCH 07/28] Add alias test --- cpp/rocksdb4pl.cpp | 2 +- prolog/rocksdb.pl | 5 +++-- test/test_rocksdb.pl | 22 +++++++++------------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 5fc5386..27dd934 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -255,7 +255,7 @@ unify_rocks(PlTerm t, dbref *ref) { if ( ref->symbol.is_null() ) { PlTerm_var tmp; PlCheckFail(tmp.unify_blob(&ref, sizeof ref, &rocks_blob)); - ref->symbol = t.as_atom(); + ref->symbol = tmp.as_atom(); rocks_alias(ref->name, ref->symbol); } return t.unify_atom(ref->name); diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index ea992a9..80122c0 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -219,7 +219,8 @@ % - alias(+Name) % Give the database a name instead of using an anonymous % handle. A named database is not subject to GC and must -% be closed explicitly. +% be closed explicitly. When the database is opened, +% RocksDB unifies with Name. % - open(+How) % If How is `once` and an alias is given, a second open simply % returns a handle to the already open database. @@ -419,7 +420,7 @@ % % True for all keys that start with Prefix. Instead of returning % the full key this predicate returns the _suffix_ of the matching -% key. This predicate succeeds deterministically no next key +% key. This predicate succeeds deterministically if no next key % exists or the next key does not match Prefix. % % Options are the same as for rocks_get/4. diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index 41eff7b..14eb84e 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -352,19 +352,15 @@ :- begin_tests(alias, [cleanup(delete_db)]). -test(basic, [blocked(assertion_error), - Noot == noot, - setup(setup_db(Dir, _, [alias(rocks_db)])), - cleanup(cleanup_db(Dir, rocks_db))]) :- - rocks_put(rocks_db, aap, noot), - rocks_get(rocks_db, aap, Noot), - rocks_delete(rocks_db, aap), - assertion(\+ rocks_get(rocks_db, aap, _)). - -test(basic2, [blocked(assertion_error), - error(permission_error(alias,rocksdb,rocks_db)), - setup(setup_db(Dir, _, [alias(rocks_db)]))]) :- - rocks_open(Dir, _, [alias(rocks_db)]). +% rocks:basic, but with a named database +test(basic, [Noot == noot, + setup(setup_db(Dir, DB, [alias('DB')])), + cleanup(cleanup_db(Dir, DB))]) :- + assertion(DB == 'DB'), + rocks_put('DB', aap, noot), + rocks_get('DB', aap, Noot), + rocks_delete('DB', aap), + assertion(\+ rocks_get('DB', aap, _)). :- end_tests(alias). From 8ff915566564504506681fc0928ec010faf428e3 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sun, 21 May 2023 11:56:57 -0700 Subject: [PATCH 08/28] dbref::pathname is now PlAtom, so dbref is unique --- cpp/rocksdb4pl.cpp | 19 ++++++++++++++----- prolog/rocksdb.pl | 3 ++- test/test_rocksdb.pl | 2 ++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 27dd934..801eccf 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -45,7 +45,7 @@ #include #include -#include // TODO: this should possibly be separate +#include // TODO: this should possibly be in a separate file /******************************* * SYMBOL * @@ -75,6 +75,7 @@ struct dbref { dbref() : db( nullptr), + pathname( PlAtom(PlAtom::null)), symbol( PlAtom(PlAtom::null)), name( PlAtom(PlAtom::null)), flags( 0), @@ -84,8 +85,9 @@ struct dbref .value = BLOB_ATOM}) { } + // All the fields must be "unique" (e.g., no std::string, but PlAtom is OK) rocksdb::DB *db; /* DB handle */ - std::string pathname; /* DB's file name (for debugging) */ + PlAtom pathname; /* DB's absolute file name (for debugging) */ PlAtom symbol; /* associated symbol */ PlAtom name; /* alias name (can be PlAtom::null) */ int flags; /* flags */ @@ -160,8 +162,9 @@ write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) PlStringBuffers _string_buffers; Sfprintf(s, "(%p", ref); - Sfprintf(s, ",path=%s", ref->pathname.c_str()); - // TODO: ref->name.as_wstring() + // TODO: ref->pathname.as_wstring(), ref->name.as_wstring() + if ( ref->pathname.not_null() ) + Sfprintf(s, ",path=%Ws", Plx_atom_wchars(ref->pathname.C_, nullptr)); if ( ref->name.not_null() ) Sfprintf(s, ",alias=%Ws", Plx_atom_wchars(ref->name.C_, nullptr)); Sfprintf(s, "%s", ")"); @@ -1269,9 +1272,12 @@ PREDICATE(rocks_open_, 3) ref->type.key = key_type; ref->type.value = value_type; ref->name = alias; - ref->pathname = fn; + ref->pathname = PlAtom(fn); if ( once ) ref->flags |= DB_OPEN_ONCE; + ref->pathname.register_ref(); + if ( ref->name.not_null() ) + ref->name.register_ref(); try { if ( ref->merger.not_null() ) @@ -1301,6 +1307,9 @@ PREDICATE(rocks_close, 1) { rocks_unalias(ref->name); ref->name.reset(); } + ref->pathname.unregister_ref(); + if ( ref->name.not_null() ) + ref->name.unregister_ref(); delete db; return true; diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index 80122c0..ac20ce6 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -288,7 +288,8 @@ rocks_open(Dir, DB, Options0) :- meta_options(is_meta, Options0, Options), - rocks_open_(Dir, DB, Options). + absolute_file_name(Dir, DirAbs), + rocks_open_(DirAbs, DB, Options). is_meta(merge). diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index 14eb84e..832afd2 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -131,6 +131,7 @@ rocks_open(Dir, RocksDB, [key(string), value(string), mode(read_write)]), rocks_put(RocksDB, "one", "àmímé níshíkíhéꜜbì"), rocks_close(RocksDB)). + test(open_twice, [error(rocks_error(_),_), % TODO: error(rocks_error('IO error: lock hold by current process, acquire time 1657004085 acquiring thread 136471553664960: /tmp/test_rocksdb/LOCK: No locks available'),_) cleanup(delete_db)]) :- test_db(Dir), @@ -139,6 +140,7 @@ call_cleanup(rocks_open(Dir, RocksDB2, []), rocks_close(RocksDB2)), rocks_close(RocksDB1)). + test(batch, [Pairs == [zus-noot], cleanup(delete_db)]) :- test_db(Dir), From a6b43c19221fc2baf5f722e5f042010a19fd451f Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Wed, 31 May 2023 19:32:01 -0700 Subject: [PATCH 09/28] rocks_close/1 with an anonymous handle doesn't throw an exception --- cpp/rocksdb4pl.cpp | 92 +++++++++++++++++++++++--------------------- prolog/rocksdb.pl | 25 +++++++++--- test/test_rocksdb.pl | 11 +++++- 3 files changed, 77 insertions(+), 51 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 801eccf..ad66a95 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -71,6 +71,17 @@ enum merger_t MERGE_SET }; +// TODO: rocks_blob.flags has PL_BLOB_UNIQUE, which means that the +// contents of a dbref object must be "unique". Therefore, none +// of the fields can be something like std::string because the +// atom lookup uses byte comparison and std::string contains a +// hidden pointer that is different for each instance. However, +// the `merger` field is created using PlTerm::record(), which +// uses PL_record(), which would result in different values even +// if the underlying terms are identical. +// ... therefore, we should probably remove the PL_BLOB_UNIQUE and +// also should change the blob from being a dbref** to being a +// dbref*. struct dbref { dbref() @@ -145,6 +156,11 @@ rocks_unalias(PlAtom name) auto lookup = alias_entries.find(name.C_); if ( lookup == alias_entries.end() ) return; + // TODO: As an alternative to removing the entry, leave it in place + // (with flags&DB_DESTROYED showing that it's been closed or + // with the value as PlAtom::null), so that rocks_close/1 can + // distinguish an alias lookup that should throw a + // PlExistenceError because it's never been opened. name.unregister_ref(); lookup->second.unregister_ref(); // alias_entries[name].unregister_ref() alias_entries.erase(lookup); @@ -160,13 +176,13 @@ static bool write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) { auto ref = *static_cast(eref.blob_data(nullptr, nullptr)); - PlStringBuffers _string_buffers; Sfprintf(s, "(%p", ref); - // TODO: ref->pathname.as_wstring(), ref->name.as_wstring() if ( ref->pathname.not_null() ) - Sfprintf(s, ",path=%Ws", Plx_atom_wchars(ref->pathname.C_, nullptr)); + Sfprintf(s, ",path=%Ws", ref->pathname.as_wstring().c_str()); if ( ref->name.not_null() ) - Sfprintf(s, ",alias=%Ws", Plx_atom_wchars(ref->name.C_, nullptr)); + Sfprintf(s, ",alias=%Ws", ref->name.as_wstring().c_str()); + if ( ref->flags & DB_DESTROYED ) + Sfprintf(s, ",CLOSED"); Sfprintf(s, "%s", ")"); if ( flags&PL_WRT_NEWLINE ) Sfprintf(s, "\n"); @@ -199,7 +215,7 @@ release_rocks_ref_(PlAtom aref) } } ref->merger.erase(); - delete ref; + delete ref; // TODO: delete **ref is done by system return true; } @@ -272,13 +288,12 @@ unify_rocks(PlTerm t, dbref *ref) [[nodiscard]] static dbref * symbol_dbref(PlAtom symbol) -{ void *data; - size_t len; +{ size_t len; PL_blob_t *type; + auto data = static_cast(symbol.blob_data(&len, &type)); - if ( (data=symbol.blob_data(&len, &type)) && type == &rocks_blob ) - { auto erd = static_cast(data); - return *erd; + if ( data && type == &rocks_blob ) + { return *data; } return static_cast(nullptr); @@ -287,32 +302,20 @@ symbol_dbref(PlAtom symbol) [[nodiscard]] static dbref * -get_rocks(PlTerm t, bool warn=true) -{ PlAtom a(PlAtom::null); - - if ( warn ) - a = t.as_atom(); - else - t.get_atom_ex(&a); - if ( t.not_null() ) - { for ( int i=0; i<2 && a.not_null(); i++ ) - { dbref *ref = symbol_dbref(a); - - if ( ref ) - { if ( !(ref->flags & DB_DESTROYED) ) - { return ref; - } else if ( warn ) - { throw PlExistenceError("rocksdb", t); - } - } - - a = rocks_get_alias_locked(a); - } - - throw PlExistenceError("rocksdb", t); +get_rocks(PlTerm t, bool throw_if_closed=true) +{ PlAtom a(t.as_atom()); // Throws type error if not an atom + + auto ref = symbol_dbref(a); + if ( ! ref ) + { a = rocks_get_alias_locked(a); + if ( a.not_null() ) + ref = symbol_dbref(a); } + if ( throw_if_closed && + ( !ref || (ref->flags & DB_DESTROYED) ) ) + throw PlExistenceError("rocksdb", t); - throw PlExistenceError("rocksdb", t); + return ref; } @@ -1199,14 +1202,14 @@ lookup_optdef_and_apply(rocksdb::Options *options, PREDICATE(rocks_open_, 3) { rocksdb::Options options; options.create_if_missing = true; - char *fn; + char *fn; // from A1 - assumes that it's already absolute file name blob_type key_type = BLOB_ATOM; blob_type value_type = BLOB_ATOM; merger_t builtin_merger = MERGE_NONE; PlAtom alias(PlAtom::null); PlRecord merger(PlRecord::null); - int once = false; - int read_only = false; + bool once = false; + bool read_only = false; static const PlAtom ATOM_key("key"); static const PlAtom ATOM_value("value"); @@ -1226,7 +1229,7 @@ PREDICATE(rocks_open_, 3) size_t arity; PlCheckFail(opt.name_arity(&name, &arity)); - if ( arity == 1 ) + if ( arity == 1 ) { if ( ATOM_key == name ) get_blob_type(opt[1], &key_type, static_cast(nullptr)); else if ( ATOM_value == name ) @@ -1259,9 +1262,8 @@ PREDICATE(rocks_open_, 3) if ( alias.not_null() && once ) { PlAtom existing = rocks_get_alias_locked(alias); if ( existing.not_null() ) - { dbref *eref; - if ( (eref=symbol_dbref(existing)) && - (eref->flags&DB_OPEN_ONCE) ) + { auto eref = symbol_dbref(existing); + if ( eref && (eref->flags&DB_OPEN_ONCE) ) return A2.unify_atom(existing); } } @@ -1289,7 +1291,7 @@ PREDICATE(rocks_open_, 3) else ok_or_throw_fail(rocksdb::DB::Open(options, fn, &ref->db)); PlCheckFail(unify_rocks(A2, ref)); - } catch(...) + } catch(...) // TODO - better to do this with a unique_ptr or similar { delete ref; throw; } @@ -1298,8 +1300,10 @@ PREDICATE(rocks_open_, 3) PREDICATE(rocks_close, 1) -{ dbref *ref = get_rocks(A1); - rocksdb::DB* db = ref->db; +{ dbref *ref = get_rocks(A1, false); + if ( !ref ) + return true; + auto db = ref->db; ref->db = nullptr; ref->flags |= DB_DESTROYED; diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index ac20ce6..5216651 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -281,9 +281,12 @@ % % @bug You must call rocks_close(Directory) to ensure clean shutdown -% Failure to call rdb_close/1 usually doesn't result in data -% loss because rocksdb can recover, depending on the setting -% of the `sync` option. +% Failure to call rdb_close/1 usually doesn't result in data +% loss because rocksdb can recover, depending on the setting of +% the `sync` option. However, it is recommended that you do a +% clean shutdown if possible, such as using at_halt/1 or +% setup_call_cleanup/3 is used to ensure clean shutdown. + % @see https://github.com/facebook/rocksdb/wiki/Known-Issues rocks_open(Dir, DB, Options0) :- @@ -296,8 +299,20 @@ %! rocks_close(+RocksDB) is det. % -% Destroy the RocksDB handle. Note that anonymous handles are -% subject to (atom) garbage collection. +% Destroy the RocksDB handle. Note that anonymous handles are +% subject to (atom) garbage collection, which will call +% rocks_close/1 as part of the garbage collection; however, +% there is no guarantee that an anonymous handle will be garbage +% collected, so it is suggested that at_halt/1 or +% setup_call_cleanup/3 is used to ensure that rocks_close/1 is +% called. +% +% rocks_close/1 throws an existence error if RocksDB isn't a +% valid handle or alias from rocks_open/3. If RocksDB is an +% anonymous handle that has been closed, rocks_close/1 silently +% succeeds; if it's an alias name that's already been closed, an +% existence error is raised (this behavior may change in +% future). %! rocks_put(+RocksDB, +Key, +Value) is det. %! rocks_put(+RocksDB, +Key, +Value, Options) is det. diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index 832afd2..067861f 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -348,7 +348,8 @@ findall(Key, rocks_enum_from(RocksDB, Key, _, aap), Keys), rocks_enum_prefix(RocksDB, Rest, _, aapj), assertion(Rest == "e"), - rocks_close(RocksDB). + rocks_close(RocksDB), % test double-close + rocks_close(RocksDB). :- end_tests(enum). @@ -362,7 +363,13 @@ rocks_put('DB', aap, noot), rocks_get('DB', aap, Noot), rocks_delete('DB', aap), - assertion(\+ rocks_get('DB', aap, _)). + assertion(\+ rocks_get('DB', aap, _)), + % The following would throw an exception because the code + % doesn't distinguish between a db alias that's been closed + % and a db alias that's never been opened: + % rocks_close('DB'), % test double-close + % rocks_close('DB'). + true. :- end_tests(alias). From 505555ff7b7c510690c46d43818757c61ed886b4 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Thu, 1 Jun 2023 13:16:35 -0700 Subject: [PATCH 10/28] Remove "once" option from rocks_open/3 --- cpp/rocksdb4pl.cpp | 17 +++-------------- prolog/rocksdb.pl | 18 +++++++++++------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index ad66a95..050f60d 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -52,7 +52,6 @@ *******************************/ #define DB_DESTROYED 0x0001 /* Was destroyed by user */ -#define DB_OPEN_ONCE 0x0002 /* open(once) option */ enum blob_type { BLOB_ATOM = 0, /* UTF-8 string as atom */ @@ -1208,7 +1207,6 @@ PREDICATE(rocks_open_, 3) merger_t builtin_merger = MERGE_NONE; PlAtom alias(PlAtom::null); PlRecord merger(PlRecord::null); - bool once = false; bool read_only = false; static const PlAtom ATOM_key("key"); @@ -1216,7 +1214,6 @@ PREDICATE(rocks_open_, 3) static const PlAtom ATOM_alias("alias"); static const PlAtom ATOM_merge("merge"); static const PlAtom ATOM_open("open"); - static const PlAtom ATOM_once("once"); static const PlAtom ATOM_mode("mode"); static const PlAtom ATOM_read_write("read_write"); static const PlAtom ATOM_read_only("read_only"); @@ -1238,12 +1235,6 @@ PREDICATE(rocks_open_, 3) merger = opt[1].record(); else if ( ATOM_alias == name ) { alias = opt[1].as_atom(); - once = true; - } else if ( ATOM_open == name ) - { if ( ATOM_once == opt[1].as_atom() ) - once = true; - else - throw PlDomainError("open_option", opt[1]); } else if ( ATOM_mode == name ) { PlAtom a = opt[1].as_atom(); if ( ATOM_read_write == a ) @@ -1259,11 +1250,11 @@ PREDICATE(rocks_open_, 3) throw PlTypeError("option", opt); } - if ( alias.not_null() && once ) + if ( alias.not_null() ) { PlAtom existing = rocks_get_alias_locked(alias); if ( existing.not_null() ) { auto eref = symbol_dbref(existing); - if ( eref && (eref->flags&DB_OPEN_ONCE) ) + if ( eref ) return A2.unify_atom(existing); } } @@ -1275,8 +1266,6 @@ PREDICATE(rocks_open_, 3) ref->type.value = value_type; ref->name = alias; ref->pathname = PlAtom(fn); - if ( once ) - ref->flags |= DB_OPEN_ONCE; ref->pathname.register_ref(); if ( ref->name.not_null() ) ref->name.register_ref(); @@ -1291,7 +1280,7 @@ PREDICATE(rocks_open_, 3) else ok_or_throw_fail(rocksdb::DB::Open(options, fn, &ref->db)); PlCheckFail(unify_rocks(A2, ref)); - } catch(...) // TODO - better to do this with a unique_ptr or similar + } catch(...) // TODO - remove this by using unique_ptr or similar { delete ref; throw; } diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index 5216651..37d853b 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -66,7 +66,6 @@ :- predicate_options(rocks_open/3, 3, [ alias(atom), - open(oneof([once])), mode(oneof([read_only,read_write])), key(oneof([atom,string,binary,int32,int64, float,double,term])), @@ -212,7 +211,12 @@ %! rocks_open(+Directory, -RocksDB, +Options) is det. % % Open a RocksDB database in Directory and unify RocksDB with a -% handle to the opened database. Most of the `DBOptions` in +% handle to the opened database. In general, this predicate +% throws an exception on failure; if an error occurs in the +% rocksdb library, the error term is of the form +% rocks_error(Message,_). +% +% Most of the `DBOptions` in % `rocksdb/include/rocksdb/options.h` are supported, in addition % to the following options: % @@ -221,9 +225,6 @@ % handle. A named database is not subject to GC and must % be closed explicitly. When the database is opened, % RocksDB unifies with Name. -% - open(+How) -% If How is `once` and an alias is given, a second open simply -% returns a handle to the already open database. % - key(+Type) % - value(+Type) % Define the type for the key and value. This must be @@ -270,9 +271,12 @@ % Define RocksDB value merging. See rocks_merge/3. % - mode(+Mode) % One of `read_write` (default) or `read_only`. The latter -% uses OpenForReadOnly() to open the database. +% uses OpenForReadOnly() to open the database. It is allowed +% to have multiple `read_only` opens, but only one +% `read_write` (which also precludes having any `read_only`); +% however, it is recommended to only open a databse once. % - optimize_for_small_db(true) - Use this if your DB is very -% small (like under 1GB) and you don't want tog +% small (like under 1GB) and you don't want to % spend lots of memory for memtables. % - increase_parallelism(true) - see DBOptions::IncreaseParallelism() % @see https://github.com/facebook/rocksdb/wiki/RocksDB-Tuning-Guide From 63e03a705ee628c74ad1ec0ca476e019071e5897 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Thu, 1 Jun 2023 13:35:10 -0700 Subject: [PATCH 11/28] Remove dbref::flags and use !db to test for DB not open --- cpp/rocksdb4pl.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 050f60d..121633e 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -51,8 +51,6 @@ * SYMBOL * *******************************/ -#define DB_DESTROYED 0x0001 /* Was destroyed by user */ - enum blob_type { BLOB_ATOM = 0, /* UTF-8 string as atom */ BLOB_STRING, /* UTF-8 string as string */ @@ -88,7 +86,6 @@ struct dbref pathname( PlAtom(PlAtom::null)), symbol( PlAtom(PlAtom::null)), name( PlAtom(PlAtom::null)), - flags( 0), builtin_merger( MERGE_NONE), merger( PlRecord(PlRecord::null)), type( { .key = BLOB_ATOM, @@ -100,7 +97,6 @@ struct dbref PlAtom pathname; /* DB's absolute file name (for debugging) */ PlAtom symbol; /* associated symbol */ PlAtom name; /* alias name (can be PlAtom::null) */ - int flags; /* flags */ merger_t builtin_merger; /* C++ Merger */ PlRecord merger; /* merge option */ struct @@ -156,7 +152,7 @@ rocks_unalias(PlAtom name) if ( lookup == alias_entries.end() ) return; // TODO: As an alternative to removing the entry, leave it in place - // (with flags&DB_DESTROYED showing that it's been closed or + // (with db==nullptr showing that it's been closed; or // with the value as PlAtom::null), so that rocks_close/1 can // distinguish an alias lookup that should throw a // PlExistenceError because it's never been opened. @@ -180,7 +176,7 @@ write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) Sfprintf(s, ",path=%Ws", ref->pathname.as_wstring().c_str()); if ( ref->name.not_null() ) Sfprintf(s, ",alias=%Ws", ref->name.as_wstring().c_str()); - if ( ref->flags & DB_DESTROYED ) + if ( !ref->db ) Sfprintf(s, ",CLOSED"); Sfprintf(s, "%s", ")"); if ( flags&PL_WRT_NEWLINE ) @@ -311,7 +307,7 @@ get_rocks(PlTerm t, bool throw_if_closed=true) ref = symbol_dbref(a); } if ( throw_if_closed && - ( !ref || (ref->flags & DB_DESTROYED) ) ) + ( !ref || !ref->db ) ) throw PlExistenceError("rocksdb", t); return ref; @@ -1295,7 +1291,6 @@ PREDICATE(rocks_close, 1) auto db = ref->db; ref->db = nullptr; - ref->flags |= DB_DESTROYED; if ( ref->name.not_null() ) { rocks_unalias(ref->name); ref->name.reset(); From 22af7857a3621cc8268d1a48549980ff7c03de7c Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Fri, 2 Jun 2023 15:00:41 -0700 Subject: [PATCH 12/28] Add blob to rocks_error term --- cpp/rocksdb4pl.cpp | 32 ++++++++++++++++++-------------- prolog/rocksdb.pl | 2 +- test/test_rocksdb.pl | 7 ++++--- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 121633e..7a0fd4e 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -95,7 +95,7 @@ struct dbref // All the fields must be "unique" (e.g., no std::string, but PlAtom is OK) rocksdb::DB *db; /* DB handle */ PlAtom pathname; /* DB's absolute file name (for debugging) */ - PlAtom symbol; /* associated symbol */ + PlAtom symbol; /* associated symbol (used for error terms) */ PlAtom name; /* alias name (can be PlAtom::null) */ merger_t builtin_merger; /* C++ Merger */ PlRecord merger; /* merge option */ @@ -318,24 +318,28 @@ get_rocks(PlTerm t, bool throw_if_closed=true) * UTIL * *******************************/ -PlException RocksError(const rocksdb::Status &status) -{ return PlGeneralError(PlCompound("rocks_error", - PlTermv(PlTerm_atom(status.ToString())))); +PlException RocksError(const rocksdb::Status &status, const dbref *ref = nullptr) +{ if ( ref && ref->symbol.not_null() ) + return PlGeneralError(PlCompound("rocks_error", + PlTermv(PlTerm_atom(status.ToString()), + PlTerm_atom(ref->symbol)))); + return PlGeneralError(PlCompound("rocks_error", + PlTermv(PlTerm_atom(status.ToString())))); } static bool -ok(const rocksdb::Status &status) +ok(const rocksdb::Status &status, const dbref *ref = nullptr) { if ( status.ok() ) return true; if ( status.IsNotFound() ) return false; - throw RocksError(status); + throw RocksError(status, ref); } static void -ok_or_throw_fail(const rocksdb::Status &status) -{ PlCheckFail(ok(status)); +ok_or_throw_fail(const rocksdb::Status &status, const dbref *ref = nullptr) +{ PlCheckFail(ok(status, ref)); } class PlSlice @@ -1330,7 +1334,7 @@ PREDICATE(rocks_put, 4) if ( ref->builtin_merger == MERGE_NONE ) { auto value = get_slice(A3, ref->type.value); - ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value->slice())); + ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value->slice()), ref); } else { PlTerm_tail list(A3); PlTerm_var tmp; @@ -1344,7 +1348,7 @@ PREDICATE(rocks_put, 4) if ( ref->builtin_merger == MERGE_SET ) sort(&value, ref->type.value); - ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value)); + ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value), ref); } return true; @@ -1358,7 +1362,7 @@ PREDICATE(rocks_merge, 4) auto key = get_slice(A2,ref->type.key); auto value = get_slice(A3, ref->type.value); - ok_or_throw_fail(ref->db->Merge(write_options(A4), key->slice(), value->slice())); + ok_or_throw_fail(ref->db->Merge(write_options(A4), key->slice(), value->slice()), ref); return true; } @@ -1386,7 +1390,7 @@ PREDICATE(rocks_get, 4) std::string value; auto key = get_slice(A2, ref->type.key); - ok_or_throw_fail(ref->db->Get(read_options(A4), key->slice(), &value)); + ok_or_throw_fail(ref->db->Get(read_options(A4), key->slice(), &value), ref); return unify_value(A3, value, ref->builtin_merger, ref->type.value); } @@ -1394,7 +1398,7 @@ PREDICATE(rocks_delete, 3) { dbref *ref = get_rocks(A1); auto key = get_slice(A2, ref->type.key); - ok_or_throw_fail(ref->db->Delete(write_options(A3), key->slice())); + ok_or_throw_fail(ref->db->Delete(write_options(A3), key->slice()), ref); return true; } @@ -1547,7 +1551,7 @@ PREDICATE(rocks_batch, 3) { batch_operation(ref, batch, e); } - ok_or_throw_fail(ref->db->Write(write_options(A3), &batch)); + ok_or_throw_fail(ref->db->Write(write_options(A3), &batch), ref); return true; } diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index 37d853b..a382322 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -214,7 +214,7 @@ % handle to the opened database. In general, this predicate % throws an exception on failure; if an error occurs in the % rocksdb library, the error term is of the form -% rocks_error(Message,_). +% rocks_error(Message) or rocks_error(Message,Blob). % % Most of the `DBOptions` in % `rocksdb/include/rocksdb/options.h` are supported, in addition diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index 067861f..a7de286 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -106,7 +106,7 @@ cleanup(delete_db)]) :- test_db(Dir), rocks_open(Dir, _RocksDB, [error_if_exists(xxx)]). -test(options2, [error(type_error(option,this_is_not_an_option(123)),_), +test(options2, [error(type_error(option,this_is_not_an_option(123))), cleanup(delete_db)]) :- test_db(Dir), rocks_open(Dir, _RocksDB, [this_is_not_an_option(123)]). @@ -114,13 +114,14 @@ cleanup(delete_db)]) :- test_db(Dir), rocks_open(Dir, _RocksDB, [mode(not_read_write)]). -test(options4, [error(rocks_error('Not implemented: Not supported operation in read only mode.'),_), +% TODO: verify error(rocks-error('...',Blob), the Blob is of type 'rocksdb' +test(options4, [error(rocks_error('Not implemented: Not supported operation in read only mode.',_)), cleanup(delete_db)]) :- test_db(Dir), rocks_open(Dir, RocksDB0, [key(string), value(string)]), rocks_close(RocksDB0), setup_call_cleanup( - rocks_open(Dir, RocksDB, [mode(read_only)]), + rocks_open(Dir, RocksDB, [mode(read_only), key(string), value(string)]), ( rocks_put(RocksDB, "one", "àmímé níshíkíhéꜜbì"), rocks_get(RocksDB, "one", "àmímé níshíkíhéꜜbì") ), From 2c3a9bcfff83d1f9c16a8d9e40fc8be9f12ede5f Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sun, 4 Jun 2023 16:14:57 -0700 Subject: [PATCH 13/28] Simplify unification, rocks_close/1, and cleanup - Move dbref cleanup code to destructor - Fix rocks_close/1 cleanup - Add acquire_rocks_ref() for consistent handling of dbref::symbol - Simplify error handling cleanup by using unique_ptr - Fix some potential memory/resource leaks Dbref blob save/load gives an error Added some "consts"; changed some pointers to references Added rocks_alias_lookup/2 --- cpp/rocksdb4pl.cpp | 415 +++++++++++++++++++++++++------------------ prolog/rocksdb.pl | 31 +++- test/test_rocksdb.pl | 21 ++- 3 files changed, 288 insertions(+), 179 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 7a0fd4e..2135474 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -47,6 +47,13 @@ #include // TODO: this should possibly be in a separate file +[[nodiscard]] static PlAtom rocks_get_alias(PlAtom name); +[[nodiscard]] static PlAtom rocks_get_alias_inside_lock(PlAtom name); +static void rocks_add_alias(PlAtom name, PlAtom symbol); +static void rocks_add_alias_inside_lock(PlAtom name, PlAtom symbol); +static void rocks_unalias(PlAtom name); +static void rocks_unalias_inside_lock(PlAtom name); + /******************************* * SYMBOL * *******************************/ @@ -62,29 +69,36 @@ enum blob_type BLOB_TERM /* Arbitrary term */ }; +static const char* blob_type_char[] = +{ "atom", + "string", + "binary", + "int32", + "int64", + "float32", + "float64", + "term" +}; + enum merger_t { MERGE_NONE = 0, MERGE_LIST, MERGE_SET }; -// TODO: rocks_blob.flags has PL_BLOB_UNIQUE, which means that the -// contents of a dbref object must be "unique". Therefore, none -// of the fields can be something like std::string because the -// atom lookup uses byte comparison and std::string contains a -// hidden pointer that is different for each instance. However, -// the `merger` field is created using PlTerm::record(), which -// uses PL_record(), which would result in different values even -// if the underlying terms are identical. -// ... therefore, we should probably remove the PL_BLOB_UNIQUE and -// also should change the blob from being a dbref** to being a -// dbref*. +static const char* merge_t_char[] = +{ "none", + "list", + "set" +}; + + struct dbref { dbref() : db( nullptr), pathname( PlAtom(PlAtom::null)), - symbol( PlAtom(PlAtom::null)), + symbol( PlAtom(PlAtom::null)), // filled in by acquire_rocks_ref_() name( PlAtom(PlAtom::null)), builtin_merger( MERGE_NONE), merger( PlRecord(PlRecord::null)), @@ -92,7 +106,22 @@ struct dbref .value = BLOB_ATOM}) { } - // All the fields must be "unique" (e.g., no std::string, but PlAtom is OK) + ~dbref() + { // See also predicate rocks_close/1 + if ( name.not_null() ) + { rocks_unalias(name); + name.unregister_ref(); + } + if ( merger.not_null() ) + merger.erase(); + if ( pathname.not_null() ) + pathname.unregister_ref(); + if ( db ) + { (void)db->Close(); // TODO: print warning on failure? + delete db; + } + } + rocksdb::DB *db; /* DB handle */ PlAtom pathname; /* DB's absolute file name (for debugging) */ PlAtom symbol; /* associated symbol (used for error terms) */ @@ -106,6 +135,32 @@ struct dbref }; +static void acquire_rocks_ref_(PlAtom eref); +static bool release_rocks_ref_(PlAtom aref); +static bool write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags); +static bool write_rocks_ref_(IOSTREAM *s, const dbref *reff, int flags); +static bool save_rocks_(PlAtom aref, IOSTREAM *fd); +static PlAtom load_rocks_(IOSTREAM *fd); + +static void acquire_rocks_ref(atom_t symbol); +static int release_rocks_ref(atom_t aref); +static int write_rocks_ref(IOSTREAM *s, atom_t eref, int flags); +static int save_rocks(atom_t aref, IOSTREAM *fd); +static atom_t load_rocks(IOSTREAM *fd); + +static PL_blob_t rocks_blob = +{ .magic = PL_BLOB_MAGIC, + .flags = PL_BLOB_UNIQUE, // TODO: 0 or PL_BLOB_NOCOPY (see PrologMergeOperator()) + .name = "rocksdb", + .release = release_rocks_ref, + .compare = nullptr, + .write = write_rocks_ref, + .acquire = acquire_rocks_ref, + .save = save_rocks, + .load = load_rocks +}; + + /******************************* * ALIAS * *******************************/ @@ -115,11 +170,10 @@ std::mutex rocksdb4pl_alias_lock; // global // the key instead of atom_t. static std::map alias_entries; -// rocks_get_alias() assumes that rocksdb4pl_alias_lock has been acquired [[nodiscard]] static PlAtom -rocks_get_alias(PlAtom name) -{ auto lookup = alias_entries.find(name.C_); +rocks_get_alias_inside_lock(PlAtom name) +{ const auto lookup = alias_entries.find(name.C_); if ( lookup == alias_entries.end() ) return PlAtom(PlAtom::null); else @@ -128,32 +182,37 @@ rocks_get_alias(PlAtom name) [[nodiscard]] static PlAtom -rocks_get_alias_locked(PlAtom name) +rocks_get_alias(PlAtom name) { std::lock_guard alias_lock_(rocksdb4pl_alias_lock); - return rocks_get_alias(name); + return rocks_get_alias_inside_lock(name); } static void -rocks_alias(PlAtom name, PlAtom symbol) -{ std::lock_guard alias_lock_(rocksdb4pl_alias_lock); - if ( rocks_get_alias(name).is_null() ) +rocks_add_alias_inside_lock(PlAtom name, PlAtom symbol) +{ const auto lookup = rocks_get_alias_inside_lock(name); + if ( lookup.is_null() ) { alias_entries.insert(std::make_pair(name.C_, symbol)); name.register_ref(); symbol.register_ref(); - } else + } else if ( lookup != symbol ) { throw PlPermissionError("alias", "rocksdb", PlTerm_atom(name)); } } static void -rocks_unalias(PlAtom name) +rocks_add_alias(PlAtom name, PlAtom symbol) { std::lock_guard alias_lock_(rocksdb4pl_alias_lock); - auto lookup = alias_entries.find(name.C_); + rocks_add_alias_inside_lock(name, symbol); +} + +static void +rocks_unalias_inside_lock(PlAtom name) +{ auto lookup = alias_entries.find(name.C_); if ( lookup == alias_entries.end() ) return; // TODO: As an alternative to removing the entry, leave it in place - // (with db==nullptr showing that it's been closed; or - // with the value as PlAtom::null), so that rocks_close/1 can + // (with db==nullptr showing that it's been closed; or with + // the value as PlAtom::null), so that rocks_close/1 can // distinguish an alias lookup that should throw a // PlExistenceError because it's never been opened. name.unregister_ref(); @@ -161,29 +220,59 @@ rocks_unalias(PlAtom name) alias_entries.erase(lookup); } +static void +rocks_unalias(PlAtom name) +{ std::lock_guard alias_lock_(rocksdb4pl_alias_lock); + rocks_unalias_inside_lock(name); +} + /******************************* * SYMBOL REFERENCES * *******************************/ -[[nodiscard]] -static bool -write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) +static void +acquire_rocks_ref_(PlAtom eref) { auto ref = *static_cast(eref.blob_data(nullptr, nullptr)); + ref->symbol = eref; +} + +static void +acquire_rocks_ref(atom_t symbol) +{ acquire_rocks_ref_(PlAtom(symbol)); +} - Sfprintf(s, "(%p", ref); + +[[nodiscard]] +static bool +write_rocks_ref_(IOSTREAM *s, const dbref *ref, int flags) +{ Sfprintf(s, "(%p", ref); if ( ref->pathname.not_null() ) Sfprintf(s, ",path=%Ws", ref->pathname.as_wstring().c_str()); if ( ref->name.not_null() ) Sfprintf(s, ",alias=%Ws", ref->name.as_wstring().c_str()); + if (ref->builtin_merger != MERGE_NONE) + Sfprintf(s, ",builtin_merger=%s", merge_t_char[ref->builtin_merger]); + if ( ref->merger.not_null() ) + { auto m(ref->merger.term()); + if ( m.not_null() ) + Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()); + } if ( !ref->db ) Sfprintf(s, ",CLOSED"); - Sfprintf(s, "%s", ")"); + Sfprintf(s, ",key=%s,value=%s)", blob_type_char[ref->type.key], blob_type_char[ref->type.value]); if ( flags&PL_WRT_NEWLINE ) Sfprintf(s, "\n"); return true; } +[[nodiscard]] +static bool +write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) +{ const auto ref = *static_cast(eref.blob_data(nullptr, nullptr)); + return write_rocks_ref_(s, ref, flags); +} + [[nodiscard]] static int // TODO: bool write_rocks_ref(IOSTREAM *s, atom_t eref, int flags) @@ -200,17 +289,7 @@ static bool release_rocks_ref_(PlAtom aref) { auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); - // TODO: remove this assertion? - assert(ref->name.is_null()); - - { auto db = ref->db; - if ( db ) - { ref->db = nullptr; - delete db; - } - } - ref->merger.erase(); - delete ref; // TODO: delete **ref is done by system + delete ref; // TODO: delete aref.blob_data() is done by Prolog return true; } @@ -224,7 +303,7 @@ release_rocks_ref(atom_t aref) [[nodiscard]] static bool save_rocks_(PlAtom aref, IOSTREAM *fd) -{ auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); +{ const auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); (void)fd; return PL_warning("Cannot save reference to (%p)", ref); @@ -241,42 +320,18 @@ static PlAtom load_rocks_(IOSTREAM *fd) { (void)fd; - return PlAtom(""); + (void)PL_warning("Cannot load reference to "); + PL_fatal_error("Cannot load reference to "); + return PlAtom(PlAtom::null); } static atom_t load_rocks(IOSTREAM *fd) -{ return load_rocks_(fd).C_; -} - -static PL_blob_t rocks_blob = -{ .magic = PL_BLOB_MAGIC, - .flags = PL_BLOB_UNIQUE, - .name = "rocksdb", - .release = release_rocks_ref, - .compare = nullptr, - .write = write_rocks_ref, - .acquire = nullptr, - .save = save_rocks, // TODO: implement - .load = load_rocks // TODO: implement -}; - - -[[nodiscard]] -static bool -unify_rocks(PlTerm t, dbref *ref) -{ if ( ref->name.not_null() ) - { if ( ref->symbol.is_null() ) - { PlTerm_var tmp; - PlCheckFail(tmp.unify_blob(&ref, sizeof ref, &rocks_blob)); - ref->symbol = tmp.as_atom(); - rocks_alias(ref->name, ref->symbol); - } - return t.unify_atom(ref->name); - } else if ( ref->symbol.not_null() ) - { return t.unify_atom(ref->symbol); - } else return ( t.unify_blob(&ref, sizeof ref, &rocks_blob) && - t.get_atom(&ref->symbol) ); +{ const auto a = load_rocks_(fd); + if ( a.not_null() ) + return a.C_; + PL_fatal_error("Cannot load reference to "); + return 0; } @@ -285,7 +340,7 @@ static dbref * symbol_dbref(PlAtom symbol) { size_t len; PL_blob_t *type; - auto data = static_cast(symbol.blob_data(&len, &type)); + const auto data = static_cast(symbol.blob_data(&len, &type)); if ( data && type == &rocks_blob ) { return *data; @@ -302,7 +357,7 @@ get_rocks(PlTerm t, bool throw_if_closed=true) auto ref = symbol_dbref(a); if ( ! ref ) - { a = rocks_get_alias_locked(a); + { a = rocks_get_alias(a); if ( a.not_null() ) ref = symbol_dbref(a); } @@ -318,13 +373,14 @@ get_rocks(PlTerm t, bool throw_if_closed=true) * UTIL * *******************************/ -PlException RocksError(const rocksdb::Status &status, const dbref *ref = nullptr) +static PlException +RocksError(const rocksdb::Status &status, const dbref *ref) { if ( ref && ref->symbol.not_null() ) return PlGeneralError(PlCompound("rocks_error", - PlTermv(PlTerm_atom(status.ToString()), - PlTerm_atom(ref->symbol)))); + PlTermv(PlTerm_atom(status.ToString()), + PlTerm_atom(ref->symbol)))); return PlGeneralError(PlCompound("rocks_error", - PlTermv(PlTerm_atom(status.ToString())))); + PlTermv(PlTerm_atom(status.ToString())))); } @@ -342,6 +398,7 @@ ok_or_throw_fail(const rocksdb::Status &status, const dbref *ref = nullptr) { PlCheckFail(ok(status, ref)); } + class PlSlice { public: @@ -407,23 +464,23 @@ get_slice(PlTerm t, blob_type type) case BLOB_BINARY: return std::make_unique(t.get_nchars(CVT_IN|CVT_EXCEPTION)); case BLOB_INT32: - { int32_t v; - t.integer(&v); - return std::make_unique>(v); - } + { int32_t v; + t.integer(&v); + return std::make_unique>(v); + } case BLOB_INT64: - { int64_t v; - t.integer(&v); - return std::make_unique>(v); - } + { int64_t v; + t.integer(&v); + return std::make_unique>(v); + } case BLOB_FLOAT32: return std::make_unique>(static_cast(t.as_float())); case BLOB_FLOAT64: return std::make_unique>(t.as_double()); case BLOB_TERM: - { PlRecordExternalCopy e(t); // declared here so that it stays in scope for return - return std::make_unique(e.data()); - } + { PlRecordExternalCopy e(t); // declared here so that it stays in scope for return + return std::make_unique(e.data()); + } default: assert(0); } @@ -615,7 +672,7 @@ call_merger(const dbref *ref, PlTermv av, std::string* new_value, try { PlQuery q(pred_call6, av); if ( q.next_solution() ) - { auto answer = get_slice(av[5], ref->type.value); + { const auto answer = get_slice(av[5], ref->type.value); new_value->assign(answer->data(), answer->size()); return true; } else @@ -624,7 +681,7 @@ call_merger(const dbref *ref, PlTermv av, std::string* new_value, } } catch(PlException &ex) { Log(logger, "%s", ex.as_string(PlEncoding::UTF8).c_str()); - return false; + throw; } } @@ -644,7 +701,7 @@ class PrologMergeOperator : public rocksdb::MergeOperator const std::deque& operand_list, std::string* new_value, rocksdb::Logger* logger) const override - { engine e; + { engine e_ctxt; PlTermv av(6); PlTerm_tail list(av[4]); PlTerm_var tmp; @@ -653,8 +710,8 @@ class PrologMergeOperator : public rocksdb::MergeOperator for (const auto& value : operand_list) { Plx_put_variable(tmp.C_); if ( !unify(tmp, value, ref->type.value) || - !list.append(tmp) ) - return false; + !list.append(tmp) ) + return false; } if ( !list.close() ) return false; @@ -674,7 +731,7 @@ class PrologMergeOperator : public rocksdb::MergeOperator const rocksdb::Slice& right_operand, std::string* new_value, rocksdb::Logger* logger) const override - { engine e; + { engine e_ctxt; PlTermv av(6); static const PlAtom ATOM_partial("partial"); @@ -698,8 +755,8 @@ template [[nodiscard]] static int cmp_number(const void *v1, const void *v2) -{ auto i1 = static_cast(v1); - auto i2 = static_cast(v2); +{ const auto i1 = static_cast(v1); + const auto i2 = static_cast(v2); return *i1 > *i2 ? 1 : *i1 < *i2 ? -1 : 0; } @@ -707,13 +764,13 @@ cmp_number(const void *v1, const void *v2) template static void sort_numbers(std::string *str) -{ auto s = str->data(); - auto len = str->length(); +{ const auto s = str->data(); + const auto len = str->length(); if ( len == 0 ) return; auto ip = reinterpret_cast(s); auto op = ip+1; - auto ep = reinterpret_cast(s+len); + const auto ep = reinterpret_cast(s+len); Number_t cv; qsort(s, len / sizeof ip, sizeof ip, cmp_number); @@ -1236,7 +1293,7 @@ PREDICATE(rocks_open_, 3) else if ( ATOM_alias == name ) { alias = opt[1].as_atom(); } else if ( ATOM_mode == name ) - { PlAtom a = opt[1].as_atom(); + { const auto a = opt[1].as_atom(); if ( ATOM_read_write == a ) read_only = false; else if ( ATOM_read_only == a ) @@ -1251,63 +1308,75 @@ PREDICATE(rocks_open_, 3) } if ( alias.not_null() ) - { PlAtom existing = rocks_get_alias_locked(alias); + { const auto existing = rocks_get_alias(alias); if ( existing.not_null() ) - { auto eref = symbol_dbref(existing); - if ( eref ) - return A2.unify_atom(existing); - } + throw PlPermissionError("alias", "rocksdb", PlTerm_atom(alias)); } - auto ref = new dbref(); + auto ref = std::make_unique(); ref->merger = merger; ref->builtin_merger = builtin_merger; ref->type.key = key_type; ref->type.value = value_type; - ref->name = alias; ref->pathname = PlAtom(fn); ref->pathname.register_ref(); + ref->name = alias; if ( ref->name.not_null() ) ref->name.register_ref(); - try - { if ( ref->merger.not_null() ) - options.merge_operator.reset(new PrologMergeOperator(ref)); - else if ( builtin_merger != MERGE_NONE ) - options.merge_operator.reset(new ListMergeOperator(ref)); - if ( read_only ) - ok_or_throw_fail(rocksdb::DB::OpenForReadOnly(options, fn, &ref->db)); - else - ok_or_throw_fail(rocksdb::DB::Open(options, fn, &ref->db)); - PlCheckFail(unify_rocks(A2, ref)); - } catch(...) // TODO - remove this by using unique_ptr or similar - { delete ref; - throw; + if ( ref->merger.not_null() ) + options.merge_operator.reset(new PrologMergeOperator(ref.get())); + else if ( builtin_merger != MERGE_NONE ) + options.merge_operator.reset(new ListMergeOperator(ref.get())); + ok_or_throw_fail(read_only + ? rocksdb::DB::OpenForReadOnly(options, fn, &ref->db) + : rocksdb::DB::Open(options, fn, &ref->db)); + + if ( ref->name.is_null() ) + { PlCheckFail(A2.unify_blob(&ref, sizeof ref, &rocks_blob)); + } else + { PlTerm_var tmp; + PlCheckFail(tmp.unify_blob(&ref, sizeof ref, &rocks_blob)); + rocks_add_alias(ref->name, tmp.as_atom()); + PlCheckFail(A2.unify_atom(ref->name)); } + + (void)ref.release(); // ref now owned by Prolog return true; } PREDICATE(rocks_close, 1) -{ dbref *ref = get_rocks(A1, false); +{ const auto ref = get_rocks(A1, false); if ( !ref ) return true; - auto db = ref->db; - ref->db = nullptr; + // The following code is a subset of dbref::~dbref(), and also can + // throw a Prolog error if ref->db->Close() fails. + if ( ref->name.not_null() ) { rocks_unalias(ref->name); - ref->name.reset(); + // ref->name is needed by write_rocks_ref_(), so don't: ref->name.unregister_ref() + } + + { auto db = ref->db; + ref->db = nullptr; + if ( db ) + ok_or_throw_fail(db->Close()); } - ref->pathname.unregister_ref(); - if ( ref->name.not_null() ) - ref->name.unregister_ref(); - delete db; return true; } +PREDICATE(rocks_alias_lookup, 2) +{ PlAtom a(rocks_get_alias(A1.as_atom())); + if ( a.not_null() ) + return A2.unify_atom(a); + return false; +} + + [[nodiscard]] static rocksdb::WriteOptions write_options(PlTerm options_term) @@ -1329,11 +1398,11 @@ write_options(PlTerm options_term) PREDICATE(rocks_put, 4) -{ dbref *ref = get_rocks(A1); - auto key = get_slice(A2, ref->type.key); +{ const auto ref = get_rocks(A1); + const auto key = get_slice(A2, ref->type.key); if ( ref->builtin_merger == MERGE_NONE ) - { auto value = get_slice(A3, ref->type.value); + { const auto value = get_slice(A3, ref->type.value); ok_or_throw_fail(ref->db->Put(write_options(A4), key->slice(), value->slice()), ref); } else { PlTerm_tail list(A3); @@ -1341,7 +1410,7 @@ PREDICATE(rocks_put, 4) std::string value; while ( list.next(tmp) ) - { auto s = get_slice(tmp, ref->type.value); + { const auto s = get_slice(tmp, ref->type.value); value += s->ToString(); } @@ -1355,12 +1424,12 @@ PREDICATE(rocks_put, 4) } PREDICATE(rocks_merge, 4) -{ dbref *ref = get_rocks(A1); +{ const auto ref = get_rocks(A1); if ( ref->merger.is_null() && ref->builtin_merger == MERGE_NONE ) throw PlPermissionError("merge", "rocksdb", A1); - auto key = get_slice(A2,ref->type.key); - auto value = get_slice(A3, ref->type.value); + const auto key = get_slice(A2,ref->type.key); + const auto value = get_slice(A3, ref->type.value); ok_or_throw_fail(ref->db->Merge(write_options(A4), key->slice(), value->slice()), ref); @@ -1386,17 +1455,17 @@ read_options(PlTerm options_term) } PREDICATE(rocks_get, 4) -{ dbref *ref = get_rocks(A1); +{ const auto ref = get_rocks(A1); std::string value; - auto key = get_slice(A2, ref->type.key); + const auto key = get_slice(A2, ref->type.key); ok_or_throw_fail(ref->db->Get(read_options(A4), key->slice(), &value), ref); return unify_value(A3, value, ref->builtin_merger, ref->type.value); } PREDICATE(rocks_delete, 3) -{ dbref *ref = get_rocks(A1); - auto key = get_slice(A2, ref->type.key); +{ const auto ref = get_rocks(A1); + const auto key = get_slice(A2, ref->type.key); ok_or_throw_fail(ref->db->Delete(write_options(A3), key->slice()), ref); return true; @@ -1422,32 +1491,31 @@ struct enum_state [[nodiscard]] static bool -unify_enum_key(PlTerm t, const enum_state *state) -{ if ( state->type == ENUM_PREFIX ) - { rocksdb::Slice k(state->it->key()); +unify_enum_key(PlTerm t, const enum_state& state) +{ if ( state.type == ENUM_PREFIX ) + { rocksdb::Slice k(state.it->key()); - // TODO: use std::string's compare - if ( k.size_ >= state->prefix.length() && - memcmp(k.data_, state->prefix.data(), state->prefix.length()) == 0 ) - { k.data_ += state->prefix.length(); - k.size_ -= state->prefix.length(); + if ( k.size_ >= state.prefix.length() && + memcmp(k.data_, state.prefix.data(), state.prefix.length()) == 0 ) + { k.data_ += state.prefix.length(); + k.size_ -= state.prefix.length(); - return unify(t, k, state->ref->type.key); + return unify(t, k, state.ref->type.key); } else return false; } else - { return unify(t, state->it->key(), state->ref->type.key); + { return unify(t, state.it->key(), state.ref->type.key); } } [[nodiscard]] static bool -enum_key_prefix(const enum_state *state) -{ if ( state->type == ENUM_PREFIX ) - { rocksdb::Slice k(state->it->key()); - return ( k.size_ >= state->prefix.length() && - memcmp(k.data_, state->prefix.data(), state->prefix.length()) == 0 ); +enum_key_prefix(const enum_state& state) +{ if ( state.type == ENUM_PREFIX ) + { rocksdb::Slice k(state.it->key()); + return ( k.size_ >= state.prefix.length() && + memcmp(k.data_, state.prefix.data(), state.prefix.length()) == 0 ); } else return true; } @@ -1467,10 +1535,10 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea state->ref->type.key == BLOB_BINARY) ) throw PlPermissionError("enum", "rocksdb", A1); - std::string prefix = A4.get_nchars(REP_UTF8|CVT_IN|CVT_EXCEPTION); + const std::string prefix = A4.get_nchars(REP_UTF8|CVT_IN|CVT_EXCEPTION); if ( type == ENUM_PREFIX ) - { state->prefix = prefix; + { state->prefix = prefix; } state->it.reset(state->ref->db->NewIterator(options)); state->it->Seek(prefix); @@ -1483,13 +1551,12 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea case PL_REDO: { PlFrame fr; for ( ; state->it->Valid(); state->it->Next() ) - { if ( unify_enum_key(A2, state.get()) && + { if ( unify_enum_key(A2, *state) && unify_value(A3, state->it->value(), state->ref->builtin_merger, state->ref->type.value) ) { state->it->Next(); - if ( state->it->Valid() && enum_key_prefix(state.get()) ) - { state.keep(); - PL_retry_address(state.get()); + if ( state->it->Valid() && enum_key_prefix(*state) ) + { PL_retry_address(state.keep()); } else { return true; } @@ -1507,18 +1574,22 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea return false; } + PREDICATE_NONDET(rocks_enum, 4) { return rocks_enum(PL_av, 3, ENUM_ALL, handle, read_options(A4)); } + PREDICATE_NONDET(rocks_enum_from, 5) { return rocks_enum(PL_av, 4, ENUM_FROM, handle, read_options(A5)); } + PREDICATE_NONDET(rocks_enum_prefix, 5) { return rocks_enum(PL_av, 4, ENUM_PREFIX, handle, read_options(A5)); } + static void batch_operation(const dbref *ref, rocksdb::WriteBatch &batch, PlTerm e) { PlAtom name(PlAtom::null); @@ -1529,11 +1600,11 @@ batch_operation(const dbref *ref, rocksdb::WriteBatch &batch, PlTerm e) PlCheckFail(e.name_arity(&name, &arity)); if ( ATOM_delete == name && arity == 1 ) - { auto key = get_slice(e[1], ref->type.key); + { const auto key = get_slice(e[1], ref->type.key); batch.Delete(key->slice()); } else if ( ATOM_put == name && arity == 2 ) - { auto key = get_slice(e[1], ref->type.key); - auto value = get_slice(e[2], ref->type.value); + { const auto key = get_slice(e[1], ref->type.key); + const auto value = get_slice(e[2], ref->type.value); batch.Put(key->slice(), value->slice()); } else { throw PlDomainError("rocks_batch_operation", e); @@ -1542,7 +1613,7 @@ batch_operation(const dbref *ref, rocksdb::WriteBatch &batch, PlTerm e) PREDICATE(rocks_batch, 3) -{ dbref *ref = get_rocks(A1); +{ const auto ref = get_rocks(A1); rocksdb::WriteBatch batch; PlTerm_tail tail(A2); PlTerm_var e; @@ -1556,12 +1627,12 @@ PREDICATE(rocks_batch, 3) } -static PlAtom ATOM_estimate_num_keys("estimate_num_keys"); - PREDICATE(rocks_property, 3) -{ dbref *ref = get_rocks(A1); +{ const auto ref = get_rocks(A1); + static PlAtom ATOM_estimate_num_keys("estimate_num_keys"); + + const auto prop = A2.as_atom(); - PlAtom prop = A2.as_atom(); if ( ATOM_estimate_num_keys == prop ) { uint64_t value; diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index a382322..acaac59 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -36,6 +36,7 @@ :- module(rocksdb, [ rocks_open/3, % +Directory, -RocksDB, +Options rocks_close/1, % +RocksDB + rocks_alias_lookup/2, % +Name, -RocksDB rocks_put/3, % +RocksDB, +Key, +Value rocks_put/4, % +RocksDB, +Key, +Value, +Options @@ -222,13 +223,16 @@ % % - alias(+Name) % Give the database a name instead of using an anonymous -% handle. A named database is not subject to GC and must -% be closed explicitly. When the database is opened, -% RocksDB unifies with Name. +% handle. A named database is not subject to GC and must be +% closed explicitly. When the database is opened, RocksDB +% unifies with Name (the underlying handle can obtained using +% rocks_alias_lookup2). + % - key(+Type) % - value(+Type) -% Define the type for the key and value. This must be -% consistent over multiple invocations. Defined types are: +% Define the type for the key and value. These must be +% consistent over multiple invocations. Default is `atom`. +% Defined types are: % - atom % Accepts an atom or string. Unifies the result with an % atom. Data is stored as a UTF-8 string in RocksDB. @@ -318,6 +322,23 @@ % existence error is raised (this behavior may change in % future). + +%! rocks_alias_lookup(+Name, -RocksDB) is semidet. +% +% Look up an alias Name (as specified in rocks_open/3 `alias` +% option and unify RocksDb with the underlying handle; fails if +% there is no open file with the alias Name. +% +% This predicate has two uses: +% - The other predicates have slightly faster performance when the +% RocksDB handle is used instead of the Name. +% - Some extra debugging information is available when the blob is printed. +% +% Note that `rocks_open(...,RocksDB,[alias(Name)])` unifies +% RocksDB with Name; if `alias(Name)` is not specified, RocksDB +% is unified with the underlying handle. + + %! rocks_put(+RocksDB, +Key, +Value) is det. %! rocks_put(+RocksDB, +Key, +Value, Options) is det. % diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index a7de286..b72f21e 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -372,6 +372,20 @@ % rocks_close('DB'). true. +test(dup, [error(permission_error(alias,rocksdb,'DB')), + setup(setup_db(Dir, DB, [alias('DB')])), + cleanup((cleanup_db(Dir, DB), + cleanup_db(Dir2, DB2)))]) :- + test_db2(Dir2), + rocks_open(Dir2, DB2, [alias('DB')]). +% As 1st dup test, but the rocks_close clears the alias entry: +test(dup, [setup(setup_db(Dir, DB, [alias('DB')])), + cleanup((cleanup_db(Dir, DB), + cleanup_db(Dir2, DB2)))]) :- + test_db2(Dir2), + rocks_close(DB), + rocks_open(Dir2, DB2, [alias('DB')]). + :- end_tests(alias). /******************************* @@ -379,6 +393,7 @@ *******************************/ test_db('/tmp/test_rocksdb'). +test_db2('/tmp/test_rocksdb2'). delete_db :- test_db(DB), @@ -401,5 +416,7 @@ % is an error in the test case and the database isn't closed, it will % cause spurious errors from all the subsequent tests. cleanup_db(Dir, RocksDB) :- - catch(ignore(rocks_close(RocksDB)), _, true), - delete_db(Dir). + catch((ignore(rocks_close(RocksDB)), + delete_db(Dir)), + _, true). + From f592d4927c6d402d4d456433fab70dd13c0de00e Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Mon, 5 Jun 2023 09:15:03 -0700 Subject: [PATCH 14/28] Merger keeps copy of some dbref fields instead of pointer to dbref - avoids needing to analyze lifetime of dbref pointer in rocks_open/3. --- cpp/rocksdb4pl.cpp | 75 ++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 2135474..fc363c0 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -93,6 +93,12 @@ static const char* merge_t_char[] = }; +struct dbref_type_kv +{ blob_type key; + blob_type value; +}; + + struct dbref { dbref() @@ -128,10 +134,7 @@ struct dbref PlAtom name; /* alias name (can be PlAtom::null) */ merger_t builtin_merger; /* C++ Merger */ PlRecord merger; /* merge option */ - struct - { blob_type key; - blob_type value; - } type; + dbref_type_kv type; /* key and value types */ }; @@ -150,7 +153,7 @@ static atom_t load_rocks(IOSTREAM *fd); static PL_blob_t rocks_blob = { .magic = PL_BLOB_MAGIC, - .flags = PL_BLOB_UNIQUE, // TODO: 0 or PL_BLOB_NOCOPY (see PrologMergeOperator()) + .flags = PL_BLOB_UNIQUE, // TODO: 0 .name = "rocksdb", .release = release_rocks_ref, .compare = nullptr, @@ -665,14 +668,14 @@ class engine [[nodiscard]] static bool -call_merger(const dbref *ref, PlTermv av, std::string* new_value, +call_merger(const dbref_type_kv& type, PlTermv av, std::string* new_value, rocksdb::Logger* logger) { static PlPredicate pred_call6(PlFunctor("call", 6), PlModule("system")); try { PlQuery q(pred_call6, av); if ( q.next_solution() ) - { const auto answer = get_slice(av[5], ref->type.value); + { const auto answer = get_slice(av[5], type.value); new_value->assign(answer->data(), answer->size()); return true; } else @@ -689,11 +692,18 @@ call_merger(const dbref *ref, PlTermv av, std::string* new_value, class PrologMergeOperator : public rocksdb::MergeOperator { private: - const dbref *ref; + dbref_type_kv type; + merger_t builtin_merger; + PlRecord merger; public: - explicit PrologMergeOperator(const dbref *reference) - : ref(reference) { } + explicit PrologMergeOperator(const dbref& ref) + : type(ref.type), builtin_merger(ref.builtin_merger), merger(ref.merger.duplicate()) + { } + + ~PrologMergeOperator() + { merger.erase(); + } virtual bool FullMerge(const rocksdb::Slice& key, @@ -709,18 +719,18 @@ class PrologMergeOperator : public rocksdb::MergeOperator for (const auto& value : operand_list) { Plx_put_variable(tmp.C_); - if ( !unify(tmp, value, ref->type.value) || + if ( !unify(tmp, value, type.value) || !list.append(tmp) ) return false; } if ( !list.close() ) return false; - if ( av[0].unify_term(ref->merger.term()) && + if ( av[0].unify_term(merger.term()) && av[1].unify_atom(ATOM_full) && - unify(av[2], key, ref->type.key) && - unify(av[3], existing_value, ref->type.value) ) - return call_merger(ref, av, new_value, logger); + unify(av[2], key, type.key) && + unify(av[3], existing_value, type.value) ) + return call_merger(type, av, new_value, logger); else return log_exception(logger); } @@ -735,12 +745,12 @@ class PrologMergeOperator : public rocksdb::MergeOperator PlTermv av(6); static const PlAtom ATOM_partial("partial"); - if ( av[0].unify_term(ref->merger.term()) && + if ( av[0].unify_term(merger.term()) && av[1].unify_atom(ATOM_partial) && - unify(av[2], key, ref->type.key) && - unify(av[3], left_operand, ref->type.value) && - unify(av[4], right_operand, ref->type.value) ) - return call_merger(ref, av, new_value, logger); + unify(av[2], key, type.key) && + unify(av[3], left_operand, type.value) && + unify(av[4], right_operand, type.value) ) + return call_merger(type, av, new_value, logger); else return log_exception(logger); } @@ -808,11 +818,18 @@ sort(std::string *str, blob_type type) class ListMergeOperator : public rocksdb::MergeOperator { private: - const dbref *ref; + dbref_type_kv type; + merger_t builtin_merger; + PlRecord merger; public: - explicit ListMergeOperator(const dbref *reference) - : ref(reference) { } + explicit ListMergeOperator(const dbref& ref) + : type(ref.type), builtin_merger(ref.builtin_merger), merger(ref.merger.duplicate()) + { } + + ~ListMergeOperator() + { merger.erase(); + } virtual bool FullMerge(const rocksdb::Slice& key, @@ -828,8 +845,8 @@ class ListMergeOperator : public rocksdb::MergeOperator { s += value; } - if ( ref->builtin_merger == MERGE_SET ) - sort(&s, ref->type.value); + if ( builtin_merger == MERGE_SET ) + sort(&s, type.value); *new_value = s; return true; } @@ -845,8 +862,8 @@ class ListMergeOperator : public rocksdb::MergeOperator std::string s(left_operand.ToString()); s += right_operand.ToString(); - if ( ref->builtin_merger == MERGE_SET ) - sort(&s, ref->type.value); + if ( builtin_merger == MERGE_SET ) + sort(&s, type.value); *new_value = s; return true; } @@ -1325,9 +1342,9 @@ PREDICATE(rocks_open_, 3) ref->name.register_ref(); if ( ref->merger.not_null() ) - options.merge_operator.reset(new PrologMergeOperator(ref.get())); + options.merge_operator.reset(new PrologMergeOperator(*ref)); else if ( builtin_merger != MERGE_NONE ) - options.merge_operator.reset(new ListMergeOperator(ref.get())); + options.merge_operator.reset(new ListMergeOperator(*ref)); ok_or_throw_fail(read_only ? rocksdb::DB::OpenForReadOnly(options, fn, &ref->db) : rocksdb::DB::Open(options, fn, &ref->db)); From 117aa166be1c3cd0e1a21262bf96c44fbfd33481 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Mon, 5 Jun 2023 15:45:27 -0700 Subject: [PATCH 15/28] Replace lazy filling in of lookup table entries by first-use PlAtom constructor --- cpp/rocksdb4pl.cpp | 590 ++++++++++++++++++++++----------------------- 1 file changed, 293 insertions(+), 297 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index fc363c0..9b89e64 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -918,76 +918,72 @@ get_blob_type(PlTerm t, blob_type *key_type, merger_t *m) } -typedef void (*ReadOptdefAction)(rocksdb::ReadOptions *options, PlTerm t); - -struct ReadOptdef -{ const char *name; - ReadOptdefAction action; - PlAtom atom; // Initially PlAtom::null; filled in as-needed by lookup -}; - -#define RD_ODEF [](rocksdb::ReadOptions *options, PlTerm arg) - -// TODO: move read_optdefs into read_options(), remove the PlAtom(PlAtom::null), -// replacing ReadOptdef::name with PlAtom ... C++ will initialize the -// "static" const structure on first use. -static ReadOptdef read_optdefs[] = -{ // "snapshot" const Snapshot* - // "iterate_lower_bound" const rocksdb::Slice* - // "iterate_upper_bound" const rocksdb::Slice* - { "readahead_size", RD_ODEF { - options->readahead_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "max_skippable_internal_keys", RD_ODEF { - options->max_skippable_internal_keys = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - // "read_tier" ReadTier - { "verify_checksums", RD_ODEF { - options->verify_checksums = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "fill_cache", RD_ODEF { - options->fill_cache = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "tailing", RD_ODEF { - options->tailing = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "managed" - not used any more - { "total_order_seek", RD_ODEF { - options->total_order_seek = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "auto_prefix_mode", RD_ODEF { - options->auto_prefix_mode = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "prefix_same_as_start", RD_ODEF { - options->prefix_same_as_start = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "pin_data", RD_ODEF { - options->pin_data = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "background_purge_on_iterator_cleanup", RD_ODEF { - options->background_purge_on_iterator_cleanup = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "ignore_range_deletions", RD_ODEF { - options->ignore_range_deletions = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "table_filter" std::function - // TODO: "iter_start_seqnum" removed from rocksdb/include/options.h? - // { "iter_start_seqnum", RD_ODEF { - // options->iter_start_seqnum = static_cast(arg); }, PlAtom(PlAtom::null) }, - // "timestamp" rocksdb::Slice* - // "iter_start_ts" rocksdb::Slice* - { - "deadline", RD_ODEF { - options->deadline = static_cast(arg.as_int64_t()); }, PlAtom(PlAtom::null) }, - { "io_timeout", RD_ODEF { - options->io_timeout = static_cast(arg.as_int64_t()); }, PlAtom(PlAtom::null) }, - { "value_size_soft_limit", RD_ODEF { - options->value_size_soft_limit = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { nullptr, nullptr, PlAtom(PlAtom::null) } -}; - - -// TODO: consolidate lookup_read_optdef_and_apply, -// lookup_write_optdef_and_apply, lookup_optdef_and_apply; -// possibly using STL map (although that doesn't support -// lazy filling in of the atoms in the lookup table) static void lookup_read_optdef_and_apply(rocksdb::ReadOptions *options, - ReadOptdef opt_defs[], PlAtom name, PlTerm opt) -{ for ( auto def=opt_defs; def->name; def++ ) - { if ( def->atom.is_null() ) // lazily fill in atoms in lookup table - def->atom = PlAtom(def->name); - if ( def->atom == name ) +{ + typedef void (*ReadOptdefAction)(rocksdb::ReadOptions *options, PlTerm t); + + struct ReadOptdef + { const PlAtom name; + const ReadOptdefAction action; + + ReadOptdef(const char *name_, ReadOptdefAction action_) + : name(name_), action(action_) { } + ReadOptdef(PlAtom atom_, ReadOptdefAction action_) + : name(atom_), action(action_) { } + }; + + #define RD_ODEF [](rocksdb::ReadOptions *o, PlTerm arg) + + static const ReadOptdef read_optdefs[] = + { // "snapshot" const Snapshot* + // "iterate_lower_bound" const rocksdb::Slice* + // "iterate_upper_bound" const rocksdb::Slice* + ReadOptdef("readahead_size", RD_ODEF { + o->readahead_size = arg.as_size_t(); } ), + ReadOptdef("max_skippable_internal_keys", RD_ODEF { + o->max_skippable_internal_keys = arg.as_uint64_t(); } ), + // "read_tier" ReadTier + ReadOptdef("verify_checksums", RD_ODEF { + o->verify_checksums = arg.as_bool(); } ), + ReadOptdef("fill_cache", RD_ODEF { + o->fill_cache = arg.as_bool(); } ), + ReadOptdef("tailing", RD_ODEF { + o->tailing = arg.as_bool(); } ), + // "managed" - not used any more + ReadOptdef("total_order_seek", RD_ODEF { + o->total_order_seek = arg.as_bool(); } ), + ReadOptdef("auto_prefix_mode", RD_ODEF { + o->auto_prefix_mode = arg.as_bool(); } ), + ReadOptdef("prefix_same_as_start", RD_ODEF { + o->prefix_same_as_start = arg.as_bool(); } ), + ReadOptdef("pin_data", RD_ODEF { + o->pin_data = arg.as_bool(); } ), + ReadOptdef("background_purge_on_iterator_cleanup", RD_ODEF { + o->background_purge_on_iterator_cleanup = arg.as_bool(); } ), + ReadOptdef("ignore_range_deletions", RD_ODEF { + o->ignore_range_deletions = arg.as_bool(); } ), + // "table_filter" std::function + // TODO: "iter_start_seqnum" removed from rocksdb/include/options.h? + // { "iter_start_seqnum", RD_ODEF { + // o->iter_start_seqnum = static_cast(arg); } }, + // "timestamp" rocksdb::Slice* + // "iter_start_ts" rocksdb::Slice* + ReadOptdef("deadline", RD_ODEF { + o->deadline = static_cast(arg.as_int64_t()); } ), + ReadOptdef("io_timeout", RD_ODEF { + o->io_timeout = static_cast(arg.as_int64_t()); } ), + ReadOptdef("value_size_soft_limit", RD_ODEF { + o->value_size_soft_limit = arg.as_uint64_t(); } ), + + ReadOptdef(PlAtom(PlAtom::null), nullptr) + }; + + #undef RD_ODEF + + for ( auto def=read_optdefs; def->name.not_null(); def++ ) + { if ( def->name == name ) { def->action(options, opt[1]); return; } @@ -996,46 +992,46 @@ lookup_read_optdef_and_apply(rocksdb::ReadOptions *options, } -typedef void (*WriteOptdefAction)(rocksdb::WriteOptions *options, PlTerm t); - -struct WriteOptdef -{ const char *name; - WriteOptdefAction action; - PlAtom atom; // Initially PlAtom::null; filled in as-needed by lookup -}; - -#define WR_ODEF [](rocksdb::WriteOptions *options, PlTerm arg) - -// TODO: move write_optdefs into write_options(), remove the PlAtom(PlAtom::null), -// replacing WriteOptdef::name with PlAtom ... C++ will initialize the -// "static" const structure on first use. -static WriteOptdef write_optdefs[] = -{ { "sync", WR_ODEF { - options->sync = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "disableWAL", WR_ODEF { - options->disableWAL = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "ignore_missing_column_families", WR_ODEF { - options->ignore_missing_column_families = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "no_slowdown", WR_ODEF { - options->no_slowdown = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "low_pri", WR_ODEF { - options->low_pri = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "memtable_insert_hint_per_batch", WR_ODEF { - options->memtable_insert_hint_per_batch = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "timestamp" rocksdb::Slice* - - { nullptr, nullptr, PlAtom(PlAtom::null) } -}; - - static void lookup_write_optdef_and_apply(rocksdb::WriteOptions *options, - WriteOptdef opt_defs[], PlAtom name, PlTerm opt) -{ for ( auto def=opt_defs; def->name; def++ ) - { if ( def->atom.is_null() ) // lazilly fill in atoms in lookup table - def->atom = PlAtom(def->name); - if ( def->atom == name ) +{ + typedef void (*WriteOptdefAction)(rocksdb::WriteOptions *o, PlTerm t); + + struct WriteOptdef + { const PlAtom name; + const WriteOptdefAction action; + + WriteOptdef(const char *name_, WriteOptdefAction action_) + : name(name_), action(action_) { } + WriteOptdef(PlAtom atom_, WriteOptdefAction action_) + : name(atom_), action(action_) { } + }; + + #define WR_ODEF [](rocksdb::WriteOptions *o, PlTerm arg) + + static const WriteOptdef optdefs[] = + { WriteOptdef("sync", WR_ODEF { + o->sync = arg.as_bool(); } ), + WriteOptdef("disableWAL", WR_ODEF { + o->disableWAL = arg.as_bool(); } ), + WriteOptdef("ignore_missing_column_families", WR_ODEF { + o->ignore_missing_column_families = arg.as_bool(); } ), + WriteOptdef("no_slowdown", WR_ODEF { + o->no_slowdown = arg.as_bool(); } ), + WriteOptdef("low_pri", WR_ODEF { + o->low_pri = arg.as_bool(); } ), + WriteOptdef("memtable_insert_hint_per_batch", WR_ODEF { + o->memtable_insert_hint_per_batch = arg.as_bool(); } ), + // "timestamp" rocksdb::Slice* + + WriteOptdef(PlAtom(PlAtom::null), nullptr) + }; + + #undef WR_ODEF + + for ( auto def=optdefs; def->name.not_null(); def++ ) + { if ( def->name == name ) { def->action(options, opt[1]); return; } @@ -1066,204 +1062,204 @@ options_set_InfoLogLevel(rocksdb::Options *options, PlTerm arg) } -typedef void (*OptdefAction)(rocksdb::Options *options, PlTerm t); +static void +lookup_open_optdef_and_apply(rocksdb::Options *options, + PlAtom name, PlTerm opt) +{ + typedef void (*OpenOptdefAction)(rocksdb::Options *options, PlTerm t); -struct Optdef -{ const char *name; - OptdefAction action; - PlAtom atom; // Initially PlAtom::null; filled in as-needed by lookup -}; + struct OpenOptdef + { const PlAtom name; + const OpenOptdefAction action; -#define ODEF [](rocksdb::Options *options, PlTerm arg) + OpenOptdef(const char *name_, OpenOptdefAction action_) + : name(name_), action(action_) { } + OpenOptdef(PlAtom atom_, OpenOptdefAction action_) + : name(atom_), action(action_) { } + }; -static Optdef optdefs[] = -{ { "prepare_for_bulk_load", ODEF { if ( arg.as_bool() ) options->PrepareForBulkLoad(); }, PlAtom(PlAtom::null) }, // TODO: what to do with false? - { "optimize_for_small_db", ODEF { if ( arg.as_bool() ) options->OptimizeForSmallDb(); }, PlAtom(PlAtom::null) }, // TODO: what to do with false? - there's no DontOptimizeForSmallDb() + #define ODEF [](rocksdb::Options *o, PlTerm arg) + + static const OpenOptdef open_optdefs[] = + { OpenOptdef("prepare_for_bulk_load", ODEF { if ( arg.as_bool() ) o->PrepareForBulkLoad(); } ), // TODO: what to do with false? + OpenOptdef("optimize_for_small_db", ODEF { if ( arg.as_bool() ) o->OptimizeForSmallDb(); } ), // TODO: what to do with false? - there's no DontOptimizeForSmallDb() #ifndef ROCKSDB_LITE - { "increase_parallelism", ODEF { if ( arg.as_bool() ) options->IncreaseParallelism(); }, PlAtom(PlAtom::null) }, + OpenOptdef("increase_parallelism", ODEF { if ( arg.as_bool() ) o->IncreaseParallelism(); } ), #endif - { "create_if_missing", ODEF { - options->create_if_missing = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "create_missing_column_families", ODEF { - options->create_missing_column_families = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "error_if_exists", ODEF { - options->error_if_exists = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "paranoid_checks", ODEF { - options->paranoid_checks = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "track_and_verify_wals_in_manifest", ODEF { - options->track_and_verify_wals_in_manifest = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "env" Env::Default - // "rate_limiter" - shared_ptr - // "sst_file_manager" - shared_ptr - // "info_log" - shared_ptr - see comment in ../README.md - { "info_log_level", options_set_InfoLogLevel, PlAtom(PlAtom::null) }, - { "max_open_files", ODEF { - options->max_open_files = arg.as_int(); }, PlAtom(PlAtom::null) }, - { "max_file_opening_threads", ODEF { - options-> max_file_opening_threads = arg.as_int(); }, PlAtom(PlAtom::null) }, - { "max_total_wal_size", ODEF { - options->max_total_wal_size = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "statistics", ODEF { - options->statistics = arg.as_bool() ? rocksdb::CreateDBStatistics() : nullptr; }, PlAtom(PlAtom::null) }, - { "use_fsync", ODEF { - options->use_fsync = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "db_paths" - vector - { "db_log_dir", ODEF { - options->db_log_dir = arg.as_string(PlEncoding::Locale); }, PlAtom(PlAtom::null) }, - { "wal_dir", ODEF { - options->wal_dir = arg.as_string(PlEncoding::Locale); }, PlAtom(PlAtom::null) }, - { "delete_obsolete_files_period_micros", ODEF { - options->delete_obsolete_files_period_micros = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "max_background_jobs", ODEF { - options->max_background_jobs = arg.as_int(); }, PlAtom(PlAtom::null) }, - // "base_background_compactions" is obsolete - // "max_background_compactions" is obsolete - { "max_subcompactions", ODEF { - options->max_subcompactions = arg.as_uint32_t(); }, PlAtom(PlAtom::null) }, - // "max_background_flushes" is obsolete - { "max_log_file_size", ODEF { - options->max_log_file_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "log_file_time_to_roll", ODEF { - options->log_file_time_to_roll = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "keep_log_file_num", ODEF { - options->keep_log_file_num = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "recycle_log_file_num", ODEF { - options->recycle_log_file_num = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "max_manifest_file_size", ODEF { - options->max_manifest_file_size = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "table_cache_numshardbits", ODEF { - options->table_cache_numshardbits = arg.as_int(); }, PlAtom(PlAtom::null) }, - { "wal_ttl_seconds", ODEF { - options->WAL_ttl_seconds = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "wal_size_limit_mb", ODEF { - options->WAL_size_limit_MB = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "manifest_preallocation_size", ODEF { - options->manifest_preallocation_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "allow_mmap_reads", ODEF { - options->allow_mmap_reads = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "allow_mmap_writes", ODEF { - options->allow_mmap_writes = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "use_direct_reads", ODEF { - options->use_direct_reads = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "use_direct_io_for_flush_and_compaction", ODEF { - options->use_direct_io_for_flush_and_compaction = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "allow_fallocate", ODEF { - options->allow_fallocate = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "is_fd_close_on_exec", ODEF { - options->is_fd_close_on_exec = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "skip_log_error_on_recovery" is obsolete - { "stats_dump_period_sec", ODEF { - options->stats_dump_period_sec = arg.as_uint32_t(); }, PlAtom(PlAtom::null) }, // TODO: match: unsigned int stats_dump_period_sec - { "stats_persist_period_sec", ODEF { - options->stats_persist_period_sec = arg.as_uint32_t(); }, PlAtom(PlAtom::null) }, // TODO: match: unsigned int stats_persist_period_sec - { "persist_stats_to_disk", ODEF { - options->persist_stats_to_disk = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "stats_history_buffer_size", ODEF { - options->stats_history_buffer_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "advise_random_on_open", ODEF { - options->advise_random_on_open = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "db_write_buffer_size", ODEF { - options->db_write_buffer_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - // "write_buffer_manager" - shared_ptr - // "access_hint_on_compaction_start" - enum AccessHint - // TODO: "new_table_reader_for_compaction_inputs" removed from rocksdb/include/options.h? - // { "new_table_reader_for_compaction_inputs", ODEF { -// options->new_table_reader_for_compaction_inputs = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "compaction_readahead_size", ODEF { - options->compaction_readahead_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "random_access_max_buffer_size", ODEF { - options->random_access_max_buffer_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "writable_file_max_buffer_size", ODEF { - options->writable_file_max_buffer_size = arg.as_size_t(); }, PlAtom(PlAtom::null) }, - { "use_adaptive_mutex", ODEF { - options->use_adaptive_mutex = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "bytes_per_sync", ODEF { - options->bytes_per_sync = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "wal_bytes_per_sync", ODEF { - options->wal_bytes_per_sync = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "strict_bytes_per_sync", ODEF { - options->strict_bytes_per_sync = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "listeners" - vector> - { "enable_thread_tracking", ODEF { - options->enable_thread_tracking = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "delayed_write_rate", ODEF { - options->delayed_write_rate = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "enable_pipelined_write", ODEF { - options->enable_pipelined_write = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "unordered_write", ODEF { - options->unordered_write = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "allow_concurrent_memtable_write", ODEF { - options->allow_concurrent_memtable_write = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "enable_write_thread_adaptive_yield", ODEF { - options->enable_write_thread_adaptive_yield = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "max_write_batch_group_size_bytes", ODEF { - options->max_write_batch_group_size_bytes = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "write_thread_max_yield_usec", ODEF { - options->write_thread_max_yield_usec = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "write_thread_slow_yield_usec", ODEF { - options->write_thread_slow_yield_usec = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "skip_stats_update_on_db_open", ODEF { - options->skip_stats_update_on_db_open = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "skip_checking_sst_file_sizes_on_db_open", ODEF { - options->skip_checking_sst_file_sizes_on_db_open = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "wal_recovery_mode" - enum WALRecoveryMode - { "allow_2pc", ODEF { - options->allow_2pc = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "row_cache" - shared_ptr - // "wal_filter" - WalFilter* - { "fail_ifoptions_file_error", ODEF { - options->fail_if_options_file_error = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "dump_malloc_stats", ODEF { - options->dump_malloc_stats = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "avoid_flush_during_recovery", ODEF { - options->avoid_flush_during_recovery = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "avoid_flush_during_shutdown", ODEF { - options->avoid_flush_during_shutdown = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "allow_ingest_behind", ODEF { - options->allow_ingest_behind = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // TODO: "preserve_deletes" removed from rocksdb/include/options.h? - // { "preserve_deletes", ODEF { -// options->preserve_deletes = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "two_write_queues", ODEF { - options->two_write_queues = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "manual_wal_flush", ODEF { - options->manual_wal_flush = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "atomic_flush", ODEF { - options->atomic_flush = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "avoid_unnecessary_blocking_io", ODEF { - options->avoid_unnecessary_blocking_io = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "write_dbid_to_manifest", ODEF { - options->write_dbid_to_manifest = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "log_readahead_size", ODEF { - options->write_dbid_to_manifest = arg.as_bool(); }, PlAtom(PlAtom::null) }, - // "file_checksum_gen_factory" - std::shared_ptr - { "best_efforts_recovery", ODEF { - options->best_efforts_recovery = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "max_bgerror_resume_count", ODEF { - options->max_bgerror_resume_count = arg.as_int(); }, PlAtom(PlAtom::null) }, - { "bgerror_resume_retry_interval", ODEF { - options->bgerror_resume_retry_interval = arg.as_uint64_t(); }, PlAtom(PlAtom::null) }, - { "allow_data_in_errors", ODEF { - options->allow_data_in_errors = arg.as_bool(); }, PlAtom(PlAtom::null) }, - { "db_host_id", ODEF { - options->db_host_id = arg.as_string(PlEncoding::Locale); }, PlAtom(PlAtom::null) }, - // "checksum_handoff_file_types" - FileTypeSet - - { nullptr, nullptr, PlAtom(PlAtom::null) } + OpenOptdef("create_if_missing", ODEF { + o->create_if_missing = arg.as_bool(); } ), + OpenOptdef("create_missing_column_families", ODEF { + o->create_missing_column_families = arg.as_bool(); } ), + OpenOptdef("error_if_exists", ODEF { + o->error_if_exists = arg.as_bool(); } ), + OpenOptdef("paranoid_checks", ODEF { + o->paranoid_checks = arg.as_bool(); } ), + OpenOptdef("track_and_verify_wals_in_manifest", ODEF { + o->track_and_verify_wals_in_manifest = arg.as_bool(); } ), + // "env" Env::Default + // "rate_limiter" - shared_ptr + // "sst_file_manager" - shared_ptr + // "info_log" - shared_ptr - see comment in ../README.md + OpenOptdef("info_log_level", options_set_InfoLogLevel ), + OpenOptdef("max_open_files", ODEF { + o->max_open_files = arg.as_int(); } ), + OpenOptdef("max_file_opening_threads", ODEF { + o-> max_file_opening_threads = arg.as_int(); } ), + OpenOptdef("max_total_wal_size", ODEF { + o->max_total_wal_size = arg.as_uint64_t(); } ), + OpenOptdef("statistics", ODEF { + o->statistics = arg.as_bool() ? rocksdb::CreateDBStatistics() : nullptr; } ), + OpenOptdef("use_fsync", ODEF { + o->use_fsync = arg.as_bool(); } ), + // "db_paths" - vector + OpenOptdef("db_log_dir", ODEF { + o->db_log_dir = arg.as_string(PlEncoding::Locale); } ), + OpenOptdef("wal_dir", ODEF { + o->wal_dir = arg.as_string(PlEncoding::Locale); } ), + OpenOptdef("delete_obsolete_files_period_micros", ODEF { + o->delete_obsolete_files_period_micros = arg.as_uint64_t(); } ), + OpenOptdef("max_background_jobs", ODEF { + o->max_background_jobs = arg.as_int(); } ), + // "base_background_compactions" is obsolete + // "max_background_compactions" is obsolete + OpenOptdef("max_subcompactions", ODEF { + o->max_subcompactions = arg.as_uint32_t(); } ), + // "max_background_flushes" is obsolete + OpenOptdef("max_log_file_size", ODEF { + o->max_log_file_size = arg.as_size_t(); } ), + OpenOptdef("log_file_time_to_roll", ODEF { + o->log_file_time_to_roll = arg.as_size_t(); } ), + OpenOptdef("keep_log_file_num", ODEF { + o->keep_log_file_num = arg.as_size_t(); } ), + OpenOptdef("recycle_log_file_num", ODEF { + o->recycle_log_file_num = arg.as_size_t(); } ), + OpenOptdef("max_manifest_file_size", ODEF { + o->max_manifest_file_size = arg.as_uint64_t(); } ), + OpenOptdef("table_cache_numshardbits", ODEF { + o->table_cache_numshardbits = arg.as_int(); } ), + OpenOptdef("wal_ttl_seconds", ODEF { + o->WAL_ttl_seconds = arg.as_uint64_t(); } ), + OpenOptdef("wal_size_limit_mb", ODEF { + o->WAL_size_limit_MB = arg.as_uint64_t(); } ), + OpenOptdef("manifest_preallocation_size", ODEF { + o->manifest_preallocation_size = arg.as_size_t(); } ), + OpenOptdef("allow_mmap_reads", ODEF { + o->allow_mmap_reads = arg.as_bool(); } ), + OpenOptdef("allow_mmap_writes", ODEF { + o->allow_mmap_writes = arg.as_bool(); } ), + OpenOptdef("use_direct_reads", ODEF { + o->use_direct_reads = arg.as_bool(); } ), + OpenOptdef("use_direct_io_for_flush_and_compaction", ODEF { + o->use_direct_io_for_flush_and_compaction = arg.as_bool(); } ), + OpenOptdef("allow_fallocate", ODEF { + o->allow_fallocate = arg.as_bool(); } ), + OpenOptdef("is_fd_close_on_exec", ODEF { + o->is_fd_close_on_exec = arg.as_bool(); } ), + // "skip_log_error_on_recovery" is obsolete + OpenOptdef("stats_dump_period_sec", ODEF { + o->stats_dump_period_sec = arg.as_uint32_t(); } ), // TODO: match: unsigned int stats_dump_period_sec + OpenOptdef("stats_persist_period_sec", ODEF { + o->stats_persist_period_sec = arg.as_uint32_t(); } ), // TODO: match: unsigned int stats_persist_period_sec + OpenOptdef("persist_stats_to_disk", ODEF { + o->persist_stats_to_disk = arg.as_bool(); } ), + OpenOptdef("stats_history_buffer_size", ODEF { + o->stats_history_buffer_size = arg.as_size_t(); } ), + OpenOptdef("advise_random_on_open", ODEF { + o->advise_random_on_open = arg.as_bool(); } ), + OpenOptdef("db_write_buffer_size", ODEF { + o->db_write_buffer_size = arg.as_size_t(); } ), + // "write_buffer_manager" - shared_ptr + // "access_hint_on_compaction_start" - enum AccessHint + // TODO: "new_table_reader_for_compaction_inputs" removed from rocksdb/include/options.h? + // OpenOptdef("new_table_reader_for_compaction_inputs", ODEF { + // o->new_table_reader_for_compaction_inputs = arg.as_bool(); } ), + OpenOptdef("compaction_readahead_size", ODEF { + o->compaction_readahead_size = arg.as_size_t(); } ), + OpenOptdef("random_access_max_buffer_size", ODEF { + o->random_access_max_buffer_size = arg.as_size_t(); } ), + OpenOptdef("writable_file_max_buffer_size", ODEF { + o->writable_file_max_buffer_size = arg.as_size_t(); } ), + OpenOptdef("use_adaptive_mutex", ODEF { + o->use_adaptive_mutex = arg.as_bool(); } ), + OpenOptdef("bytes_per_sync", ODEF { + o->bytes_per_sync = arg.as_uint64_t(); } ), + OpenOptdef("wal_bytes_per_sync", ODEF { + o->wal_bytes_per_sync = arg.as_uint64_t(); } ), + OpenOptdef("strict_bytes_per_sync", ODEF { + o->strict_bytes_per_sync = arg.as_bool(); } ), + // "listeners" - vector> + OpenOptdef("enable_thread_tracking", ODEF { + o->enable_thread_tracking = arg.as_bool(); } ), + OpenOptdef("delayed_write_rate", ODEF { + o->delayed_write_rate = arg.as_uint64_t(); } ), + OpenOptdef("enable_pipelined_write", ODEF { + o->enable_pipelined_write = arg.as_bool(); } ), + OpenOptdef("unordered_write", ODEF { + o->unordered_write = arg.as_bool(); } ), + OpenOptdef("allow_concurrent_memtable_write", ODEF { + o->allow_concurrent_memtable_write = arg.as_bool(); } ), + OpenOptdef("enable_write_thread_adaptive_yield", ODEF { + o->enable_write_thread_adaptive_yield = arg.as_bool(); } ), + OpenOptdef("max_write_batch_group_size_bytes", ODEF { + o->max_write_batch_group_size_bytes = arg.as_uint64_t(); } ), + OpenOptdef("write_thread_max_yield_usec", ODEF { + o->write_thread_max_yield_usec = arg.as_uint64_t(); } ), + OpenOptdef("write_thread_slow_yield_usec", ODEF { + o->write_thread_slow_yield_usec = arg.as_uint64_t(); } ), + OpenOptdef("skip_stats_update_on_db_open", ODEF { + o->skip_stats_update_on_db_open = arg.as_bool(); } ), + OpenOptdef("skip_checking_sst_file_sizes_on_db_open", ODEF { + o->skip_checking_sst_file_sizes_on_db_open = arg.as_bool(); } ), + // "wal_recovery_mode" - enum WALRecoveryMode + OpenOptdef("allow_2pc", ODEF { + o->allow_2pc = arg.as_bool(); } ), + // "row_cache" - shared_ptr + // "wal_filter" - WalFilter* + OpenOptdef("fail_ifoptions_file_error", ODEF { + o->fail_if_options_file_error = arg.as_bool(); } ), + OpenOptdef("dump_malloc_stats", ODEF { + o->dump_malloc_stats = arg.as_bool(); } ), + OpenOptdef("avoid_flush_during_recovery", ODEF { + o->avoid_flush_during_recovery = arg.as_bool(); } ), + OpenOptdef("avoid_flush_during_shutdown", ODEF { + o->avoid_flush_during_shutdown = arg.as_bool(); } ), + OpenOptdef("allow_ingest_behind", ODEF { + o->allow_ingest_behind = arg.as_bool(); } ), + // TODO: "preserve_deletes" removed from rocksdb/include/options.h? + // OpenOptdef("preserve_deletes", ODEF { + // o->preserve_deletes = arg.as_bool(); } ), + OpenOptdef("two_write_queues", ODEF { + o->two_write_queues = arg.as_bool(); } ), + OpenOptdef("manual_wal_flush", ODEF { + o->manual_wal_flush = arg.as_bool(); } ), + OpenOptdef("atomic_flush", ODEF { + o->atomic_flush = arg.as_bool(); } ), + OpenOptdef("avoid_unnecessary_blocking_io", ODEF { + o->avoid_unnecessary_blocking_io = arg.as_bool(); } ), + OpenOptdef("write_dbid_to_manifest", ODEF { + o->write_dbid_to_manifest = arg.as_bool(); } ), + OpenOptdef("log_readahead_size", ODEF { + o->write_dbid_to_manifest = arg.as_bool(); } ), + // "file_checksum_gen_factory" - std::shared_ptr + OpenOptdef("best_efforts_recovery", ODEF { + o->best_efforts_recovery = arg.as_bool(); } ), + OpenOptdef("max_bgerror_resume_count", ODEF { + o->max_bgerror_resume_count = arg.as_int(); } ), + OpenOptdef("bgerror_resume_retry_interval", ODEF { + o->bgerror_resume_retry_interval = arg.as_uint64_t(); } ), + OpenOptdef("allow_data_in_errors", ODEF { + o->allow_data_in_errors = arg.as_bool(); } ), + OpenOptdef("db_host_id", ODEF { + o->db_host_id = arg.as_string(PlEncoding::Locale); } ), + // "checksum_handoff_file_types" - FileTypeSet + + { PlAtom(PlAtom::null), nullptr } }; -// TODO: move optdefs into options(), remove the PlAtom(PlAtom::null), -// replacing Optdef::name with PlAtom ... C++ will initialize the -// "static" const structure on first use. +#undef ODEF -static void -lookup_optdef_and_apply(rocksdb::Options *options, - Optdef opt_defs[], - PlAtom name, PlTerm opt) -{ for ( auto def=opt_defs; def->name; def++ ) - { if ( def->atom.is_null() ) // lazilly fill in atoms in lookup table - def->atom = PlAtom(def->name); - if ( def->atom == name ) + for ( auto def=open_optdefs; def->name.not_null(); def++ ) + { if ( def->name == name ) { def->action(options, opt[1]); return; } @@ -1318,7 +1314,7 @@ PREDICATE(rocks_open_, 3) else throw PlDomainError("mode_option", opt[1]); } else - { lookup_optdef_and_apply(&options, optdefs, name, opt); + { lookup_open_optdef_and_apply(&options, name, opt); } } else throw PlTypeError("option", opt); @@ -1406,7 +1402,7 @@ write_options(PlTerm options_term) PlCheckFail(opt.name_arity(&name, &arity)); if ( arity == 1 ) - lookup_write_optdef_and_apply(&options, write_optdefs, name, opt); + lookup_write_optdef_and_apply(&options, name, opt); else throw PlTypeError("option", opt); } @@ -1464,7 +1460,7 @@ read_options(PlTerm options_term) size_t arity; PlCheckFail(opt.name_arity(&name, &arity)); if ( arity == 1 ) - lookup_read_optdef_and_apply(&options, read_optdefs, name, opt); + lookup_read_optdef_and_apply(&options, name, opt); else throw PlTypeError("option", opt); } From 81bf8e473eda68e1ced2e9fb96844820fdf864ac Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Tue, 6 Jun 2023 09:49:31 -0700 Subject: [PATCH 16/28] Preparing for C++ blob - Do all blob casts through cast_dbref_blob() - PL_BLOB_NOCOPY and no default copy constructors Changed some const pointers to const references Standadized some parameter names --- cpp/rocksdb4pl.cpp | 346 ++++++++++++++++++++++++--------------------- 1 file changed, 187 insertions(+), 159 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 9b89e64..702390b 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -99,6 +99,9 @@ struct dbref_type_kv }; +#define DBREF_MAGIC 0xDB00DB01 + + struct dbref { dbref() @@ -112,6 +115,10 @@ struct dbref .value = BLOB_ATOM}) { } + dbref(const dbref&) = delete; + dbref(dbref&&) = delete; + dbref& operator =(const dbref&) = delete; + ~dbref() { // See also predicate rocks_close/1 if ( name.not_null() ) @@ -128,6 +135,7 @@ struct dbref } } + unsigned magic = DBREF_MAGIC; rocksdb::DB *db; /* DB handle */ PlAtom pathname; /* DB's absolute file name (for debugging) */ PlAtom symbol; /* associated symbol (used for error terms) */ @@ -138,22 +146,22 @@ struct dbref }; -static void acquire_rocks_ref_(PlAtom eref); +static void acquire_rocks_ref_(PlAtom aref); static bool release_rocks_ref_(PlAtom aref); -static bool write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags); -static bool write_rocks_ref_(IOSTREAM *s, const dbref *reff, int flags); +static bool write_rocks_ref_(IOSTREAM *s, PlAtom aref, int flags); +static bool write_rocks_ref_(IOSTREAM *s, const dbref& ref, int flags); static bool save_rocks_(PlAtom aref, IOSTREAM *fd); static PlAtom load_rocks_(IOSTREAM *fd); static void acquire_rocks_ref(atom_t symbol); static int release_rocks_ref(atom_t aref); -static int write_rocks_ref(IOSTREAM *s, atom_t eref, int flags); +static int write_rocks_ref(IOSTREAM *s, atom_t aref, int flags); static int save_rocks(atom_t aref, IOSTREAM *fd); static atom_t load_rocks(IOSTREAM *fd); static PL_blob_t rocks_blob = { .magic = PL_BLOB_MAGIC, - .flags = PL_BLOB_UNIQUE, // TODO: 0 + .flags = PL_BLOB_NOCOPY, .name = "rocksdb", .release = release_rocks_ref, .compare = nullptr, @@ -234,10 +242,31 @@ rocks_unalias(PlAtom name) * SYMBOL REFERENCES * *******************************/ +[[nodiscard]] +static dbref * +cast_dbref_blob(PlAtom aref) +{ size_t len; + PL_blob_t *type; + auto ref = static_cast(aref.blob_data(&len, &type)); + if ( ref && type == &rocks_blob) + { assert(len == sizeof *ref && ref->magic == DBREF_MAGIC); + return ref; + } + return nullptr; +} + +[[nodiscard]] +static dbref * +cast_dbref_blob_check(PlAtom aref) +{ auto ref = cast_dbref_blob(aref); + assert(ref); + return ref; +} + static void -acquire_rocks_ref_(PlAtom eref) -{ auto ref = *static_cast(eref.blob_data(nullptr, nullptr)); - ref->symbol = eref; +acquire_rocks_ref_(PlAtom aref) +{ auto ref = cast_dbref_blob_check(aref); + ref->symbol = aref; } static void @@ -248,22 +277,22 @@ acquire_rocks_ref(atom_t symbol) [[nodiscard]] static bool -write_rocks_ref_(IOSTREAM *s, const dbref *ref, int flags) -{ Sfprintf(s, "(%p", ref); - if ( ref->pathname.not_null() ) - Sfprintf(s, ",path=%Ws", ref->pathname.as_wstring().c_str()); - if ( ref->name.not_null() ) - Sfprintf(s, ",alias=%Ws", ref->name.as_wstring().c_str()); - if (ref->builtin_merger != MERGE_NONE) - Sfprintf(s, ",builtin_merger=%s", merge_t_char[ref->builtin_merger]); - if ( ref->merger.not_null() ) - { auto m(ref->merger.term()); +write_rocks_ref_(IOSTREAM *s, const dbref& ref, int flags) +{ Sfprintf(s, "(%p", &ref); + if ( ref.pathname.not_null() ) + Sfprintf(s, ",path=%Ws", ref.pathname.as_wstring().c_str()); + if ( ref.name.not_null() ) + Sfprintf(s, ",alias=%Ws", ref.name.as_wstring().c_str()); + if (ref.builtin_merger != MERGE_NONE) + Sfprintf(s, ",builtin_merger=%s", merge_t_char[ref.builtin_merger]); + if ( ref.merger.not_null() ) + { auto m(ref.merger.term()); if ( m.not_null() ) Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()); } - if ( !ref->db ) + if ( !ref.db ) Sfprintf(s, ",CLOSED"); - Sfprintf(s, ",key=%s,value=%s)", blob_type_char[ref->type.key], blob_type_char[ref->type.value]); + Sfprintf(s, ",key=%s,value=%s)", blob_type_char[ref.type.key], blob_type_char[ref.type.value]); if ( flags&PL_WRT_NEWLINE ) Sfprintf(s, "\n"); return true; @@ -271,15 +300,15 @@ write_rocks_ref_(IOSTREAM *s, const dbref *ref, int flags) [[nodiscard]] static bool -write_rocks_ref_(IOSTREAM *s, PlAtom eref, int flags) -{ const auto ref = *static_cast(eref.blob_data(nullptr, nullptr)); - return write_rocks_ref_(s, ref, flags); +write_rocks_ref_(IOSTREAM *s, PlAtom aref, int flags) +{ const auto ref = cast_dbref_blob_check(aref); + return write_rocks_ref_(s, *ref, flags); } [[nodiscard]] static int // TODO: bool -write_rocks_ref(IOSTREAM *s, atom_t eref, int flags) -{ return write_rocks_ref_(s, PlAtom(eref), flags); +write_rocks_ref(IOSTREAM *s, atom_t aref, int flags) +{ return write_rocks_ref_(s, PlAtom(aref), flags); } @@ -290,9 +319,9 @@ GC a rocks dbref blob from the atom garbage collector. [[nodiscard]] static bool release_rocks_ref_(PlAtom aref) -{ auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); +{ auto ref = cast_dbref_blob_check(aref); - delete ref; // TODO: delete aref.blob_data() is done by Prolog + delete ref; return true; } @@ -306,7 +335,7 @@ release_rocks_ref(atom_t aref) [[nodiscard]] static bool save_rocks_(PlAtom aref, IOSTREAM *fd) -{ const auto ref = *static_cast(aref.blob_data(nullptr, nullptr)); +{ const auto ref = cast_dbref_blob_check(aref); (void)fd; return PL_warning("Cannot save reference to (%p)", ref); @@ -341,15 +370,7 @@ load_rocks(IOSTREAM *fd) [[nodiscard]] static dbref * symbol_dbref(PlAtom symbol) -{ size_t len; - PL_blob_t *type; - const auto data = static_cast(symbol.blob_data(&len, &type)); - - if ( data && type == &rocks_blob ) - { return *data; - } - - return static_cast(nullptr); +{ return cast_dbref_blob(symbol); } @@ -377,7 +398,7 @@ get_rocks(PlTerm t, bool throw_if_closed=true) *******************************/ static PlException -RocksError(const rocksdb::Status &status, const dbref *ref) +RocksError(const rocksdb::Status& status, const dbref *ref) { if ( ref && ref->symbol.not_null() ) return PlGeneralError(PlCompound("rocks_error", PlTermv(PlTerm_atom(status.ToString()), @@ -388,7 +409,7 @@ RocksError(const rocksdb::Status &status, const dbref *ref) static bool -ok(const rocksdb::Status &status, const dbref *ref = nullptr) +ok(const rocksdb::Status& status, const dbref *ref) { if ( status.ok() ) return true; if ( status.IsNotFound() ) @@ -397,7 +418,7 @@ ok(const rocksdb::Status &status, const dbref *ref = nullptr) } static void -ok_or_throw_fail(const rocksdb::Status &status, const dbref *ref = nullptr) +ok_or_throw_fail(const rocksdb::Status& status, const dbref *ref) { PlCheckFail(ok(status, ref)); } @@ -492,7 +513,7 @@ get_slice(PlTerm t, blob_type type) [[nodiscard]] static bool -unify(PlTerm t, const rocksdb::Slice &s, blob_type type) +unify(PlTerm t, const rocksdb::Slice& s, blob_type type) { switch ( type ) { case BLOB_ATOM: return t.unify_chars(PL_ATOM|REP_UTF8, s.size_, s.data_); @@ -561,7 +582,7 @@ unify(PlTerm t, const rocksdb::Slice *s, blob_type type) [[nodiscard]] static bool -unify(PlTerm t, const std::string &s, blob_type type) +unify(PlTerm t, const std::string& s, blob_type type) { rocksdb::Slice sl(s); return unify(t, sl, type); @@ -569,7 +590,7 @@ unify(PlTerm t, const std::string &s, blob_type type) [[nodiscard]] static bool -unify_value(PlTerm t, const rocksdb::Slice &s, merger_t merge, blob_type type) +unify_value(PlTerm t, const rocksdb::Slice& s, merger_t merge, blob_type type) { if ( merge == MERGE_NONE ) return unify(t, s, type); @@ -622,7 +643,7 @@ unify_value(PlTerm t, const rocksdb::Slice &s, merger_t merge, blob_type type) [[nodiscard]] static bool -unify_value(PlTerm t, const std::string &s, merger_t merge, blob_type type) +unify_value(PlTerm t, const std::string& s, merger_t merge, blob_type type) { rocksdb::Slice sl(s); return unify_value(t, sl, merge, type); @@ -682,7 +703,7 @@ call_merger(const dbref_type_kv& type, PlTermv av, std::string* new_value, { Log(logger, "merger failed"); return false; } - } catch(PlException &ex) + } catch(PlException& ex) { Log(logger, "%s", ex.as_string(PlEncoding::UTF8).c_str()); throw; } @@ -934,48 +955,49 @@ lookup_read_optdef_and_apply(rocksdb::ReadOptions *options, : name(atom_), action(action_) { } }; + // TODO: #define RD_ODEF [options](PlTerm arg) #define RD_ODEF [](rocksdb::ReadOptions *o, PlTerm arg) static const ReadOptdef read_optdefs[] = { // "snapshot" const Snapshot* - // "iterate_lower_bound" const rocksdb::Slice* - // "iterate_upper_bound" const rocksdb::Slice* + // "iterate_lower_bound" const rocksdb::Slice* + // "iterate_upper_bound" const rocksdb::Slice* ReadOptdef("readahead_size", RD_ODEF { - o->readahead_size = arg.as_size_t(); } ), + o->readahead_size = arg.as_size_t(); } ), ReadOptdef("max_skippable_internal_keys", RD_ODEF { - o->max_skippable_internal_keys = arg.as_uint64_t(); } ), - // "read_tier" ReadTier + o->max_skippable_internal_keys = arg.as_uint64_t(); } ), + // "read_tier" ReadTier ReadOptdef("verify_checksums", RD_ODEF { - o->verify_checksums = arg.as_bool(); } ), + o->verify_checksums = arg.as_bool(); } ), ReadOptdef("fill_cache", RD_ODEF { - o->fill_cache = arg.as_bool(); } ), + o->fill_cache = arg.as_bool(); } ), ReadOptdef("tailing", RD_ODEF { - o->tailing = arg.as_bool(); } ), - // "managed" - not used any more + o->tailing = arg.as_bool(); } ), + // "managed" - not used any more ReadOptdef("total_order_seek", RD_ODEF { - o->total_order_seek = arg.as_bool(); } ), + o->total_order_seek = arg.as_bool(); } ), ReadOptdef("auto_prefix_mode", RD_ODEF { - o->auto_prefix_mode = arg.as_bool(); } ), + o->auto_prefix_mode = arg.as_bool(); } ), ReadOptdef("prefix_same_as_start", RD_ODEF { - o->prefix_same_as_start = arg.as_bool(); } ), + o->prefix_same_as_start = arg.as_bool(); } ), ReadOptdef("pin_data", RD_ODEF { - o->pin_data = arg.as_bool(); } ), + o->pin_data = arg.as_bool(); } ), ReadOptdef("background_purge_on_iterator_cleanup", RD_ODEF { - o->background_purge_on_iterator_cleanup = arg.as_bool(); } ), + o->background_purge_on_iterator_cleanup = arg.as_bool(); } ), ReadOptdef("ignore_range_deletions", RD_ODEF { - o->ignore_range_deletions = arg.as_bool(); } ), - // "table_filter" std::function - // TODO: "iter_start_seqnum" removed from rocksdb/include/options.h? - // { "iter_start_seqnum", RD_ODEF { - // o->iter_start_seqnum = static_cast(arg); } }, - // "timestamp" rocksdb::Slice* - // "iter_start_ts" rocksdb::Slice* + o->ignore_range_deletions = arg.as_bool(); } ), + // "table_filter" std::function + // TODO: "iter_start_seqnum" removed from rocksdb/include/options.h? + // { "iter_start_seqnum", RD_ODEF { + // o->iter_start_seqnum = static_cast(arg); } }, + // "timestamp" rocksdb::Slice* + // "iter_start_ts" rocksdb::Slice* ReadOptdef("deadline", RD_ODEF { - o->deadline = static_cast(arg.as_int64_t()); } ), + o->deadline = static_cast(arg.as_int64_t()); } ), ReadOptdef("io_timeout", RD_ODEF { - o->io_timeout = static_cast(arg.as_int64_t()); } ), + o->io_timeout = static_cast(arg.as_int64_t()); } ), ReadOptdef("value_size_soft_limit", RD_ODEF { - o->value_size_soft_limit = arg.as_uint64_t(); } ), + o->value_size_soft_limit = arg.as_uint64_t(); } ), ReadOptdef(PlAtom(PlAtom::null), nullptr) }; @@ -1008,21 +1030,22 @@ lookup_write_optdef_and_apply(rocksdb::WriteOptions *options, : name(atom_), action(action_) { } }; + // TODO: #define WR_ODEF [options](PlTerm arg) #define WR_ODEF [](rocksdb::WriteOptions *o, PlTerm arg) static const WriteOptdef optdefs[] = { WriteOptdef("sync", WR_ODEF { - o->sync = arg.as_bool(); } ), + o->sync = arg.as_bool(); } ), WriteOptdef("disableWAL", WR_ODEF { - o->disableWAL = arg.as_bool(); } ), + o->disableWAL = arg.as_bool(); } ), WriteOptdef("ignore_missing_column_families", WR_ODEF { - o->ignore_missing_column_families = arg.as_bool(); } ), + o->ignore_missing_column_families = arg.as_bool(); } ), WriteOptdef("no_slowdown", WR_ODEF { - o->no_slowdown = arg.as_bool(); } ), + o->no_slowdown = arg.as_bool(); } ), WriteOptdef("low_pri", WR_ODEF { - o->low_pri = arg.as_bool(); } ), + o->low_pri = arg.as_bool(); } ), WriteOptdef("memtable_insert_hint_per_batch", WR_ODEF { - o->memtable_insert_hint_per_batch = arg.as_bool(); } ), + o->memtable_insert_hint_per_batch = arg.as_bool(); } ), // "timestamp" rocksdb::Slice* WriteOptdef(PlAtom(PlAtom::null), nullptr) @@ -1064,7 +1087,7 @@ options_set_InfoLogLevel(rocksdb::Options *options, PlTerm arg) static void lookup_open_optdef_and_apply(rocksdb::Options *options, - PlAtom name, PlTerm opt) + PlAtom name, PlTerm opt) { typedef void (*OpenOptdefAction)(rocksdb::Options *options, PlTerm t); @@ -1078,6 +1101,7 @@ lookup_open_optdef_and_apply(rocksdb::Options *options, : name(atom_), action(action_) { } }; + // TODO: #define ODEF [options](PlTerm arg) #define ODEF [](rocksdb::Options *o, PlTerm arg) static const OpenOptdef open_optdefs[] = @@ -1087,170 +1111,170 @@ lookup_open_optdef_and_apply(rocksdb::Options *options, OpenOptdef("increase_parallelism", ODEF { if ( arg.as_bool() ) o->IncreaseParallelism(); } ), #endif OpenOptdef("create_if_missing", ODEF { - o->create_if_missing = arg.as_bool(); } ), + o->create_if_missing = arg.as_bool(); } ), OpenOptdef("create_missing_column_families", ODEF { - o->create_missing_column_families = arg.as_bool(); } ), + o->create_missing_column_families = arg.as_bool(); } ), OpenOptdef("error_if_exists", ODEF { - o->error_if_exists = arg.as_bool(); } ), + o->error_if_exists = arg.as_bool(); } ), OpenOptdef("paranoid_checks", ODEF { - o->paranoid_checks = arg.as_bool(); } ), + o->paranoid_checks = arg.as_bool(); } ), OpenOptdef("track_and_verify_wals_in_manifest", ODEF { - o->track_and_verify_wals_in_manifest = arg.as_bool(); } ), + o->track_and_verify_wals_in_manifest = arg.as_bool(); } ), // "env" Env::Default // "rate_limiter" - shared_ptr // "sst_file_manager" - shared_ptr // "info_log" - shared_ptr - see comment in ../README.md OpenOptdef("info_log_level", options_set_InfoLogLevel ), OpenOptdef("max_open_files", ODEF { - o->max_open_files = arg.as_int(); } ), + o->max_open_files = arg.as_int(); } ), OpenOptdef("max_file_opening_threads", ODEF { - o-> max_file_opening_threads = arg.as_int(); } ), + o-> max_file_opening_threads = arg.as_int(); } ), OpenOptdef("max_total_wal_size", ODEF { - o->max_total_wal_size = arg.as_uint64_t(); } ), + o->max_total_wal_size = arg.as_uint64_t(); } ), OpenOptdef("statistics", ODEF { - o->statistics = arg.as_bool() ? rocksdb::CreateDBStatistics() : nullptr; } ), + o->statistics = arg.as_bool() ? rocksdb::CreateDBStatistics() : nullptr; } ), OpenOptdef("use_fsync", ODEF { - o->use_fsync = arg.as_bool(); } ), + o->use_fsync = arg.as_bool(); } ), // "db_paths" - vector OpenOptdef("db_log_dir", ODEF { - o->db_log_dir = arg.as_string(PlEncoding::Locale); } ), + o->db_log_dir = arg.as_string(PlEncoding::Locale); } ), OpenOptdef("wal_dir", ODEF { - o->wal_dir = arg.as_string(PlEncoding::Locale); } ), + o->wal_dir = arg.as_string(PlEncoding::Locale); } ), OpenOptdef("delete_obsolete_files_period_micros", ODEF { - o->delete_obsolete_files_period_micros = arg.as_uint64_t(); } ), + o->delete_obsolete_files_period_micros = arg.as_uint64_t(); } ), OpenOptdef("max_background_jobs", ODEF { - o->max_background_jobs = arg.as_int(); } ), + o->max_background_jobs = arg.as_int(); } ), // "base_background_compactions" is obsolete // "max_background_compactions" is obsolete OpenOptdef("max_subcompactions", ODEF { - o->max_subcompactions = arg.as_uint32_t(); } ), + o->max_subcompactions = arg.as_uint32_t(); } ), // "max_background_flushes" is obsolete OpenOptdef("max_log_file_size", ODEF { - o->max_log_file_size = arg.as_size_t(); } ), + o->max_log_file_size = arg.as_size_t(); } ), OpenOptdef("log_file_time_to_roll", ODEF { - o->log_file_time_to_roll = arg.as_size_t(); } ), + o->log_file_time_to_roll = arg.as_size_t(); } ), OpenOptdef("keep_log_file_num", ODEF { - o->keep_log_file_num = arg.as_size_t(); } ), + o->keep_log_file_num = arg.as_size_t(); } ), OpenOptdef("recycle_log_file_num", ODEF { - o->recycle_log_file_num = arg.as_size_t(); } ), + o->recycle_log_file_num = arg.as_size_t(); } ), OpenOptdef("max_manifest_file_size", ODEF { - o->max_manifest_file_size = arg.as_uint64_t(); } ), + o->max_manifest_file_size = arg.as_uint64_t(); } ), OpenOptdef("table_cache_numshardbits", ODEF { - o->table_cache_numshardbits = arg.as_int(); } ), + o->table_cache_numshardbits = arg.as_int(); } ), OpenOptdef("wal_ttl_seconds", ODEF { - o->WAL_ttl_seconds = arg.as_uint64_t(); } ), + o->WAL_ttl_seconds = arg.as_uint64_t(); } ), OpenOptdef("wal_size_limit_mb", ODEF { - o->WAL_size_limit_MB = arg.as_uint64_t(); } ), + o->WAL_size_limit_MB = arg.as_uint64_t(); } ), OpenOptdef("manifest_preallocation_size", ODEF { - o->manifest_preallocation_size = arg.as_size_t(); } ), + o->manifest_preallocation_size = arg.as_size_t(); } ), OpenOptdef("allow_mmap_reads", ODEF { - o->allow_mmap_reads = arg.as_bool(); } ), + o->allow_mmap_reads = arg.as_bool(); } ), OpenOptdef("allow_mmap_writes", ODEF { - o->allow_mmap_writes = arg.as_bool(); } ), + o->allow_mmap_writes = arg.as_bool(); } ), OpenOptdef("use_direct_reads", ODEF { - o->use_direct_reads = arg.as_bool(); } ), + o->use_direct_reads = arg.as_bool(); } ), OpenOptdef("use_direct_io_for_flush_and_compaction", ODEF { - o->use_direct_io_for_flush_and_compaction = arg.as_bool(); } ), + o->use_direct_io_for_flush_and_compaction = arg.as_bool(); } ), OpenOptdef("allow_fallocate", ODEF { - o->allow_fallocate = arg.as_bool(); } ), + o->allow_fallocate = arg.as_bool(); } ), OpenOptdef("is_fd_close_on_exec", ODEF { - o->is_fd_close_on_exec = arg.as_bool(); } ), + o->is_fd_close_on_exec = arg.as_bool(); } ), // "skip_log_error_on_recovery" is obsolete OpenOptdef("stats_dump_period_sec", ODEF { - o->stats_dump_period_sec = arg.as_uint32_t(); } ), // TODO: match: unsigned int stats_dump_period_sec + o->stats_dump_period_sec = arg.as_uint32_t(); } ), // TODO: match: unsigned int stats_dump_period_sec OpenOptdef("stats_persist_period_sec", ODEF { - o->stats_persist_period_sec = arg.as_uint32_t(); } ), // TODO: match: unsigned int stats_persist_period_sec + o->stats_persist_period_sec = arg.as_uint32_t(); } ), // TODO: match: unsigned int stats_persist_period_sec OpenOptdef("persist_stats_to_disk", ODEF { - o->persist_stats_to_disk = arg.as_bool(); } ), + o->persist_stats_to_disk = arg.as_bool(); } ), OpenOptdef("stats_history_buffer_size", ODEF { - o->stats_history_buffer_size = arg.as_size_t(); } ), + o->stats_history_buffer_size = arg.as_size_t(); } ), OpenOptdef("advise_random_on_open", ODEF { - o->advise_random_on_open = arg.as_bool(); } ), + o->advise_random_on_open = arg.as_bool(); } ), OpenOptdef("db_write_buffer_size", ODEF { - o->db_write_buffer_size = arg.as_size_t(); } ), + o->db_write_buffer_size = arg.as_size_t(); } ), // "write_buffer_manager" - shared_ptr // "access_hint_on_compaction_start" - enum AccessHint // TODO: "new_table_reader_for_compaction_inputs" removed from rocksdb/include/options.h? // OpenOptdef("new_table_reader_for_compaction_inputs", ODEF { // o->new_table_reader_for_compaction_inputs = arg.as_bool(); } ), OpenOptdef("compaction_readahead_size", ODEF { - o->compaction_readahead_size = arg.as_size_t(); } ), + o->compaction_readahead_size = arg.as_size_t(); } ), OpenOptdef("random_access_max_buffer_size", ODEF { - o->random_access_max_buffer_size = arg.as_size_t(); } ), + o->random_access_max_buffer_size = arg.as_size_t(); } ), OpenOptdef("writable_file_max_buffer_size", ODEF { - o->writable_file_max_buffer_size = arg.as_size_t(); } ), + o->writable_file_max_buffer_size = arg.as_size_t(); } ), OpenOptdef("use_adaptive_mutex", ODEF { - o->use_adaptive_mutex = arg.as_bool(); } ), + o->use_adaptive_mutex = arg.as_bool(); } ), OpenOptdef("bytes_per_sync", ODEF { - o->bytes_per_sync = arg.as_uint64_t(); } ), + o->bytes_per_sync = arg.as_uint64_t(); } ), OpenOptdef("wal_bytes_per_sync", ODEF { - o->wal_bytes_per_sync = arg.as_uint64_t(); } ), + o->wal_bytes_per_sync = arg.as_uint64_t(); } ), OpenOptdef("strict_bytes_per_sync", ODEF { - o->strict_bytes_per_sync = arg.as_bool(); } ), + o->strict_bytes_per_sync = arg.as_bool(); } ), // "listeners" - vector> OpenOptdef("enable_thread_tracking", ODEF { - o->enable_thread_tracking = arg.as_bool(); } ), + o->enable_thread_tracking = arg.as_bool(); } ), OpenOptdef("delayed_write_rate", ODEF { - o->delayed_write_rate = arg.as_uint64_t(); } ), + o->delayed_write_rate = arg.as_uint64_t(); } ), OpenOptdef("enable_pipelined_write", ODEF { - o->enable_pipelined_write = arg.as_bool(); } ), + o->enable_pipelined_write = arg.as_bool(); } ), OpenOptdef("unordered_write", ODEF { - o->unordered_write = arg.as_bool(); } ), + o->unordered_write = arg.as_bool(); } ), OpenOptdef("allow_concurrent_memtable_write", ODEF { - o->allow_concurrent_memtable_write = arg.as_bool(); } ), + o->allow_concurrent_memtable_write = arg.as_bool(); } ), OpenOptdef("enable_write_thread_adaptive_yield", ODEF { - o->enable_write_thread_adaptive_yield = arg.as_bool(); } ), + o->enable_write_thread_adaptive_yield = arg.as_bool(); } ), OpenOptdef("max_write_batch_group_size_bytes", ODEF { - o->max_write_batch_group_size_bytes = arg.as_uint64_t(); } ), + o->max_write_batch_group_size_bytes = arg.as_uint64_t(); } ), OpenOptdef("write_thread_max_yield_usec", ODEF { - o->write_thread_max_yield_usec = arg.as_uint64_t(); } ), + o->write_thread_max_yield_usec = arg.as_uint64_t(); } ), OpenOptdef("write_thread_slow_yield_usec", ODEF { - o->write_thread_slow_yield_usec = arg.as_uint64_t(); } ), + o->write_thread_slow_yield_usec = arg.as_uint64_t(); } ), OpenOptdef("skip_stats_update_on_db_open", ODEF { - o->skip_stats_update_on_db_open = arg.as_bool(); } ), + o->skip_stats_update_on_db_open = arg.as_bool(); } ), OpenOptdef("skip_checking_sst_file_sizes_on_db_open", ODEF { - o->skip_checking_sst_file_sizes_on_db_open = arg.as_bool(); } ), + o->skip_checking_sst_file_sizes_on_db_open = arg.as_bool(); } ), // "wal_recovery_mode" - enum WALRecoveryMode OpenOptdef("allow_2pc", ODEF { - o->allow_2pc = arg.as_bool(); } ), + o->allow_2pc = arg.as_bool(); } ), // "row_cache" - shared_ptr // "wal_filter" - WalFilter* OpenOptdef("fail_ifoptions_file_error", ODEF { - o->fail_if_options_file_error = arg.as_bool(); } ), + o->fail_if_options_file_error = arg.as_bool(); } ), OpenOptdef("dump_malloc_stats", ODEF { - o->dump_malloc_stats = arg.as_bool(); } ), + o->dump_malloc_stats = arg.as_bool(); } ), OpenOptdef("avoid_flush_during_recovery", ODEF { - o->avoid_flush_during_recovery = arg.as_bool(); } ), + o->avoid_flush_during_recovery = arg.as_bool(); } ), OpenOptdef("avoid_flush_during_shutdown", ODEF { - o->avoid_flush_during_shutdown = arg.as_bool(); } ), + o->avoid_flush_during_shutdown = arg.as_bool(); } ), OpenOptdef("allow_ingest_behind", ODEF { - o->allow_ingest_behind = arg.as_bool(); } ), + o->allow_ingest_behind = arg.as_bool(); } ), // TODO: "preserve_deletes" removed from rocksdb/include/options.h? // OpenOptdef("preserve_deletes", ODEF { // o->preserve_deletes = arg.as_bool(); } ), OpenOptdef("two_write_queues", ODEF { - o->two_write_queues = arg.as_bool(); } ), + o->two_write_queues = arg.as_bool(); } ), OpenOptdef("manual_wal_flush", ODEF { - o->manual_wal_flush = arg.as_bool(); } ), + o->manual_wal_flush = arg.as_bool(); } ), OpenOptdef("atomic_flush", ODEF { - o->atomic_flush = arg.as_bool(); } ), + o->atomic_flush = arg.as_bool(); } ), OpenOptdef("avoid_unnecessary_blocking_io", ODEF { - o->avoid_unnecessary_blocking_io = arg.as_bool(); } ), + o->avoid_unnecessary_blocking_io = arg.as_bool(); } ), OpenOptdef("write_dbid_to_manifest", ODEF { - o->write_dbid_to_manifest = arg.as_bool(); } ), + o->write_dbid_to_manifest = arg.as_bool(); } ), OpenOptdef("log_readahead_size", ODEF { - o->write_dbid_to_manifest = arg.as_bool(); } ), + o->write_dbid_to_manifest = arg.as_bool(); } ), // "file_checksum_gen_factory" - std::shared_ptr OpenOptdef("best_efforts_recovery", ODEF { - o->best_efforts_recovery = arg.as_bool(); } ), + o->best_efforts_recovery = arg.as_bool(); } ), OpenOptdef("max_bgerror_resume_count", ODEF { - o->max_bgerror_resume_count = arg.as_int(); } ), + o->max_bgerror_resume_count = arg.as_int(); } ), OpenOptdef("bgerror_resume_retry_interval", ODEF { - o->bgerror_resume_retry_interval = arg.as_uint64_t(); } ), + o->bgerror_resume_retry_interval = arg.as_uint64_t(); } ), OpenOptdef("allow_data_in_errors", ODEF { - o->allow_data_in_errors = arg.as_bool(); } ), + o->allow_data_in_errors = arg.as_bool(); } ), OpenOptdef("db_host_id", ODEF { - o->db_host_id = arg.as_string(PlEncoding::Locale); } ), + o->db_host_id = arg.as_string(PlEncoding::Locale); } ), // "checksum_handoff_file_types" - FileTypeSet { PlAtom(PlAtom::null), nullptr } @@ -1326,6 +1350,9 @@ PREDICATE(rocks_open_, 3) throw PlPermissionError("alias", "rocksdb", PlTerm_atom(alias)); } + // Allocating the blob uses unique_ptr so that it'll be + // deleted if an error happens - this is disabled by ref.release() + // before returning success. auto ref = std::make_unique(); ref->merger = merger; ref->builtin_merger = builtin_merger; @@ -1343,18 +1370,19 @@ PREDICATE(rocks_open_, 3) options.merge_operator.reset(new ListMergeOperator(*ref)); ok_or_throw_fail(read_only ? rocksdb::DB::OpenForReadOnly(options, fn, &ref->db) - : rocksdb::DB::Open(options, fn, &ref->db)); + : rocksdb::DB::Open(options, fn, &ref->db), + ref.get()); if ( ref->name.is_null() ) - { PlCheckFail(A2.unify_blob(&ref, sizeof ref, &rocks_blob)); + { PlCheckFail(A2.unify_blob(ref.get(), sizeof *ref, &rocks_blob)); } else { PlTerm_var tmp; - PlCheckFail(tmp.unify_blob(&ref, sizeof ref, &rocks_blob)); + PlCheckFail(tmp.unify_blob(ref.get(), sizeof *ref, &rocks_blob)); rocks_add_alias(ref->name, tmp.as_atom()); PlCheckFail(A2.unify_atom(ref->name)); } - (void)ref.release(); // ref now owned by Prolog + (void)ref.release(); // ref now owned by Prolog, deleted in release_rocks_ref_() return true; } @@ -1375,7 +1403,7 @@ PREDICATE(rocks_close, 1) { auto db = ref->db; ref->db = nullptr; if ( db ) - ok_or_throw_fail(db->Close()); + ok_or_throw_fail(db->Close(), ref); } return true; @@ -1604,7 +1632,7 @@ PREDICATE_NONDET(rocks_enum_prefix, 5) static void -batch_operation(const dbref *ref, rocksdb::WriteBatch &batch, PlTerm e) +batch_operation(const dbref& ref, rocksdb::WriteBatch *batch, PlTerm e) { PlAtom name(PlAtom::null); size_t arity; @@ -1613,12 +1641,12 @@ batch_operation(const dbref *ref, rocksdb::WriteBatch &batch, PlTerm e) PlCheckFail(e.name_arity(&name, &arity)); if ( ATOM_delete == name && arity == 1 ) - { const auto key = get_slice(e[1], ref->type.key); - batch.Delete(key->slice()); + { const auto key = get_slice(e[1], ref.type.key); + batch->Delete(key->slice()); } else if ( ATOM_put == name && arity == 2 ) - { const auto key = get_slice(e[1], ref->type.key); - const auto value = get_slice(e[2], ref->type.value); - batch.Put(key->slice(), value->slice()); + { const auto key = get_slice(e[1], ref.type.key); + const auto value = get_slice(e[2], ref.type.value); + batch->Put(key->slice(), value->slice()); } else { throw PlDomainError("rocks_batch_operation", e); } @@ -1632,7 +1660,7 @@ PREDICATE(rocks_batch, 3) PlTerm_var e; while ( tail.next(e) ) - { batch_operation(ref, batch, e); + { batch_operation(*ref, &batch, e); } ok_or_throw_fail(ref->db->Write(write_options(A3), &batch), ref); From 7c9a4bdf31a72e05bfc6d63fa172bd5511284dd7 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Tue, 6 Jun 2023 19:32:36 -0700 Subject: [PATCH 17/28] Use C++ API for blobs - Some of the code will go to SWI-cpp2.h (and documentation to pl2cpp2.doc). - Documentation needs improvement. - Need to add a few more tests (especially error handling) and test with ASAN. --- cpp/rocksdb4pl.cpp | 623 +++++++++++++++++++++++++++++++------------ test/test_rocksdb.pl | 13 +- 2 files changed, 461 insertions(+), 175 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 702390b..7c33df9 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -45,7 +45,391 @@ #include #include -#include // TODO: this should possibly be in a separate file +#include // This could be put in a separate file + +// ------------------- To be added to SWI-cpp2.h for blobs ----------- + +// TODO: these should be in a namespace + +/*** Documentation (This will go into pl2cpp2.doc) + +\subsection{Blobs} +\label{sec:cpp2-blobs} + + +Disclaimer: +The blob API for C++ is not completely general, but is designed to +make a specific use case easier to write. For other use cases, the +underlying C API can still be used. The use case is: +\begin{itemize} +\item The blob contains the foreign object (e.g., contains a + pointer to a database connection). +\item The blob is created by a predicate that makes the foreign + object and stores it (or a pointer to it) within the blob. + For example, makes a connection to a database or compiles + a regular expression into an internal form. +\item Optionally, there is a predicate that deletes the foreign object. +\item The blob will not be subclassed. +\item The blob is defined as a subclass of \ctype{PlBlob}, which + provides a number of fields and methods, of which a few + can be overridden in the blob (notably: write(), compare(), + and the destructor). +\item The blob's constructor throws an exception and cleans up + any resources if it cannot create the blob. +\item The foreign object can be deleted when the blob is deleted. +\item The blob's allocation is controlled by Prolog and its + destructor is envoked when the blob is garbage collected. +end{itemize} + +A Prolog blob consists of five parts: +\begin{itemize} +\item A \ctype{PL_blob_t} structure that defines the callbacks. +\item A "contents" structure that contains the blob. +\item A "create" or "open" predicate that unifies one of its arguments + with a newly created blob that contains the foreign object. +\item (Optionally) a "close" predicate that does the opposit of the + "create"/"open" predicate. +\item Predicates that manipulate the foreign object (e.g., for a file-like + object, these could be read, write, seek, etc.). +\end{itemize} + +For the \ctype{PL_blot_t structure, this API provides a set of +template functions that allow easily setting up the callbacks, and +allow defining the corresonding methods int he blob "contents" class. + +For the "contents" structure, which is subclassed from \ctype{PlBlob}, +the programmer defines the various fields, a constructor that +initializes them, and a destructor. Optionally, methods can be +defined for one of more of blob \cfuncref{compare}{}, +\cfuncref{write}{}, \cfuncref{save}{}, \cfuncref{load}{} (the +\cfuncref{acquire}{} and \cfuncref{release}{} callbacks should +normally be allowed to default to what is defined in \ctype{PlBlob}. + +There is a mismatch between how Prolog does memory management (and +garbage collection) and how C++ does it. In particular, Prolog assumes +that cleanup will be done in the \cfuncref{release}{} function +associated with the blob whereas C++ typically does cleanup in a +destructor. The blob interface gets around this mismatch by providing +a default \cfuncref{release}{} function that assumes that the blob was +created using \const{PL_BLOB_NOCOPY} and manages memory using a +\ctype{std::unique_ptr}. + +The C blob interface has a flag that determines how memory is managed: +\const{PL_BLOB_NOCOPY}. If this is set, Prolog does not do a call to +cfuncref{free} when the blob is garbage collected; instead, it assumes +that the blob's cfuncref{release} function will free the memory. + +The C++ API for blobs only supports blobs with +\const{PL_BLOB_NOCOPY}.\footnote{The API can probably also support +blobs with \const{PL_BLOB_UNIQUE}, but there seems to be little +point in setting this flag for non-text blobs.} + +\subsubsection{How to define a blob using C++} +\label{sec:cpp2-blobs-howto} + +TL;DR: Use PL_BLOB_NOCOPY (or an extra level of indirection) and the +default \ctype{PlBlob} wrappers; no copy constructor, move +constructor, or assignment operator; create blob with +\exam{make_unique}, use \exam{unique_ptr::release}, do \exam{delete} +in the "release" function. + +Here is minimal sample code for creating a blob that owns a connection +to a database: + +\begin{code} +PL_blob_t my_blob = +{ .magic = PL_BLOB_MAGIC, + .flags = PL_BLOB_NOCOPY, + .name = "my_blob", + .release = blob_release, + .write = blob_write, + .acquire = blob_acquire, + .save = blob_save, + .load = blob_save +}; + +struct my_blob_contents : public PlBlob +{ DbConnection *connection; + + explicit my_blob_contents() + : DbConnection(nullptr)) { } + + ~my_blob_contents() + { if ( connection ) + { // This is similar to close_my_blob/1, except there's + // no check for an error because there's nothing we + // can do on an error because throwing a C++ exception + // inside of C code will cause a runtime error. + (void)connection->close(); + } + } +}; + +// %! create_my_blob(+Options: list, -MyBlob) is semidet. +PREDICATE(create_my_blob, 2) +{ auto ref = std::make_unique(...); + // ... fill in the fields of *ref from options in A1 ... + int rc = do_open(..., &ref->connection); + if ( !rc ) + throw PlGeneralError(PlCompound("my_blob_error", ref->symbol_term())); + PlCheckFail(A2.unify_blob(ref.get(), sizeof *ref, &my_blob)); + (void)ref.release(); + return true; + +// %! close_my_blob(+MyBlob) is det. +// % Close the connection, silently succeeding if is already +// % closed; throw an exception if something goes wrong. +PREDICATE(close_my_blob, 1) +{ auto ref = cast_blob_check(A1.as_atom()); + auto c = ref->connection; + ref->connection = nullptr; + if ( c ) + { int rc = c->close(); + delete c; + if ( !rc ) + throw PlGeneralError(PlCompound("my_blob_error", ...)); + } + return true; +} +\end{code} + +Explanation of the \ctype{my_blob} structure: +\begin{itemize} + +\item The \exam{magic}, \exam{flags}, and \exam{name} flags are required. + +\item The \examl{release} and \examl{acquire} fields are required. The + defaults given will normally suffice (see more on this in the + definition of \exam{my_blob_contents}). + +\item If the fields \exam{save} and \exam{load} are given, they + provide a default implementation that throws an error on an + attempt to save the blob (e.g., by using + \qsave_program/[1,2]). If they are omitted, the defaults for + \exam{save} and \exam{load} are used, which save the internal + form of the blob, which is probably not what you want. If you + wish to define your own \exam{save} and \exam{load}, you must + also add \exam{save} and \exam{load} methods to + \ctype{my_blob_contents}. + +\item The \exam{compare} and/or \exam{write} fields may optionally be + defined, analagously to the \exam{acquire} and \examl{release} + fields. If given, you must also add a corresponding + \exam{compare} and/or \exam{write} method(s) to + \ctype{my_blob_contents}. + +\end{itemize} + +Explanation of the \ctype{my_blob_contents} structure: + +\begin{itemize} + +\item \exam{PlBlob provides default methods plus a few + utility methods: + \begin{itemize} + \item Copy and move constructors are disabled, as is the + assignment operator. + \item validate() verifies that the blob is valid, assumign that + the default \cfuncref{acquire}{} has been called. + \item acquire() is used by the default \cfuncref{acquire}{} function. + \item symbol_not_null() tests whether \exam{symbol} has been set + (this will normally be the case, unless the \cfuncref{acquire}{} + function hasn't been called as part of the blob creation + process. + \item symbol_term() Creates a term the contains the blob, for + use in error terms. It is always safe to use this; if + the symbol hasn't bene set, symbol_term() returns a "var" term. + \item compare_using_ptrs() The default method of comparison, used to + break ties between otherwise equal blobs. + \item write() A default implementation that outputs \exam{(ptr)}. + \item save() Generates an error when attempting to save the blob. + \item laod() generates an error when attemptint to laod the blob. + \end{itemize} + +\item The constructor must initialize fields so that the destructor + can determine whether to delete them or not. Typically, this is + done by setting pointers to \const{nullptr} or by setting a + flag. + +\item The destructor is called by \cfuncref{release}{}, assuming you've + set \exam{my_blob::release} to \exam{blob_release}. + +\item This blob uses the default methods for compare() and write(). + You can change this by defining your own compare() and write() + methods. If you define compare(), you must add the related line + to \exam{my_blob} and you should call compare_using_ptrs() if + your comparison of the two objects shows equality (this breaks + the tie for two otherwise equal by distinct objects). + +\end{itemize} + +Explanation of open_my_blob/2: + +\begin{itemize} + +\item \exam{std::make_unique(...)} creates a + \ctype{std::unique_ptr} that is deleted when it goes out + of scope. If an exception occurs between the creation of blob + (as a \ctype{std::unique_ptr}) and the call to + \cfuncref{unify_blob}{}, the pointer will be freed (and the + \ctype{MyBlob} destructor will be called. + +\item The exception term contains the blob, using + \exam{ref->symbol_term()}. + +\item \exam{ref.release()} prevents the pointer from being + automatically deleted; it is now "owned" by Prolog and is + deleted in the \cfuncref{release}{} function (which is defined in + \ctype{PlBlob}). + +\end{itemize} + +Explanation of close_my_blob/1: + +\begin{itemize} + +\item This code is similar to ~my_blob_contents(), except it throws + an error if something goes wrong during the "close". + +\item Other predicates that use the blob access the + blob from the arguments the same way, namely + \exam{cast_blob_check(A1.as_atom())}. + +\end{itemize} + +\subsubsection{A digression on the semantics of atoms and equality} +\label{sec:cppw-blobs-equality} + +Prolog atoms differ from strings in that atom equality can be +determined simply by comparing the atom indexes without comparing the +string values. If the \const[PL_BLOB_UNIQUE} flag is set, then every +time a blob is created, it is looked up in the atom table to see if +there is another blob with the same bit pattern. If the flag is not +set, each time a blob is created, it gets a new entry in the atom +table. + +Things become a bit more complicated when we want to sort blobs. If +the user defines the \cfuncref{compare}{} function to return 0 +("equal") based on only some of the fields in the blob, then it is +possible for the \cfuncref{compare}{} to mark two blobs as equal even +if they have different indexes in the atom table. To avoid this +inconsistency, if the \cfuncref{compare}{} function determines that +the two blobs are equivalent, it should still do a final comparison +using the blob pointers to enforce a total ordering - +\cfuncref{compare_using_ptrs}{} will do that. For example, the blob +for PCRE2 regular expressions compares the patterns of the two blobs +and if they are equal, compares the blob pointers. In this way, if two +blobs have the same pattern, they will not compare as equal, so sort/2 +won't remove one of them as a duplicate. + + ***/ + +template [[nodiscard]] +static C_t * +cast_blob(PlAtom aref) +{ size_t len; + PL_blob_t *type; + auto ref = static_cast(aref.blob_data(&len, &type)); + if ( ref && type == ref->blob_t_) + { assert(len == sizeof *ref); + assert(ref->validate()); + return ref; + } + return nullptr; +} + +template [[nodiscard]] +static C_t* +cast_blob_check(PlAtom aref) +{ auto ref = cast_blob(aref); + assert(ref); + return ref; +} + +template +void blob_acquire(atom_t a) +{ PlAtom a_(a); + auto data = cast_blob_check(a_); + data->acquire(a_); +} + +template [[nodiscard]] +int blob_release(atom_t a) +{ auto data = cast_blob_check(PlAtom(a)); + delete data; + return true; +} + +template [[nodiscard]] +int blob_compare(atom_t a, atom_t b) +{ const auto data = cast_blob_check(PlAtom(a)); + return data->compare(PlAtom(b)); +} + +template [[nodiscard]] +int blob_write(IOSTREAM *s, atom_t a, int flags) +{ const auto data = cast_blob_check(PlAtom(a)); + return data->write(s, flags); +} + +template [[nodiscard]] +int blob_save(atom_t a, IOSTREAM *fd) +{ const auto data = cast_blob_check(PlAtom(a)); + return data->save(fd); +} + +template [[nodiscard]] +atom_t blob_load(IOSTREAM *fd) +{ return C_t::load(fd).C_; +} + +template +class PlBlob +{ +public: + explicit PlBlob() + : blob_t_(&blob_t), + symbol_(PlAtom(PlAtom::null)) // filled in by acquire() + { } + explicit PlBlob(const PlBlob&) = delete; + explicit PlBlob(PlBlob&&) = delete; + PlBlob& operator =(const PlBlob&) = delete; + ~PlBlob() = default; + + bool validate() const { return blob_t_ == &blob_t; } + + void acquire(PlAtom _symbol) { symbol_ = _symbol; } + + bool symbol_not_null() const { return symbol_.not_null(); } + PlTerm symbol_term() const + { if ( symbol_not_null() ) + return PlTerm_atom(symbol_); + return PlTerm_var(); + } + + // TODO: document that release() is handled by the destructor + + // compare() is provided as a fall-back for when other comparison's don't work. + // It's OK to specify it; but it's the same as the default comparison + int compare_using_ptrs(PlAtom _b) const { + const auto b = cast_blob_check(_b); + return this > b ? 1 : this < b ? -1 : 0; } + + bool write(IOSTREAM *s, int flags) const { Sfprintf(s, "<%s>(%p)", blob_t.name, this); return true; } + + bool save(IOSTREAM *fd) const { return PL_warning("Cannot save reference to <%s>(%p)", blob_t.name, this); } + + static PlAtom load(IOSTREAM *fd) + { (void)PL_warning("Cannot load reference to <%s>", blob_t.name); + PL_fatal_error("Cannot load reference to <%s>", blob_t.name); + return PlAtom(PlAtom::null); + } + + const PL_blob_t *blob_t_; + PlAtom symbol_; /* associated symbol (used for error terms) */ +}; + +// ------------------- end: To be added to SWI-cpp2.h for blobs ----------- [[nodiscard]] static PlAtom rocks_get_alias(PlAtom name); [[nodiscard]] static PlAtom rocks_get_alias_inside_lock(PlAtom name); @@ -92,22 +476,33 @@ static const char* merge_t_char[] = "set" }; - struct dbref_type_kv { blob_type key; blob_type value; }; -#define DBREF_MAGIC 0xDB00DB01 +struct dbref; + + +static PL_blob_t rocks_blob = +{ .magic = PL_BLOB_MAGIC, + .flags = PL_BLOB_NOCOPY, + .name = "rocksdb", + .release = blob_release, + .compare = blob_compare, + .write = blob_write, + .acquire = blob_acquire, + .save = blob_save, + .load = blob_load +}; -struct dbref +struct dbref : PlBlob { dbref() : db( nullptr), pathname( PlAtom(PlAtom::null)), - symbol( PlAtom(PlAtom::null)), // filled in by acquire_rocks_ref_() name( PlAtom(PlAtom::null)), builtin_merger( MERGE_NONE), merger( PlRecord(PlRecord::null)), @@ -115,9 +510,36 @@ struct dbref .value = BLOB_ATOM}) { } - dbref(const dbref&) = delete; - dbref(dbref&&) = delete; - dbref& operator =(const dbref&) = delete; + bool write(IOSTREAM *s, int flags) const /* override */ + { Sfprintf(s, "<%s>(%p", blob_t_->name, this); + if ( pathname.not_null() ) + Sfprintf(s, ",path=%Ws", pathname.as_wstring().c_str()); + if ( name.not_null() ) + Sfprintf(s, ",alias=%Ws", name.as_wstring().c_str()); + if (builtin_merger != MERGE_NONE) + Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]); + if ( merger.not_null() ) + { auto m(merger.term()); + if ( m.not_null() ) + Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()); + } + if ( !db ) + Sfprintf(s, ",CLOSED"); + Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); + if ( flags&PL_WRT_NEWLINE ) + Sfprintf(s, "\n"); + return true; + } + + int compare(PlAtom _b) const + { const auto b = cast_blob_check(_b); + if ( this == b ) // Prolog should have already done this test, but it's cheap + return 0; + int c_pathname = PlTerm_atom(pathname).compare(PlTerm_atom(b->pathname)); + if ( c_pathname != 0 ) + return c_pathname; + return compare_using_ptrs(_b); + } ~dbref() { // See also predicate rocks_close/1 @@ -135,10 +557,8 @@ struct dbref } } - unsigned magic = DBREF_MAGIC; rocksdb::DB *db; /* DB handle */ PlAtom pathname; /* DB's absolute file name (for debugging) */ - PlAtom symbol; /* associated symbol (used for error terms) */ PlAtom name; /* alias name (can be PlAtom::null) */ merger_t builtin_merger; /* C++ Merger */ PlRecord merger; /* merge option */ @@ -146,32 +566,6 @@ struct dbref }; -static void acquire_rocks_ref_(PlAtom aref); -static bool release_rocks_ref_(PlAtom aref); -static bool write_rocks_ref_(IOSTREAM *s, PlAtom aref, int flags); -static bool write_rocks_ref_(IOSTREAM *s, const dbref& ref, int flags); -static bool save_rocks_(PlAtom aref, IOSTREAM *fd); -static PlAtom load_rocks_(IOSTREAM *fd); - -static void acquire_rocks_ref(atom_t symbol); -static int release_rocks_ref(atom_t aref); -static int write_rocks_ref(IOSTREAM *s, atom_t aref, int flags); -static int save_rocks(atom_t aref, IOSTREAM *fd); -static atom_t load_rocks(IOSTREAM *fd); - -static PL_blob_t rocks_blob = -{ .magic = PL_BLOB_MAGIC, - .flags = PL_BLOB_NOCOPY, - .name = "rocksdb", - .release = release_rocks_ref, - .compare = nullptr, - .write = write_rocks_ref, - .acquire = acquire_rocks_ref, - .save = save_rocks, - .load = load_rocks -}; - - /******************************* * ALIAS * *******************************/ @@ -242,135 +636,16 @@ rocks_unalias(PlAtom name) * SYMBOL REFERENCES * *******************************/ -[[nodiscard]] -static dbref * -cast_dbref_blob(PlAtom aref) -{ size_t len; - PL_blob_t *type; - auto ref = static_cast(aref.blob_data(&len, &type)); - if ( ref && type == &rocks_blob) - { assert(len == sizeof *ref && ref->magic == DBREF_MAGIC); - return ref; - } - return nullptr; -} - -[[nodiscard]] -static dbref * -cast_dbref_blob_check(PlAtom aref) -{ auto ref = cast_dbref_blob(aref); - assert(ref); - return ref; -} - -static void -acquire_rocks_ref_(PlAtom aref) -{ auto ref = cast_dbref_blob_check(aref); - ref->symbol = aref; -} - -static void -acquire_rocks_ref(atom_t symbol) -{ acquire_rocks_ref_(PlAtom(symbol)); -} - - -[[nodiscard]] -static bool -write_rocks_ref_(IOSTREAM *s, const dbref& ref, int flags) -{ Sfprintf(s, "(%p", &ref); - if ( ref.pathname.not_null() ) - Sfprintf(s, ",path=%Ws", ref.pathname.as_wstring().c_str()); - if ( ref.name.not_null() ) - Sfprintf(s, ",alias=%Ws", ref.name.as_wstring().c_str()); - if (ref.builtin_merger != MERGE_NONE) - Sfprintf(s, ",builtin_merger=%s", merge_t_char[ref.builtin_merger]); - if ( ref.merger.not_null() ) - { auto m(ref.merger.term()); - if ( m.not_null() ) - Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()); - } - if ( !ref.db ) - Sfprintf(s, ",CLOSED"); - Sfprintf(s, ",key=%s,value=%s)", blob_type_char[ref.type.key], blob_type_char[ref.type.value]); - if ( flags&PL_WRT_NEWLINE ) - Sfprintf(s, "\n"); - return true; -} - -[[nodiscard]] -static bool -write_rocks_ref_(IOSTREAM *s, PlAtom aref, int flags) -{ const auto ref = cast_dbref_blob_check(aref); - return write_rocks_ref_(s, *ref, flags); -} - -[[nodiscard]] -static int // TODO: bool -write_rocks_ref(IOSTREAM *s, atom_t aref, int flags) -{ return write_rocks_ref_(s, PlAtom(aref), flags); -} - /* - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - GC a rocks dbref blob from the atom garbage collector. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */ -[[nodiscard]] -static bool -release_rocks_ref_(PlAtom aref) -{ auto ref = cast_dbref_blob_check(aref); - - delete ref; - - return true; -} - -[[nodiscard]] -static int // TODO: bool -release_rocks_ref(atom_t aref) -{ return release_rocks_ref_(PlAtom(aref)); -} - -[[nodiscard]] -static bool -save_rocks_(PlAtom aref, IOSTREAM *fd) -{ const auto ref = cast_dbref_blob_check(aref); - (void)fd; - - return PL_warning("Cannot save reference to (%p)", ref); -} - -[[nodiscard]] -static int // TODO: bool -save_rocks(atom_t aref, IOSTREAM *fd) -{ return save_rocks_(PlAtom(aref), fd); -} - -[[nodiscard]] -static PlAtom -load_rocks_(IOSTREAM *fd) -{ (void)fd; - - (void)PL_warning("Cannot load reference to "); - PL_fatal_error("Cannot load reference to "); - return PlAtom(PlAtom::null); -} - -static atom_t -load_rocks(IOSTREAM *fd) -{ const auto a = load_rocks_(fd); - if ( a.not_null() ) - return a.C_; - PL_fatal_error("Cannot load reference to "); - return 0; -} - [[nodiscard]] static dbref * symbol_dbref(PlAtom symbol) -{ return cast_dbref_blob(symbol); +{ return cast_blob(symbol); } @@ -380,7 +655,7 @@ get_rocks(PlTerm t, bool throw_if_closed=true) { PlAtom a(t.as_atom()); // Throws type error if not an atom auto ref = symbol_dbref(a); - if ( ! ref ) + if ( !ref ) { a = rocks_get_alias(a); if ( a.not_null() ) ref = symbol_dbref(a); @@ -399,10 +674,10 @@ get_rocks(PlTerm t, bool throw_if_closed=true) static PlException RocksError(const rocksdb::Status& status, const dbref *ref) -{ if ( ref && ref->symbol.not_null() ) +{ if ( ref ) return PlGeneralError(PlCompound("rocks_error", PlTermv(PlTerm_atom(status.ToString()), - PlTerm_atom(ref->symbol)))); + ref->symbol_term()))); return PlGeneralError(PlCompound("rocks_error", PlTermv(PlTerm_atom(status.ToString())))); } @@ -782,8 +1057,7 @@ class PrologMergeOperator : public rocksdb::MergeOperator } }; -template -[[nodiscard]] +template [[nodiscard]] static int cmp_number(const void *v1, const void *v2) { const auto i1 = static_cast(v1); @@ -949,10 +1223,10 @@ lookup_read_optdef_and_apply(rocksdb::ReadOptions *options, { const PlAtom name; const ReadOptdefAction action; - ReadOptdef(const char *name_, ReadOptdefAction action_) - : name(name_), action(action_) { } - ReadOptdef(PlAtom atom_, ReadOptdefAction action_) - : name(atom_), action(action_) { } + ReadOptdef(const char *_name, ReadOptdefAction _action) + : name(_name), action(_action) { } + ReadOptdef(PlAtom _atom, ReadOptdefAction _action) + : name(_atom), action(_action) { } }; // TODO: #define RD_ODEF [options](PlTerm arg) @@ -1024,10 +1298,10 @@ lookup_write_optdef_and_apply(rocksdb::WriteOptions *options, { const PlAtom name; const WriteOptdefAction action; - WriteOptdef(const char *name_, WriteOptdefAction action_) - : name(name_), action(action_) { } - WriteOptdef(PlAtom atom_, WriteOptdefAction action_) - : name(atom_), action(action_) { } + WriteOptdef(const char *_name, WriteOptdefAction _action) + : name(_name), action(_action) { } + WriteOptdef(PlAtom _atom, WriteOptdefAction _action) + : name(_atom), action(_action) { } }; // TODO: #define WR_ODEF [options](PlTerm arg) @@ -1095,10 +1369,10 @@ lookup_open_optdef_and_apply(rocksdb::Options *options, { const PlAtom name; const OpenOptdefAction action; - OpenOptdef(const char *name_, OpenOptdefAction action_) - : name(name_), action(action_) { } - OpenOptdef(PlAtom atom_, OpenOptdefAction action_) - : name(atom_), action(action_) { } + OpenOptdef(const char *_name, OpenOptdefAction _action) + : name(_name), action(_action) { } + OpenOptdef(PlAtom _atom, OpenOptdefAction _action) + : name(_atom), action(_action) { } }; // TODO: #define ODEF [options](PlTerm arg) @@ -1353,6 +1627,7 @@ PREDICATE(rocks_open_, 3) // Allocating the blob uses unique_ptr so that it'll be // deleted if an error happens - this is disabled by ref.release() // before returning success. + auto ref = std::make_unique(); ref->merger = merger; ref->builtin_merger = builtin_merger; @@ -1397,7 +1672,7 @@ PREDICATE(rocks_close, 1) if ( ref->name.not_null() ) { rocks_unalias(ref->name); - // ref->name is needed by write_rocks_ref_(), so don't: ref->name.unregister_ref() + // ref->name is needed by write() callabck, so don't do this: ref->name.unregister_ref() } { auto db = ref->db; diff --git a/test/test_rocksdb.pl b/test/test_rocksdb.pl index b72f21e..fff4600 100644 --- a/test/test_rocksdb.pl +++ b/test/test_rocksdb.pl @@ -133,7 +133,7 @@ rocks_put(RocksDB, "one", "àmímé níshíkíhéꜜbì"), rocks_close(RocksDB)). -test(open_twice, [error(rocks_error(_),_), % TODO: error(rocks_error('IO error: lock hold by current process, acquire time 1657004085 acquiring thread 136471553664960: /tmp/test_rocksdb/LOCK: No locks available'),_) +test(open_twice, [error(rocks_error(_,_)), % TODO: error(rocks_error('IO error: lock hold by current process, acquire time 1657004085 acquiring thread 136471553664960: /tmp/test_rocksdb/LOCK: No locks available'),_) cleanup(delete_db)]) :- test_db(Dir), setup_call_cleanup( @@ -385,6 +385,16 @@ test_db2(Dir2), rocks_close(DB), rocks_open(Dir2, DB2, [alias('DB')]). +test(compare, [setup(setup_db(Dir, DB)), + cleanup((cleanup_db(Dir, DB), + cleanup_db(Dir3, DB3)))]) :- + % This test assumes blobs get allocated in ascending order; we're + % checking that the ordering is by pathname, not by the default + % ordering of pointer. + test_db3(Dir3), + rocks_open(Dir3, DB3, []), + assertion(Dir3 @< Dir), + assertion(DB3 @< DB). :- end_tests(alias). @@ -394,6 +404,7 @@ test_db('/tmp/test_rocksdb'). test_db2('/tmp/test_rocksdb2'). +test_db3('/tmp/test_rocksd0'). delete_db :- test_db(DB), From 890875fb09a169446ad63d594b9b816e684eaa7b Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Wed, 7 Jun 2023 10:37:07 -0700 Subject: [PATCH 18/28] Version bump --- README.md | 2 +- pack.pl | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8a1baea..d5de874 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ Should do the trick. Note that this clones RocksDB and builds it the way we need the library. This requires significant disk space (1.4Gb) and takes long (several minutes on a modern machine). -This code depends on version 2 of the `SWI-cpp.h` file that is part of +This code depends on version 2 of the `SWI-cpp2.h` file that is part of SWI-Prolog version 9.1.0 and later. #### Why are you not using the pre-built librocksdb? diff --git a/pack.pl b/pack.pl index 45e202b..2f7c231 100644 --- a/pack.pl +++ b/pack.pl @@ -1,7 +1,7 @@ name(rocksdb). title('SWI-Prolog interface to RocksDB'). -version('0.13.0'). -keywords([database, embedded, nosql]). +version('0.13.1'). +keywords([database, embedded, nosql, 'RocksDB']). author('Jan Wielemaker', 'jan@swi-prolog.org'). packager( 'Jan Wielemaker', 'jan@swi-prolog.org' ). maintainer( 'Jan Wielemaker', 'jan@swi-prolog.org' ). From 072072383920f745fbe35af9b4f001f0a1024617 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sat, 10 Jun 2023 16:25:42 -0700 Subject: [PATCH 19/28] Remove comments about PL_BLOB_UNIQUE - foreign.doc will be updated Fix some typos --- cpp/rocksdb4pl.cpp | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 7c33df9..e60bc7c 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -93,7 +93,7 @@ A Prolog blob consists of five parts: object, these could be read, write, seek, etc.). \end{itemize} -For the \ctype{PL_blot_t structure, this API provides a set of +For the \ctype{PL_blob_t} structure, this API provides a set of template functions that allow easily setting up the callbacks, and allow defining the corresonding methods int he blob "contents" class. @@ -198,7 +198,7 @@ Explanation of the \ctype{my_blob} structure: \item The \exam{magic}, \exam{flags}, and \exam{name} flags are required. -\item The \examl{release} and \examl{acquire} fields are required. The +\item The \exam{release} and \exam{acquire} fields are required. The defaults given will normally suffice (see more on this in the definition of \exam{my_blob_contents}). @@ -213,7 +213,7 @@ Explanation of the \ctype{my_blob} structure: \ctype{my_blob_contents}. \item The \exam{compare} and/or \exam{write} fields may optionally be - defined, analagously to the \exam{acquire} and \examl{release} + defined, analagously to the \exam{acquire} and \exam{release} fields. If given, you must also add a corresponding \exam{compare} and/or \exam{write} method(s) to \ctype{my_blob_contents}. @@ -224,7 +224,7 @@ Explanation of the \ctype{my_blob_contents} structure: \begin{itemize} -\item \exam{PlBlob provides default methods plus a few +\item \ctype{PlBlob} provides default methods plus a few utility methods: \begin{itemize} \item Copy and move constructors are disabled, as is the @@ -297,31 +297,6 @@ Explanation of close_my_blob/1: \end{itemize} -\subsubsection{A digression on the semantics of atoms and equality} -\label{sec:cppw-blobs-equality} - -Prolog atoms differ from strings in that atom equality can be -determined simply by comparing the atom indexes without comparing the -string values. If the \const[PL_BLOB_UNIQUE} flag is set, then every -time a blob is created, it is looked up in the atom table to see if -there is another blob with the same bit pattern. If the flag is not -set, each time a blob is created, it gets a new entry in the atom -table. - -Things become a bit more complicated when we want to sort blobs. If -the user defines the \cfuncref{compare}{} function to return 0 -("equal") based on only some of the fields in the blob, then it is -possible for the \cfuncref{compare}{} to mark two blobs as equal even -if they have different indexes in the atom table. To avoid this -inconsistency, if the \cfuncref{compare}{} function determines that -the two blobs are equivalent, it should still do a final comparison -using the blob pointers to enforce a total ordering - -\cfuncref{compare_using_ptrs}{} will do that. For example, the blob -for PCRE2 regular expressions compares the patterns of the two blobs -and if they are equal, compares the blob pointers. In this way, if two -blobs have the same pattern, they will not compare as equal, so sort/2 -won't remove one of them as a duplicate. - ***/ template [[nodiscard]] From 22118523c896e7d87c3adb6f00e6c8c648a53b4c Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Sun, 11 Jun 2023 17:01:49 -0700 Subject: [PATCH 20/28] Moved blob API code to SWI-cpp2.h --- cpp/rocksdb4pl.cpp | 395 +++------------------------------------------ 1 file changed, 18 insertions(+), 377 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index e60bc7c..d0a27f3 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -47,364 +47,6 @@ #include // This could be put in a separate file -// ------------------- To be added to SWI-cpp2.h for blobs ----------- - -// TODO: these should be in a namespace - -/*** Documentation (This will go into pl2cpp2.doc) - -\subsection{Blobs} -\label{sec:cpp2-blobs} - - -Disclaimer: -The blob API for C++ is not completely general, but is designed to -make a specific use case easier to write. For other use cases, the -underlying C API can still be used. The use case is: -\begin{itemize} -\item The blob contains the foreign object (e.g., contains a - pointer to a database connection). -\item The blob is created by a predicate that makes the foreign - object and stores it (or a pointer to it) within the blob. - For example, makes a connection to a database or compiles - a regular expression into an internal form. -\item Optionally, there is a predicate that deletes the foreign object. -\item The blob will not be subclassed. -\item The blob is defined as a subclass of \ctype{PlBlob}, which - provides a number of fields and methods, of which a few - can be overridden in the blob (notably: write(), compare(), - and the destructor). -\item The blob's constructor throws an exception and cleans up - any resources if it cannot create the blob. -\item The foreign object can be deleted when the blob is deleted. -\item The blob's allocation is controlled by Prolog and its - destructor is envoked when the blob is garbage collected. -end{itemize} - -A Prolog blob consists of five parts: -\begin{itemize} -\item A \ctype{PL_blob_t} structure that defines the callbacks. -\item A "contents" structure that contains the blob. -\item A "create" or "open" predicate that unifies one of its arguments - with a newly created blob that contains the foreign object. -\item (Optionally) a "close" predicate that does the opposit of the - "create"/"open" predicate. -\item Predicates that manipulate the foreign object (e.g., for a file-like - object, these could be read, write, seek, etc.). -\end{itemize} - -For the \ctype{PL_blob_t} structure, this API provides a set of -template functions that allow easily setting up the callbacks, and -allow defining the corresonding methods int he blob "contents" class. - -For the "contents" structure, which is subclassed from \ctype{PlBlob}, -the programmer defines the various fields, a constructor that -initializes them, and a destructor. Optionally, methods can be -defined for one of more of blob \cfuncref{compare}{}, -\cfuncref{write}{}, \cfuncref{save}{}, \cfuncref{load}{} (the -\cfuncref{acquire}{} and \cfuncref{release}{} callbacks should -normally be allowed to default to what is defined in \ctype{PlBlob}. - -There is a mismatch between how Prolog does memory management (and -garbage collection) and how C++ does it. In particular, Prolog assumes -that cleanup will be done in the \cfuncref{release}{} function -associated with the blob whereas C++ typically does cleanup in a -destructor. The blob interface gets around this mismatch by providing -a default \cfuncref{release}{} function that assumes that the blob was -created using \const{PL_BLOB_NOCOPY} and manages memory using a -\ctype{std::unique_ptr}. - -The C blob interface has a flag that determines how memory is managed: -\const{PL_BLOB_NOCOPY}. If this is set, Prolog does not do a call to -cfuncref{free} when the blob is garbage collected; instead, it assumes -that the blob's cfuncref{release} function will free the memory. - -The C++ API for blobs only supports blobs with -\const{PL_BLOB_NOCOPY}.\footnote{The API can probably also support -blobs with \const{PL_BLOB_UNIQUE}, but there seems to be little -point in setting this flag for non-text blobs.} - -\subsubsection{How to define a blob using C++} -\label{sec:cpp2-blobs-howto} - -TL;DR: Use PL_BLOB_NOCOPY (or an extra level of indirection) and the -default \ctype{PlBlob} wrappers; no copy constructor, move -constructor, or assignment operator; create blob with -\exam{make_unique}, use \exam{unique_ptr::release}, do \exam{delete} -in the "release" function. - -Here is minimal sample code for creating a blob that owns a connection -to a database: - -\begin{code} -PL_blob_t my_blob = -{ .magic = PL_BLOB_MAGIC, - .flags = PL_BLOB_NOCOPY, - .name = "my_blob", - .release = blob_release, - .write = blob_write, - .acquire = blob_acquire, - .save = blob_save, - .load = blob_save -}; - -struct my_blob_contents : public PlBlob -{ DbConnection *connection; - - explicit my_blob_contents() - : DbConnection(nullptr)) { } - - ~my_blob_contents() - { if ( connection ) - { // This is similar to close_my_blob/1, except there's - // no check for an error because there's nothing we - // can do on an error because throwing a C++ exception - // inside of C code will cause a runtime error. - (void)connection->close(); - } - } -}; - -// %! create_my_blob(+Options: list, -MyBlob) is semidet. -PREDICATE(create_my_blob, 2) -{ auto ref = std::make_unique(...); - // ... fill in the fields of *ref from options in A1 ... - int rc = do_open(..., &ref->connection); - if ( !rc ) - throw PlGeneralError(PlCompound("my_blob_error", ref->symbol_term())); - PlCheckFail(A2.unify_blob(ref.get(), sizeof *ref, &my_blob)); - (void)ref.release(); - return true; - -// %! close_my_blob(+MyBlob) is det. -// % Close the connection, silently succeeding if is already -// % closed; throw an exception if something goes wrong. -PREDICATE(close_my_blob, 1) -{ auto ref = cast_blob_check(A1.as_atom()); - auto c = ref->connection; - ref->connection = nullptr; - if ( c ) - { int rc = c->close(); - delete c; - if ( !rc ) - throw PlGeneralError(PlCompound("my_blob_error", ...)); - } - return true; -} -\end{code} - -Explanation of the \ctype{my_blob} structure: -\begin{itemize} - -\item The \exam{magic}, \exam{flags}, and \exam{name} flags are required. - -\item The \exam{release} and \exam{acquire} fields are required. The - defaults given will normally suffice (see more on this in the - definition of \exam{my_blob_contents}). - -\item If the fields \exam{save} and \exam{load} are given, they - provide a default implementation that throws an error on an - attempt to save the blob (e.g., by using - \qsave_program/[1,2]). If they are omitted, the defaults for - \exam{save} and \exam{load} are used, which save the internal - form of the blob, which is probably not what you want. If you - wish to define your own \exam{save} and \exam{load}, you must - also add \exam{save} and \exam{load} methods to - \ctype{my_blob_contents}. - -\item The \exam{compare} and/or \exam{write} fields may optionally be - defined, analagously to the \exam{acquire} and \exam{release} - fields. If given, you must also add a corresponding - \exam{compare} and/or \exam{write} method(s) to - \ctype{my_blob_contents}. - -\end{itemize} - -Explanation of the \ctype{my_blob_contents} structure: - -\begin{itemize} - -\item \ctype{PlBlob} provides default methods plus a few - utility methods: - \begin{itemize} - \item Copy and move constructors are disabled, as is the - assignment operator. - \item validate() verifies that the blob is valid, assumign that - the default \cfuncref{acquire}{} has been called. - \item acquire() is used by the default \cfuncref{acquire}{} function. - \item symbol_not_null() tests whether \exam{symbol} has been set - (this will normally be the case, unless the \cfuncref{acquire}{} - function hasn't been called as part of the blob creation - process. - \item symbol_term() Creates a term the contains the blob, for - use in error terms. It is always safe to use this; if - the symbol hasn't bene set, symbol_term() returns a "var" term. - \item compare_using_ptrs() The default method of comparison, used to - break ties between otherwise equal blobs. - \item write() A default implementation that outputs \exam{(ptr)}. - \item save() Generates an error when attempting to save the blob. - \item laod() generates an error when attemptint to laod the blob. - \end{itemize} - -\item The constructor must initialize fields so that the destructor - can determine whether to delete them or not. Typically, this is - done by setting pointers to \const{nullptr} or by setting a - flag. - -\item The destructor is called by \cfuncref{release}{}, assuming you've - set \exam{my_blob::release} to \exam{blob_release}. - -\item This blob uses the default methods for compare() and write(). - You can change this by defining your own compare() and write() - methods. If you define compare(), you must add the related line - to \exam{my_blob} and you should call compare_using_ptrs() if - your comparison of the two objects shows equality (this breaks - the tie for two otherwise equal by distinct objects). - -\end{itemize} - -Explanation of open_my_blob/2: - -\begin{itemize} - -\item \exam{std::make_unique(...)} creates a - \ctype{std::unique_ptr} that is deleted when it goes out - of scope. If an exception occurs between the creation of blob - (as a \ctype{std::unique_ptr}) and the call to - \cfuncref{unify_blob}{}, the pointer will be freed (and the - \ctype{MyBlob} destructor will be called. - -\item The exception term contains the blob, using - \exam{ref->symbol_term()}. - -\item \exam{ref.release()} prevents the pointer from being - automatically deleted; it is now "owned" by Prolog and is - deleted in the \cfuncref{release}{} function (which is defined in - \ctype{PlBlob}). - -\end{itemize} - -Explanation of close_my_blob/1: - -\begin{itemize} - -\item This code is similar to ~my_blob_contents(), except it throws - an error if something goes wrong during the "close". - -\item Other predicates that use the blob access the - blob from the arguments the same way, namely - \exam{cast_blob_check(A1.as_atom())}. - -\end{itemize} - - ***/ - -template [[nodiscard]] -static C_t * -cast_blob(PlAtom aref) -{ size_t len; - PL_blob_t *type; - auto ref = static_cast(aref.blob_data(&len, &type)); - if ( ref && type == ref->blob_t_) - { assert(len == sizeof *ref); - assert(ref->validate()); - return ref; - } - return nullptr; -} - -template [[nodiscard]] -static C_t* -cast_blob_check(PlAtom aref) -{ auto ref = cast_blob(aref); - assert(ref); - return ref; -} - -template -void blob_acquire(atom_t a) -{ PlAtom a_(a); - auto data = cast_blob_check(a_); - data->acquire(a_); -} - -template [[nodiscard]] -int blob_release(atom_t a) -{ auto data = cast_blob_check(PlAtom(a)); - delete data; - return true; -} - -template [[nodiscard]] -int blob_compare(atom_t a, atom_t b) -{ const auto data = cast_blob_check(PlAtom(a)); - return data->compare(PlAtom(b)); -} - -template [[nodiscard]] -int blob_write(IOSTREAM *s, atom_t a, int flags) -{ const auto data = cast_blob_check(PlAtom(a)); - return data->write(s, flags); -} - -template [[nodiscard]] -int blob_save(atom_t a, IOSTREAM *fd) -{ const auto data = cast_blob_check(PlAtom(a)); - return data->save(fd); -} - -template [[nodiscard]] -atom_t blob_load(IOSTREAM *fd) -{ return C_t::load(fd).C_; -} - -template -class PlBlob -{ -public: - explicit PlBlob() - : blob_t_(&blob_t), - symbol_(PlAtom(PlAtom::null)) // filled in by acquire() - { } - explicit PlBlob(const PlBlob&) = delete; - explicit PlBlob(PlBlob&&) = delete; - PlBlob& operator =(const PlBlob&) = delete; - ~PlBlob() = default; - - bool validate() const { return blob_t_ == &blob_t; } - - void acquire(PlAtom _symbol) { symbol_ = _symbol; } - - bool symbol_not_null() const { return symbol_.not_null(); } - PlTerm symbol_term() const - { if ( symbol_not_null() ) - return PlTerm_atom(symbol_); - return PlTerm_var(); - } - - // TODO: document that release() is handled by the destructor - - // compare() is provided as a fall-back for when other comparison's don't work. - // It's OK to specify it; but it's the same as the default comparison - int compare_using_ptrs(PlAtom _b) const { - const auto b = cast_blob_check(_b); - return this > b ? 1 : this < b ? -1 : 0; } - - bool write(IOSTREAM *s, int flags) const { Sfprintf(s, "<%s>(%p)", blob_t.name, this); return true; } - - bool save(IOSTREAM *fd) const { return PL_warning("Cannot save reference to <%s>(%p)", blob_t.name, this); } - - static PlAtom load(IOSTREAM *fd) - { (void)PL_warning("Cannot load reference to <%s>", blob_t.name); - PL_fatal_error("Cannot load reference to <%s>", blob_t.name); - return PlAtom(PlAtom::null); - } - - const PL_blob_t *blob_t_; - PlAtom symbol_; /* associated symbol (used for error terms) */ -}; - -// ------------------- end: To be added to SWI-cpp2.h for blobs ----------- [[nodiscard]] static PlAtom rocks_get_alias(PlAtom name); [[nodiscard]] static PlAtom rocks_get_alias_inside_lock(PlAtom name); @@ -459,7 +101,6 @@ struct dbref_type_kv struct dbref; - static PL_blob_t rocks_blob = { .magic = PL_BLOB_MAGIC, .flags = PL_BLOB_NOCOPY, @@ -485,35 +126,35 @@ struct dbref : PlBlob .value = BLOB_ATOM}) { } - bool write(IOSTREAM *s, int flags) const /* override */ - { Sfprintf(s, "<%s>(%p", blob_t_->name, this); - if ( pathname.not_null() ) - Sfprintf(s, ",path=%Ws", pathname.as_wstring().c_str()); + bool write_fields(IOSTREAM *s, int flags) const override + { if ( pathname.not_null() ) + if ( !Sfprintf(s, ",path=%Ws", pathname.as_wstring().c_str()) ) + return false; if ( name.not_null() ) - Sfprintf(s, ",alias=%Ws", name.as_wstring().c_str()); + if ( !Sfprintf(s, ",alias=%Ws", name.as_wstring().c_str()) ) + return false; if (builtin_merger != MERGE_NONE) - Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]); + if ( !Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]) ) + return false; if ( merger.not_null() ) { auto m(merger.term()); if ( m.not_null() ) - Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()); + if ( !Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()) ) + return false; } if ( !db ) - Sfprintf(s, ",CLOSED"); - Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); - if ( flags&PL_WRT_NEWLINE ) - Sfprintf(s, "\n"); - return true; + if ( !Sfprintf(s, ",CLOSED") ) + return false; + return Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); } - int compare(PlAtom _b) const - { const auto b = cast_blob_check(_b); - if ( this == b ) // Prolog should have already done this test, but it's cheap - return 0; - int c_pathname = PlTerm_atom(pathname).compare(PlTerm_atom(b->pathname)); + int compare_fields(const PlBlob* _b_data) const override + { // dynamic_cast is safer, but slower: + auto b_data = static_cast(_b_data); + int c_pathname = PlTerm_atom(pathname).compare(PlTerm_atom(b_data->pathname)); if ( c_pathname != 0 ) return c_pathname; - return compare_using_ptrs(_b); + return 0; } ~dbref() From d45f9fe81ee43968ed98ad10716337de9a6b7c21 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Mon, 3 Jul 2023 17:34:05 -0700 Subject: [PATCH 21/28] Use improved blob interface --- cpp/rocksdb4pl.cpp | 44 ++++++++++++++++++++++---------------------- prolog/rocksdb.pl | 6 +++--- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index d0a27f3..9f0bbba 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -116,22 +116,27 @@ static PL_blob_t rocks_blob = struct dbref : PlBlob { - dbref() - : db( nullptr), - pathname( PlAtom(PlAtom::null)), - name( PlAtom(PlAtom::null)), - builtin_merger( MERGE_NONE), - merger( PlRecord(PlRecord::null)), - type( { .key = BLOB_ATOM, - .value = BLOB_ATOM}) - { } + rocksdb::DB *db = nullptr; // DB handle + PlAtom pathname = PlAtom(PlAtom::null); // DB's absolute file name (for debugging) + PlAtom name = PlAtom(PlAtom::null); // alias name (can be PlAtom::null) + merger_t builtin_merger = MERGE_NONE; // C++ Merger + PlRecord merger = PlRecord(PlRecord::null); // merge option + dbref_type_kv type = { // key and value types + .key = BLOB_ATOM, + .value = BLOB_ATOM}; + + explicit dbref() { } + + virtual size_t blob_size_() const override { return sizeof *this; } bool write_fields(IOSTREAM *s, int flags) const override { if ( pathname.not_null() ) - if ( !Sfprintf(s, ",path=%Ws", pathname.as_wstring().c_str()) ) + if ( !Sfprintf(s, ",path=") || + !pathname.write(s, flags) ) return false; if ( name.not_null() ) - if ( !Sfprintf(s, ",alias=%Ws", name.as_wstring().c_str()) ) + if ( !Sfprintf(s, ",alias=") || + !name.write(s, flags) ) return false; if (builtin_merger != MERGE_NONE) if ( !Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]) ) @@ -139,8 +144,10 @@ struct dbref : PlBlob if ( merger.not_null() ) { auto m(merger.term()); if ( m.not_null() ) - if ( !Sfprintf(s, ",merger=%Ws", m.as_wstring().c_str()) ) + { if ( !Sfprintf(s, ",merger=") || + !m.write(s, 1200, flags) ) return false; + } } if ( !db ) if ( !Sfprintf(s, ",CLOSED") ) @@ -172,13 +179,6 @@ struct dbref : PlBlob delete db; } } - - rocksdb::DB *db; /* DB handle */ - PlAtom pathname; /* DB's absolute file name (for debugging) */ - PlAtom name; /* alias name (can be PlAtom::null) */ - merger_t builtin_merger; /* C++ Merger */ - PlRecord merger; /* merge option */ - dbref_type_kv type; /* key and value types */ }; @@ -1184,7 +1184,7 @@ lookup_open_optdef_and_apply(rocksdb::Options *options, PREDICATE(rocks_open_, 3) { rocksdb::Options options; - options.create_if_missing = true; + options.create_if_missing = true; // caller can override this default value char *fn; // from A1 - assumes that it's already absolute file name blob_type key_type = BLOB_ATOM; blob_type value_type = BLOB_ATOM; @@ -1265,10 +1265,10 @@ PREDICATE(rocks_open_, 3) ref.get()); if ( ref->name.is_null() ) - { PlCheckFail(A2.unify_blob(ref.get(), sizeof *ref, &rocks_blob)); + { PlCheckFail(A2.unify_blob(ref.get())); } else { PlTerm_var tmp; - PlCheckFail(tmp.unify_blob(ref.get(), sizeof *ref, &rocks_blob)); + PlCheckFail(tmp.unify_blob(ref.get())); rocks_add_alias(ref->name, tmp.as_atom()); PlCheckFail(A2.unify_atom(ref->name)); } diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index acaac59..cf115c9 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -217,9 +217,9 @@ % rocksdb library, the error term is of the form % rocks_error(Message) or rocks_error(Message,Blob). % -% Most of the `DBOptions` in -% `rocksdb/include/rocksdb/options.h` are supported, in addition -% to the following options: +% Most of the `DBOptions` in `rocksdb/include/rocksdb/options.h` +% are supported. `create_if_exists` defaults to `true`. +% Additional options are: % % - alias(+Name) % Give the database a name instead of using an anonymous From 270bce579de975b2ce1dfa21ee9374c0617d4ace Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Tue, 4 Jul 2023 10:22:14 -0700 Subject: [PATCH 22/28] Use new convenience macro for defining blobs --- cpp/rocksdb4pl.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 9f0bbba..a012beb 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -101,17 +101,7 @@ struct dbref_type_kv struct dbref; -static PL_blob_t rocks_blob = -{ .magic = PL_BLOB_MAGIC, - .flags = PL_BLOB_NOCOPY, - .name = "rocksdb", - .release = blob_release, - .compare = blob_compare, - .write = blob_write, - .acquire = blob_acquire, - .save = blob_save, - .load = blob_load -}; +static PL_blob_t rocks_blob = PL_BLOB_DEFINITION(dbref, "rocksdb"); struct dbref : PlBlob From 4e04dfe1a2141b883f1dec0cc898b1ec4416654c Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Tue, 4 Jul 2023 16:45:30 -0700 Subject: [PATCH 23/28] PlBlob no longer uses a template --- cpp/rocksdb4pl.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index a012beb..ad6eb02 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -103,8 +103,7 @@ struct dbref; static PL_blob_t rocks_blob = PL_BLOB_DEFINITION(dbref, "rocksdb"); - -struct dbref : PlBlob +struct dbref : public PlBlob { rocksdb::DB *db = nullptr; // DB handle PlAtom pathname = PlAtom(PlAtom::null); // DB's absolute file name (for debugging) @@ -115,9 +114,10 @@ struct dbref : PlBlob .key = BLOB_ATOM, .value = BLOB_ATOM}; - explicit dbref() { } + explicit dbref() : + PlBlob(&rocks_blob) { } - virtual size_t blob_size_() const override { return sizeof *this; } + PL_BLOB_SIZE bool write_fields(IOSTREAM *s, int flags) const override { if ( pathname.not_null() ) @@ -145,7 +145,7 @@ struct dbref : PlBlob return Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); } - int compare_fields(const PlBlob* _b_data) const override + int compare_fields(const PlBlob* _b_data) const override { // dynamic_cast is safer, but slower: auto b_data = static_cast(_b_data); int c_pathname = PlTerm_atom(pathname).compare(PlTerm_atom(b_data->pathname)); @@ -1231,8 +1231,8 @@ PREDICATE(rocks_open_, 3) } // Allocating the blob uses unique_ptr so that it'll be - // deleted if an error happens - this is disabled by ref.release() - // before returning success. + // deleted if an error happens - the auto-deletion is disabled by + // ref.release() before returning success. auto ref = std::make_unique(); ref->merger = merger; From ca308ffede7e0e50c62f8c64b04f0e1d08e264d3 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Tue, 4 Jul 2023 19:01:40 -0700 Subject: [PATCH 24/28] Put blob "vtable" functions into a class --- cpp/rocksdb4pl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index ad6eb02..37fe5fb 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -251,7 +251,7 @@ GC a rocks dbref blob from the atom garbage collector. [[nodiscard]] static dbref * symbol_dbref(PlAtom symbol) -{ return cast_blob(symbol); +{ return PlBlobV::cast(symbol); } From d71b46b6cd111523ebc81c7b4c1c1a3ea1046417 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Thu, 13 Jul 2023 12:39:43 -0700 Subject: [PATCH 25/28] Catch C++ exceptions in blob callbacks --- cpp/rocksdb4pl.cpp | 60 ++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 37fe5fb..0c44a8f 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -120,38 +120,46 @@ struct dbref : public PlBlob PL_BLOB_SIZE bool write_fields(IOSTREAM *s, int flags) const override - { if ( pathname.not_null() ) - if ( !Sfprintf(s, ",path=") || - !pathname.write(s, flags) ) - return false; - if ( name.not_null() ) - if ( !Sfprintf(s, ",alias=") || - !name.write(s, flags) ) - return false; - if (builtin_merger != MERGE_NONE) - if ( !Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]) ) - return false; - if ( merger.not_null() ) - { auto m(merger.term()); - if ( m.not_null() ) - { if ( !Sfprintf(s, ",merger=") || - !m.write(s, 1200, flags) ) - return false; - } + { try + { if ( pathname.not_null() ) + if ( !Sfprintf(s, ",path=") || + !pathname.write(s, flags) ) + return false; + if ( name.not_null() ) + if ( !Sfprintf(s, ",alias=") || + !name.write(s, flags) ) + return false; + if (builtin_merger != MERGE_NONE) + if ( !Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]) ) + return false; + if ( merger.not_null() ) + { auto m(merger.term()); + if ( m.not_null() ) + { if ( !Sfprintf(s, ",merger=") || + !m.write(s, 1200, flags) ) + return false; + } + } + if ( !db ) + if ( !Sfprintf(s, ",CLOSED") ) + return false; + return Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); + } catch ( const PlExceptionBase& ) + { return false; } - if ( !db ) - if ( !Sfprintf(s, ",CLOSED") ) - return false; - return Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); } int compare_fields(const PlBlob* _b_data) const override { // dynamic_cast is safer, but slower: auto b_data = static_cast(_b_data); - int c_pathname = PlTerm_atom(pathname).compare(PlTerm_atom(b_data->pathname)); - if ( c_pathname != 0 ) - return c_pathname; - return 0; + try + { int c_pathname = PlTerm_atom(pathname).compare(PlTerm_atom(b_data->pathname)); + if ( c_pathname != 0 ) + return c_pathname; + return 0; + } catch ( const PlExceptionBase& ) + { return 0; // TODO: signal an error? + } } ~dbref() From 2142533c99fb560d4a082bb6b7cb823558eb0856 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Tue, 8 Aug 2023 14:14:41 -0700 Subject: [PATCH 26/28] Fix close and error handling; use new blob_unify w/unique_ptr --- cpp/rocksdb4pl.cpp | 136 +++++++++++++++++++++------------------------ prolog/rocksdb.pl | 7 +-- 2 files changed, 67 insertions(+), 76 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 0c44a8f..9748a91 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -48,12 +48,15 @@ #include // This could be put in a separate file +struct dbref; [[nodiscard]] static PlAtom rocks_get_alias(PlAtom name); [[nodiscard]] static PlAtom rocks_get_alias_inside_lock(PlAtom name); static void rocks_add_alias(PlAtom name, PlAtom symbol); static void rocks_add_alias_inside_lock(PlAtom name, PlAtom symbol); static void rocks_unalias(PlAtom name); static void rocks_unalias_inside_lock(PlAtom name); +static bool ok(const rocksdb::Status& status, const dbref *ref); +static void ok_or_throw_fail(const rocksdb::Status& status, const dbref *ref); /******************************* * SYMBOL * @@ -99,17 +102,18 @@ struct dbref_type_kv }; -struct dbref; - static PL_blob_t rocks_blob = PL_BLOB_DEFINITION(dbref, "rocksdb"); struct dbref : public PlBlob { rocksdb::DB *db = nullptr; // DB handle - PlAtom pathname = PlAtom(PlAtom::null); // DB's absolute file name (for debugging) + PlAtom pathname = PlAtom(PlAtom::null); // DB's absolute file name PlAtom name = PlAtom(PlAtom::null); // alias name (can be PlAtom::null) + std::string pathname_s; + std::string name_s; merger_t builtin_merger = MERGE_NONE; // C++ Merger PlRecord merger = PlRecord(PlRecord::null); // merge option + bool debug = false; dbref_type_kv type = { // key and value types .key = BLOB_ATOM, .value = BLOB_ATOM}; @@ -120,33 +124,29 @@ struct dbref : public PlBlob PL_BLOB_SIZE bool write_fields(IOSTREAM *s, int flags) const override - { try - { if ( pathname.not_null() ) - if ( !Sfprintf(s, ",path=") || - !pathname.write(s, flags) ) - return false; - if ( name.not_null() ) - if ( !Sfprintf(s, ",alias=") || - !name.write(s, flags) ) - return false; - if (builtin_merger != MERGE_NONE) - if ( !Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]) ) - return false; - if ( merger.not_null() ) - { auto m(merger.term()); - if ( m.not_null() ) - { if ( !Sfprintf(s, ",merger=") || - !m.write(s, 1200, flags) ) - return false; - } - } - if ( !db ) - if ( !Sfprintf(s, ",CLOSED") ) - return false; - return Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]); - } catch ( const PlExceptionBase& ) - { return false; + { if ( ! debug ) + return true; + if ( !pathname_s.empty() ) + if ( Sfprintf(s, ",path=%s", pathname_s.c_str()) < 0 ) + return false; + if ( !name_s.empty() ) + if ( Sfprintf(s, ",alias=%s", name_s.c_str()) < 0 ) + return false; + if (builtin_merger != MERGE_NONE) + if ( Sfprintf(s, ",builtin_merger=%s", merge_t_char[builtin_merger]) < 0 ) + return false; + if ( merger.not_null() ) + { auto m(merger.term()); + if ( m.not_null() ) + { if ( Sfprintf(s, ",merger=") < 0 || + !m.write(s, 1200, flags) ) + return false; + } } + if ( !db ) + if ( Sfprintf(s, ",CLOSED") < 0 ) + return false; + return Sfprintf(s, ",key=%s,value=%s)", blob_type_char[type.key], blob_type_char[type.value]) >= 0; } int compare_fields(const PlBlob* _b_data) const override @@ -162,20 +162,31 @@ struct dbref : public PlBlob } } - ~dbref() - { // See also predicate rocks_close/1 + bool close() noexcept + { bool rc = true; + if ( db ) + { rc = ok(db->Close(), this); + db = nullptr; + } + if ( pathname.not_null() ) + { pathname.unregister_ref(); + pathname.reset(); + } if ( name.not_null() ) { rocks_unalias(name); name.unregister_ref(); + name.reset(); } if ( merger.not_null() ) - merger.erase(); - if ( pathname.not_null() ) - pathname.unregister_ref(); - if ( db ) - { (void)db->Close(); // TODO: print warning on failure? - delete db; + { merger.erase(); + merger.reset(); } + return rc; + } + + ~dbref() + { if ( !close() ) + Sdprintf("*** ERROR: Close rocksdb failed: %s\n", pathname_s.c_str()); // Can't use PL_warning() } }; @@ -251,28 +262,16 @@ rocks_unalias(PlAtom name) *******************************/ -/* - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -GC a rocks dbref blob from the atom garbage collector. -- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */ - - -[[nodiscard]] -static dbref * -symbol_dbref(PlAtom symbol) -{ return PlBlobV::cast(symbol); -} - - [[nodiscard]] static dbref * get_rocks(PlTerm t, bool throw_if_closed=true) { PlAtom a(t.as_atom()); // Throws type error if not an atom - auto ref = symbol_dbref(a); + auto ref = PlBlobV::cast(a); if ( !ref ) { a = rocks_get_alias(a); if ( a.not_null() ) - ref = symbol_dbref(a); + ref = PlBlobV::cast(a); } if ( throw_if_closed && ( !ref || !ref->db ) ) @@ -1190,6 +1189,7 @@ PREDICATE(rocks_open_, 3) PlAtom alias(PlAtom::null); PlRecord merger(PlRecord::null); bool read_only = false; + bool debug = false; static const PlAtom ATOM_key("key"); static const PlAtom ATOM_value("value"); @@ -1199,6 +1199,7 @@ PREDICATE(rocks_open_, 3) static const PlAtom ATOM_mode("mode"); static const PlAtom ATOM_read_write("read_write"); static const PlAtom ATOM_read_only("read_only"); + static const PlAtom ATOM_debug("debug"); PlCheckFail(Plx_get_file_name(A1.C_, &fn, PL_FILE_OSPATH)); PlTerm_tail tail(A3); @@ -1225,6 +1226,8 @@ PREDICATE(rocks_open_, 3) read_only = true; else throw PlDomainError("mode_option", opt[1]); + } else if (ATOM_debug == name ) + { debug = opt[1].as_bool(); } else { lookup_open_optdef_and_apply(&options, name, opt); } @@ -1252,6 +1255,9 @@ PREDICATE(rocks_open_, 3) ref->name = alias; if ( ref->name.not_null() ) ref->name.register_ref(); + ref->pathname_s = fn; + ref->name_s = alias.not_null() ? alias.as_string() : ""; + ref->debug = debug; if ( ref->merger.not_null() ) options.merge_operator.reset(new PrologMergeOperator(*ref)); @@ -1263,15 +1269,15 @@ PREDICATE(rocks_open_, 3) ref.get()); if ( ref->name.is_null() ) - { PlCheckFail(A2.unify_blob(ref.get())); + { std::unique_ptr refb(ref.release()); + PlCheckFail(A2.unify_blob(&refb)); } else { PlTerm_var tmp; - PlCheckFail(tmp.unify_blob(ref.get())); - rocks_add_alias(ref->name, tmp.as_atom()); - PlCheckFail(A2.unify_atom(ref->name)); + std::unique_ptr refb(ref.release()); + PlCheckFail(tmp.unify_blob(&refb)); + rocks_add_alias(alias, tmp.as_atom()); + PlCheckFail(A2.unify_atom(alias)); } - - (void)ref.release(); // ref now owned by Prolog, deleted in release_rocks_ref_() return true; } @@ -1280,21 +1286,7 @@ PREDICATE(rocks_close, 1) { const auto ref = get_rocks(A1, false); if ( !ref ) return true; - - // The following code is a subset of dbref::~dbref(), and also can - // throw a Prolog error if ref->db->Close() fails. - - if ( ref->name.not_null() ) - { rocks_unalias(ref->name); - // ref->name is needed by write() callabck, so don't do this: ref->name.unregister_ref() - } - - { auto db = ref->db; - ref->db = nullptr; - if ( db ) - ok_or_throw_fail(db->Close(), ref); - } - + PlCheckFail(ref->close()); return true; } diff --git a/prolog/rocksdb.pl b/prolog/rocksdb.pl index cf115c9..e8f2752 100644 --- a/prolog/rocksdb.pl +++ b/prolog/rocksdb.pl @@ -72,6 +72,7 @@ float,double,term])), value(any), merge(callable), + debug(boolean), prepare_for_bulk_load(oneof([true])), optimize_for_small_db(oneof([true])), increase_parallelism(oneof([true])), @@ -279,10 +280,8 @@ % to have multiple `read_only` opens, but only one % `read_write` (which also precludes having any `read_only`); % however, it is recommended to only open a databse once. -% - optimize_for_small_db(true) - Use this if your DB is very -% small (like under 1GB) and you don't want to -% spend lots of memory for memtables. -% - increase_parallelism(true) - see DBOptions::IncreaseParallelism() +% - debug(true) Output more information when displaying +% the rocksdb "blob". % @see https://github.com/facebook/rocksdb/wiki/RocksDB-Tuning-Guide % @see http://rocksdb.org/blog/2018/08/01/rocksdb-tuning-advisor.html % @see https://github.com/EighteenZi/rocksdb_wiki/blob/master/RocksDB-Tuning-Guide.md From 98a6e70efa522f636d5b9aaddfabe9fe217a252f Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Thu, 10 Aug 2023 11:25:01 -0700 Subject: [PATCH 27/28] Replace PlForeignContextPtr by PlControl::context_unique_ptr --- cpp/rocksdb4pl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/rocksdb4pl.cpp b/cpp/rocksdb4pl.cpp index 9748a91..ab91b4b 100644 --- a/cpp/rocksdb4pl.cpp +++ b/cpp/rocksdb4pl.cpp @@ -1446,11 +1446,11 @@ enum_key_prefix(const enum_state& state) [[nodiscard]] static foreign_t rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::ReadOptions options) -{ PlForeignContextPtr state(handle); +{ auto state = handle.context_unique_ptr(); switch ( handle.foreign_control() ) { case PL_FIRST_CALL: - state.set(new enum_state(get_rocks(A1))); + state.reset(new enum_state(get_rocks(A1))); if ( ac >= 4 ) { if ( !(state->ref->type.key == BLOB_ATOM || state->ref->type.key == BLOB_STRING || @@ -1478,7 +1478,7 @@ rocks_enum(PlTermv PL_av, int ac, enum_type type, PlControl handle, rocksdb::Rea state->ref->builtin_merger, state->ref->type.value) ) { state->it->Next(); if ( state->it->Valid() && enum_key_prefix(*state) ) - { PL_retry_address(state.keep()); + { PL_retry_address(state.release()); } else { return true; } From b20a4066d1aae20d721a0a33f1dc26db5e80c6f3 Mon Sep 17 00:00:00 2001 From: Peter Ludemann Date: Thu, 10 Aug 2023 11:36:45 -0700 Subject: [PATCH 28/28] Sync to rocksdb v8.3.2 --- rocksdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rocksdb b/rocksdb index 6a43615..3f7c92b 160000 --- a/rocksdb +++ b/rocksdb @@ -1 +1 @@ -Subproject commit 6a436150417120a3f9732d65a2a5c2b8d19b60fc +Subproject commit 3f7c92b9753b697ce6a5ea737086d2751f17956c