Skip to content

Commit

Permalink
config: Add ConfObject::get() that accepts string literals
Browse files Browse the repository at this point in the history 
Almost all ConfObject::get() calls pass string literals, which are de-duplicated
when placed in the rodata section with the string literals that were used to construct
the ConfObject. Pass these strings through to the getter and use them for a fast address
comparison instead of doing a slower string comparison on every key in the ConfObject.

Unfortunately, the overloaded get(const char *) causes a conflict with ConfArray::get(size_t)
because any use like get(1) will look for an int variant of get(), which will also match
the const char * variant. Add several get() overloads for various integer types to weed out
the undesirable ones. The int variant is also undesirable, but unfortunately required
for get(1) to work. All get-wrappers can be inlined and optimized away.

Also change the ConfirmKey convenience function to const char *, so that the optimized
string address comparison works with it as well.
  • Loading branch information
@MattiasTF
MattiasTF committed Oct 17, 2024
1 parent a7738e5 commit 076fd08
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 26 deletions.
27 changes: 16 additions & 11 deletions software/src/config.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,6 @@ ConfigRoot *Config::Confirm()
return &confirmconf;
}

String Config::ConfirmKey()
{
return Config::confirm_key;
}

Config Config::Uint8(uint8_t u)
{
return Config::Uint(u, std::numeric_limits<uint8_t>::lowest(), std::numeric_limits<uint8_t>::max());
Expand Down Expand Up @@ -278,28 +273,38 @@ static void abort_on_object_get_failure(const Config *conf, const char *key)
esp_system_abort(msg);
}

Config::Wrap Config::get(const String &s)
Config::Wrap Config::get(const char *s, size_t s_len)
{
ASSERT_MAIN_THREAD();
if (!this->is<Config::ConfObject>()) {
abort_on_object_get_failure(this, s.c_str());
abort_on_object_get_failure(this, s);
}
Wrap wrap(value.val.o.get(s));
Wrap wrap(value.val.o.get(s, s_len));

return wrap;
}

const Config::ConstWrap Config::get(const String &s) const
const Config::ConstWrap Config::get(const char *s, size_t s_len) const
{
ASSERT_MAIN_THREAD();
if (!this->is<Config::ConfObject>()) {
abort_on_object_get_failure(this, s.c_str());
abort_on_object_get_failure(this, s);
}
ConstWrap wrap(value.val.o.get(s));
ConstWrap wrap(value.val.o.get(s, s_len));

return wrap;
}

Config::Wrap Config::get(const String &s)
{
return get(s.c_str(), s.length());
}

const Config::ConstWrap Config::get(const String &s) const
{
return get(s.c_str(), s.length());
}

[[gnu::noinline]]
[[gnu::noreturn]]
static void abort_on_array_get_failure(const Config *conf, size_t i)
Expand Down
20 changes: 17 additions & 3 deletions software/src/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ struct Config {
static Slot *allocSlotBuf(size_t elements);
static void freeSlotBuf(Slot *buf);

Config *get(const String &s);
const Config *get(const String &s) const;
Config *get(const char *s, size_t s_len);
const Config *get(const char *s, size_t s_len) const;
const Slot *getSlot() const;
Slot *getSlot();

Expand Down Expand Up @@ -537,7 +537,7 @@ struct Config {

static ConfigRoot *Confirm();
// Just for convenience.
static String ConfirmKey();
static const char *ConfirmKey() {return Config::confirm_key;}
static constexpr const char *confirm_key = "do_i_know_what_i_am_doing";

static Config Uint8(uint8_t u);
Expand Down Expand Up @@ -593,10 +593,24 @@ struct Config {
const ConstWrap get() const;

// for ConfObject
Wrap get(const char *s, size_t s_len = 0);
const ConstWrap get(const char *s, size_t s_len = 0) const;
Wrap get(const String &s);
const ConstWrap get(const String &s) const;

// for ConfArray
Wrap get(int8_t ) = delete;
const ConstWrap get(int8_t ) const = delete;
Wrap get(int16_t) = delete;
const ConstWrap get(int16_t) const = delete;
Wrap get(long ) = delete;
const ConstWrap get(long ) const = delete;
inline Wrap get(int i) {return get(static_cast<size_t>(i));} // These casts should be safe, as negative values become huge positive values,
inline const ConstWrap get(int i) const {return get(static_cast<size_t>(i));} // and the nested get() performs an array bounds check.
inline Wrap get(uint8_t i) {return get(static_cast<size_t>(i));}
inline const ConstWrap get(uint8_t i) const {return get(static_cast<size_t>(i));}
inline Wrap get(uint16_t i) {return get(static_cast<size_t>(i));}
inline const ConstWrap get(uint16_t i) const {return get(static_cast<size_t>(i));}
Wrap get(size_t i);
const ConstWrap get(size_t i) const;
Wrap add();
Expand Down
50 changes: 38 additions & 12 deletions software/src/config/conf_object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

#include "config/private.h"

#include "event_log_prefix.h"
#include "main_dependencies.h"
#include "tools/memory.h"

bool Config::ConfObject::slotEmpty(size_t i)
Expand All @@ -45,44 +47,68 @@ static void abort_on_key_not_found(const char *needle)
esp_system_abort(msg);
}

Config *Config::ConfObject::get(const String &needle)
Config *Config::ConfObject::get(const char *needle, size_t needle_len)
{
const auto *slot = this->getSlot();
const auto schema = slot->schema;
const auto size = schema->length;
const auto keys = schema->keys;

const auto needle_length = needle.length();
const auto needle_cstr = needle.c_str();
if (string_is_in_rodata(needle)) {
for (size_t i = 0; i < size; ++i) {
if (keys[i].val == needle) { // Address comparison, not string comparison
return &slot->values[i];
}
}

logger.printfln("Key '%s' in rodata but not in keys.", needle);
}

if (!needle_len) {
needle_len = strlen(needle);
}

for (size_t i = 0; i < size; ++i) {
if (schema->keys[i].length != needle_length)
if (keys[i].length != needle_len)
continue;

if (memcmp(schema->keys[i].val, needle_cstr, needle_length) == 0)
if (memcmp(keys[i].val, needle, needle_len) == 0)
return &slot->values[i];
}

abort_on_key_not_found(needle_cstr);
abort_on_key_not_found(needle);
}

const Config *Config::ConfObject::get(const String &needle) const
const Config *Config::ConfObject::get(const char *needle, size_t needle_len) const
{
const auto *slot = this->getSlot();
const auto schema = slot->schema;
const auto size = schema->length;
const auto keys = schema->keys;

const auto needle_length = needle.length();
const auto needle_cstr = needle.c_str();
if (string_is_in_rodata(needle)) {
for (size_t i = 0; i < size; ++i) {
if (keys[i].val == needle) { // Address comparison, not string comparison
return &slot->values[i];
}
}

logger.printfln("Key '%s' in rodata but not in keys.", needle);
}

if (!needle_len) {
needle_len = strlen(needle);
}

for (size_t i = 0; i < size; ++i) {
if (schema->keys[i].length != needle_length)
if (keys[i].length != needle_len)
continue;

if (memcmp(schema->keys[i].val, needle_cstr, needle_length) == 0)
if (memcmp(keys[i].val, needle, needle_len) == 0)
return &slot->values[i];
}

abort_on_key_not_found(needle_cstr);
abort_on_key_not_found(needle);
}

const Config::ConfObject::Slot *Config::ConfObject::getSlot() const { return &object_buf[idx]; }
Expand Down

0 comments on commit 076fd08

Please sign in to comment.