From 12bd267efce216a72d621821db6b7acf95217067 Mon Sep 17 00:00:00 2001 From: Daniel Kolesa Date: Thu, 21 Apr 2022 04:08:44 +0200 Subject: [PATCH] implement atomic variable storage Variable values are global, and can be accessed concurrently from different cubescript threads. Make storage of var values atomic so that it is safe to do so. This is done with a simple load-store wrapper. It does not do any other operations on the values, so we can handle floats and pointers this way as well. Floats are simply stored in the smallest integer type that can fully fit the float. --- include/cubescript/cubescript/value.hh | 1 + include/cubescript/cubescript_conf.hh | 5 + src/cs_ident.cc | 163 ++++++++++++++++++++++--- src/cs_ident.hh | 54 +++++++- src/cs_state.cc | 11 +- 5 files changed, 205 insertions(+), 29 deletions(-) diff --git a/include/cubescript/cubescript/value.hh b/include/cubescript/cubescript/value.hh index 9c93289..68211d6 100644 --- a/include/cubescript/cubescript/value.hh +++ b/include/cubescript/cubescript/value.hh @@ -166,6 +166,7 @@ private: */ struct LIBCUBESCRIPT_EXPORT string_ref { friend struct any_value; + friend struct var_value; friend struct string_pool; /** @brief String references are not default-constructible. */ diff --git a/include/cubescript/cubescript_conf.hh b/include/cubescript/cubescript_conf.hh index 5ef430a..22b87ba 100644 --- a/include/cubescript/cubescript_conf.hh +++ b/include/cubescript/cubescript_conf.hh @@ -53,6 +53,8 @@ namespace cubescript { * Define `LIBCUBESCRIPT_CONF_USER_FLOAT` in your custom conf file * to disable the builtin. * + * Must be at most as large as the largest standard integer type. + * * @see integer_type * @see FLOAT_FORMAT * @see ROUND_FLOAT_FORMAT @@ -139,6 +141,9 @@ static_assert( static_assert( std::is_floating_point_v, "float_type must be floating point" ); +static_assert( + sizeof(float_type) <= sizeof(unsigned long long), "float_type is too large" +); } /* namespace cubescript */ diff --git a/src/cs_ident.cc b/src/cs_ident.cc index 7237bb1..2f17eb7 100644 --- a/src/cs_ident.cc +++ b/src/cs_ident.cc @@ -4,9 +4,138 @@ #include "cs_thread.hh" #include "cs_vm.hh" #include "cs_error.hh" +#include "cs_strman.hh" + +#include namespace cubescript { +template +static inline T var_load(unsigned char const *base) noexcept { + std::atomic const *p{}; + std::memcpy(&p, &base, sizeof(void *)); + return p->load(); +} + +template +static inline void var_store(unsigned char *base, T v) noexcept { + std::atomic *p{}; + std::memcpy(&p, &base, sizeof(void *)); + p->store(v); +} + +/* the ctors are non-atomic; that's okay, these are called during ident + * creation before anything is stored, so there is no chance of data race + */ + +var_value::var_value(integer_type v): p_type{value_type::INTEGER} { + new (p_stor) std::atomic{v}; + new (p_ostor) std::atomic{0}; +} + +var_value::var_value(float_type v): p_type{value_type::FLOAT} { + FS vs{}; + std::memcpy(&vs, &v, sizeof(v)); + new (p_stor) std::atomic{vs}; + new (p_ostor) std::atomic{0}; +} + +var_value::var_value(std::string_view const &v, state &cs): + p_type{value_type::STRING} +{ + new (p_stor) std::atomic{ + state_p{cs}.ts().istate->strman->add(v) + }; + new (p_ostor) std::atomic{nullptr}; +} + +var_value::~var_value() { + if (type() == value_type::STRING) { + str_managed_unref(var_load(p_stor)); + } +} + +static inline void var_save( + var_value const &v, unsigned char *top, unsigned char *fromp +) noexcept { + switch (v.type()) { + case value_type::INTEGER: + var_store(top, var_load(fromp)); + var_store(fromp, 0); + return; + case value_type::FLOAT: { + using FST = typename var_value::FS; + FST vs{}; + float_type fv = 0; + std::memcpy(&vs, &fv, sizeof(fv)); + var_store(top, var_load(fromp)); + var_store(fromp, vs); + return; + } + case value_type::STRING: { + auto *p = var_load(top); + if (p) { + str_managed_unref(p); + } + var_store(top, var_load(fromp)); + var_store(fromp, nullptr); + } + default: + break; + } + abort(); /* unreachable unless buggy */ +} + +void var_value::save() { + var_save(*this, p_ostor, p_stor); +} + +void var_value::restore() { + var_save(*this, p_stor, p_ostor); +} + +void var_value::steal_value(any_value &v, state &cs) { + switch (type()) { + case value_type::INTEGER: + var_store(p_stor, v.force_integer()); + return; + case value_type::FLOAT: { + FS vs{}; + float_type fv = v.force_float(); + std::memcpy(&vs, &fv, sizeof(fv)); + var_store(p_stor, vs); + return; + } + case value_type::STRING: { + auto sv = v.force_string(cs); + var_store(p_stor, str_managed_ref(sv.data())); + return; + } + default: + break; + } + abort(); /* unreachable unless buggy */ +} + +any_value var_value::to_value() const { + switch (type()) { + case value_type::INTEGER: + return var_load(p_stor); + case value_type::FLOAT: { + float_type fv{}; + FS vs = var_load(p_stor); + std::memcpy(&fv, &vs, sizeof(fv)); + return fv; + } + case value_type::STRING: + return string_ref{var_load(p_stor)}; + default: + break; + } + abort(); /* unreachable unless buggy */ + return any_value{}; +} + ident_impl::ident_impl(ident_type tp, string_ref nm, int fl): p_name{nm}, p_type{int(tp)}, p_flags{fl} {} @@ -19,8 +148,18 @@ bool ident_is_callable(ident const *id) { return !!static_cast(id)->p_cb_cftv; } -var_impl::var_impl(string_ref name, int fl): - ident_impl{ident_type::VAR, name, fl} +var_impl::var_impl(string_ref name, int fl, integer_type v): + ident_impl{ident_type::VAR, name, fl}, p_storage{v} +{} + +var_impl::var_impl(string_ref name, int fl, float_type v): + ident_impl{ident_type::VAR, name, fl}, p_storage{v} +{} + +var_impl::var_impl( + string_ref name, int fl, std::string_view const &v, state &cs +): + ident_impl{ident_type::VAR, name, fl}, p_storage{v, cs} {} alias_impl::alias_impl( @@ -237,7 +376,7 @@ LIBCUBESCRIPT_EXPORT void builtin_var::save(state &cs) { } if (!(p_impl->p_flags & IDENT_FLAG_OVERRIDDEN)) { auto *imp = static_cast(p_impl); - imp->p_override = std::move(imp->p_storage); + imp->p_storage.save(); p_impl->p_flags |= IDENT_FLAG_OVERRIDDEN; } } else { @@ -267,27 +406,13 @@ LIBCUBESCRIPT_EXPORT any_value builtin_var::call( } LIBCUBESCRIPT_EXPORT any_value builtin_var::value() const { - return static_cast(p_impl)->p_storage; + return static_cast(p_impl)->p_storage.to_value(); } LIBCUBESCRIPT_EXPORT void builtin_var::set_raw_value( state &cs, any_value val ) { - switch (static_cast(p_impl)->p_storage.type()) { - case value_type::INTEGER: - val.force_integer(); - break; - case value_type::FLOAT: - val.force_float(); - break; - case value_type::STRING: - val.force_string(cs); - break; - default: - abort(); /* unreachable unless we have a bug */ - break; - } - static_cast(p_impl)->p_storage = std::move(val); + static_cast(p_impl)->p_storage.steal_value(val, cs); } LIBCUBESCRIPT_EXPORT void builtin_var::set_value( diff --git a/src/cs_ident.hh b/src/cs_ident.hh index 89db278..11c8f3a 100644 --- a/src/cs_ident.hh +++ b/src/cs_ident.hh @@ -4,12 +4,59 @@ #include #include +#include +#include namespace cubescript { static constexpr std::size_t MAX_ARGUMENTS = 32; using argset = std::bitset; +struct var_value { + /* ugly but does the trick */ + using FS = std::conditional_t< + sizeof(float_type) == sizeof(unsigned char), + unsigned char, + std::conditional_t< + sizeof(float_type) == sizeof(unsigned short), + unsigned short, + std::conditional_t< + sizeof(float_type) == sizeof(unsigned long), + unsigned long, + unsigned long long + > + > + >; + + var_value(integer_type v); + var_value(float_type v); + var_value(std::string_view const &v, state &cs); + + ~var_value(); + + value_type type() const { + return p_type; + } + + void save(); + void restore(); + + void steal_value(any_value &v, state &cs); + any_value to_value() const; + +private: + using VU = union { + std::atomic i; + std::atomic f; + std::atomic s; + }; + + /* fixed upon creation */ + value_type p_type; + alignas(VU) unsigned char p_stor[sizeof(VU)]; + alignas(VU) unsigned char p_ostor[sizeof(VU)]; +}; + enum { ID_UNKNOWN = -1, ID_VAR, ID_COMMAND, ID_ALIAS, ID_LOCAL, ID_DO, ID_DOARGS, ID_IF, ID_BREAK, ID_CONTINUE, ID_RESULT, @@ -68,12 +115,13 @@ struct ident_impl { bool ident_is_callable(ident const *id); struct var_impl: ident_impl, builtin_var { - var_impl(string_ref name, int flags); + var_impl(string_ref name, int flags, integer_type v); + var_impl(string_ref name, int flags, float_type v); + var_impl(string_ref name, int flags, std::string_view const &v, state &cs); command *get_setter(thread_state &ts) const; - any_value p_storage{}; - any_value p_override{}; + var_value p_storage; }; void var_changed(thread_state &ts, builtin_var &id, any_value &oldval); diff --git a/src/cs_state.cc b/src/cs_state.cc index 4cbb272..dce45fd 100644 --- a/src/cs_state.cc +++ b/src/cs_state.cc @@ -398,7 +398,7 @@ LIBCUBESCRIPT_EXPORT void state::clear_override(ident &id) { case ident_type::VAR: { auto &v = static_cast(id); any_value oldv = v.value(); - v.p_storage = std::move(v.p_override); + v.p_storage.restore(); var_changed(*p_tstate, v, oldv); static_cast( static_cast(&v) @@ -453,9 +453,8 @@ LIBCUBESCRIPT_EXPORT builtin_var &state::new_var( std::string_view n, integer_type v, bool read_only, var_type vtp ) { auto *iv = p_tstate->istate->create( - string_ref{*this, n},var_flags(read_only, vtp) + string_ref{*this, n}, var_flags(read_only, vtp), v ); - iv->p_storage.set_integer(v); try { var_name_check(*this, p_tstate->istate->get_ident(n), n); } catch (...) { @@ -470,9 +469,8 @@ LIBCUBESCRIPT_EXPORT builtin_var &state::new_var( std::string_view n, float_type v, bool read_only, var_type vtp ) { auto *fv = p_tstate->istate->create( - string_ref{*this, n}, var_flags(read_only, vtp) + string_ref{*this, n}, var_flags(read_only, vtp), v ); - fv->p_storage.set_float(v); try { var_name_check(*this, p_tstate->istate->get_ident(n), n); } catch (...) { @@ -487,9 +485,8 @@ LIBCUBESCRIPT_EXPORT builtin_var &state::new_var( std::string_view n, std::string_view v, bool read_only, var_type vtp ) { auto *sv = p_tstate->istate->create( - string_ref{*this, n}, var_flags(read_only, vtp) + string_ref{*this, n}, var_flags(read_only, vtp), v, *this ); - sv->p_storage.set_string(v, *this); try { var_name_check(*this, p_tstate->istate->get_ident(n), n); } catch (...) {