Improve performance of "P1Parser::parse" by using std::map. Various refactoring.

CMakeLists.txt

@@ -40,7 +40,7 @@ include(${cmake_template_SOURCE_DIR}/cmake/CompilerWarnings.cmake)
 include(${cmake_template_SOURCE_DIR}/cmake/Sanitizers.cmake)
 
 # dsmr_parser_test
-file(GLOB_RECURSE dsmr_parser_test_src_files CONFIGURE_DEPENDS "src/*.h" "src/*.cpp")
+file(GLOB_RECURSE dsmr_parser_test_src_files CONFIGURE_DEPENDS "src/*.h" "src/*.cpp" "tests/*.h" "tests/*.cpp")
 add_executable(dsmr_parser_test ${dsmr_parser_test_src_files})
 target_include_directories(dsmr_parser_test PRIVATE ${CMAKE_SOURCE_DIR}/src ${CMAKE_BINARY_DIR}/doctest)
 target_compile_features(dsmr_parser_test PRIVATE cxx_std_20)

README.md

@@ -4,7 +4,7 @@ The primary goal is to make the parser independent of the Arduino framework and
 # Features
 * Combines all fixes from [matthijskooijman/arduino-dsmr](https://github.com/matthijskooijman/arduino-dsmr) and [glmnet/arduino-dsmr](https://github.com/glmnet/arduino-dsmr) including unmerged pull requests.
 * Added an extensive unit test suite
-* Small refactoring and code optimizations
+* Code optimizations
 * Supported compilers: MSVC, GCC, Clang
 * Header-only library, no dependencies
 * Code can be used on any platform, not only embedded.

src/dsmr_parser/fields.h

@@ -26,7 +26,7 @@ struct ParsedField {
     f.apply(*static_cast<T*>(this));
   }
   // By defaults, fields have no unit
-  static const char* unit() { return ""; }
+  static const char* unit() noexcept { return ""; }
 };
 
 template <typename T, size_t minlen, size_t maxlen>
@@ -54,9 +54,9 @@ struct TimestampField : StringField<T, 13, 13> {};
 // efficient integer value. The unit() and int_unit() methods on
 // FixedField return the corresponding units for these values.
 struct FixedValue {
-  operator float() const { return val(); }
-  float val() const { return static_cast<float>(_value) / 1000.0f; }
-  uint32_t int_val() const { return _value; }
+  operator float() const noexcept { return val(); }
+  float val() const noexcept { return static_cast<float>(_value) / 1000.0f; }
+  uint32_t int_val() const noexcept { return _value; }
 
   uint32_t _value;
 };
@@ -90,8 +90,8 @@ struct FixedField : ParsedField<T> {
     return res_float;
   }
 
-  static const char* unit() { return _unit; }
-  static const char* int_unit() { return _int_unit; }
+  static const char* unit() noexcept { return _unit; }
+  static const char* int_unit() noexcept { return _int_unit; }
 };
 
 struct TimestampedFixedValue : public FixedValue {
@@ -153,7 +153,7 @@ struct IntField : ParsedField<T> {
     return res;
   }
 
-  static const char* unit() { return _unit; }
+  static const char* unit() noexcept { return _unit; }
 };
 
 // Take the average value of multiple values. Example:

src/dsmr_parser/parser.h

@@ -1,22 +1,10 @@
 #pragma once
 
 #include "util.h"
+#include <unordered_map>
 
 namespace dsmr_parser {
 
-// uses polynomial x^16+x^15+x^2+1
-inline uint16_t crc16_update(uint16_t crc, uint8_t data) {
-  crc ^= data;
-  for (size_t i = 0; i < 8; ++i) {
-    if (crc & 1) {
-      crc = (crc >> 1) ^ 0xA001;
-    } else {
-      crc = (crc >> 1);
-    }
-  }
-  return crc;
-}
-
 // ParsedData is a template for the result of parsing a Dsmr P1 message.
 // You pass the fields you want to add to it as template arguments.
 //
@@ -41,25 +29,38 @@ inline uint16_t crc16_update(uint16_t crc, uint8_t data) {
 // to loop over all the fields inside it.
 template <typename... Ts>
 struct ParsedData final : Ts... {
-  ParseResult<void> parse_line(const ObisId& obisId, const char* str, const char* end) {
-    ParseResult<void> res;
-    const auto& try_one = [&](auto& field) -> bool {
-      using FieldType = std::decay_t<decltype(field)>;
-      if (obisId != FieldType::id) {
-        return false;
-      }
-
-      if (field.present())
-        res = ParseResult<void>().fail("Duplicate field", str);
-      else {
-        field.present() = true;
-        res = field.parse(str, end);
-      }
-      return true;
-    };
+private:
+  static const auto& fields_map() {
+    static const auto& m = []() {
+      const auto hasher = [](const ObisId& id) noexcept {
+        std::uint64_t x = 0;
+        std::memcpy(&x, id.v.data(), 6);
+        return std::hash<std::uint64_t>{}(x);
+      };
+      using FieldParseFunc = ParseResult<void> (*)(ParsedData&, const char*, const char*);
+      std::unordered_map<ObisId, FieldParseFunc, decltype(hasher)> tmp;
+      (void)std::initializer_list<int>{(tmp.emplace(Ts::id,
+                                                    [](ParsedData& self, const char* str, const char* end) {
+                                                      auto& field = static_cast<Ts&>(self);
+                                                      ParseResult<void> res;
+                                                      if (field.present())
+                                                        return res.fail("Duplicate field", str);
+                                                      field.present() = true;
+                                                      return field.parse(str, end);
+                                                    }),
+                                        0)...};
+      return tmp;
+    }();
+    return m;
+  }
 
-    const bool found = (try_one(static_cast<Ts&>(*this)) || ...);
-    return found ? res : ParseResult<void>().until(str);
+public:
+  ParseResult<void> parse_line(const ObisId& obisId, const char* str, const char* end) {
+    const auto& m = fields_map();
+    auto it = m.find(obisId);
+    if (it == m.end())
+      return ParseResult<void>().until(str);
+    return it->second(*this, str, end);
   }
 
   template <typename F>
@@ -166,7 +167,7 @@ struct NumParser final {
 };
 
 struct ObisIdParser final {
-  static ParseResult<ObisId> parse(const char* str, const char* end) {
+  static ParseResult<ObisId> parse(const char* str, const char* end) noexcept {
     // Parse a Obis ID of the form 1-2:3.4.5.6
     // Stops parsing on the first unrecognized character. Any unparsed
     // parts are set to 255.
@@ -208,7 +209,7 @@ struct CrcParser final {
 private:
   static const size_t CRC_LEN = 4;
 
-  static bool hex_nibble(char c, uint8_t& out) {
+  static bool hex_nibble(char c, uint8_t& out) noexcept {
     if (c >= '0' && c <= '9') {
       out = static_cast<uint8_t>(c - '0');
       return true;
@@ -247,13 +248,27 @@ struct CrcParser final {
 };
 
 struct P1Parser final {
+private:
+  // uses polynomial x^16+x^15+x^2+1
+  static uint16_t crc16_update(uint16_t crc, uint8_t data) noexcept {
+    crc ^= data;
+    for (size_t i = 0; i < 8; ++i) {
+      if (crc & 1) {
+        crc = (crc >> 1) ^ 0xA001;
+      } else {
+        crc = (crc >> 1);
+      }
+    }
+    return crc;
+  }
 
+public:
   // Parse a complete P1 telegram. The string passed should start
   // with '/' and run up to and including the ! and the following
   // four byte checksum. It's ok if the string is longer, the .next
   // pointer in the result will indicate the next unprocessed byte.
   template <typename... Ts>
-  static ParseResult<void> parse(ParsedData<Ts...>* data, const char* str, size_t n, bool unknown_error = false, bool check_crc = true) {
+  static ParseResult<void> parse(ParsedData<Ts...>& data, const char* str, size_t n, bool unknown_error = false, bool check_crc = true) {
     ParseResult<void> res;
 
     const char* const buf_begin = str;
@@ -303,7 +318,7 @@ struct P1Parser final {
   // character after the leading /, end should point to the ! before the
   // checksum. Does not verify the checksum.
   template <typename... Ts>
-  static ParseResult<void> parse_data(ParsedData<Ts...>* data, const char* str, const char* end, bool unknown_error = false) {
+  static ParseResult<void> parse_data(ParsedData<Ts...>& data, const char* str, const char* end, bool unknown_error = false) {
     // Split into lines and parse those
     const char* line_end = str;
     const char* line_start = str;
@@ -327,7 +342,7 @@ struct P1Parser final {
         //
         // Offer it for processing using the all-ones Obis ID, which
         // is not otherwise valid.
-        ParseResult<void> tmp = data->parse_line(ObisId(255, 255, 255, 255, 255, 255), line_start, line_end);
+        ParseResult<void> tmp = data.parse_line(ObisId(255, 255, 255, 255, 255, 255), line_start, line_end);
         if (tmp.err)
           return tmp;
         line_start = ++line_end;
@@ -383,7 +398,7 @@ struct P1Parser final {
   }
 
   template <typename Data>
-  static ParseResult<void> parse_line(Data* data, const char* line, const char* end, bool unknown_error) {
+  static ParseResult<void> parse_line(Data& data, const char* line, const char* end, bool unknown_error) {
     ParseResult<void> res;
     if (line == end)
       return res;
@@ -392,7 +407,7 @@ struct P1Parser final {
     if (idres.err)
       return idres;
 
-    ParseResult<void> datares = data->parse_line(idres.result, idres.next, end);
+    ParseResult<void> datares = data.parse_line(idres.result, idres.next, end);
     if (datares.err)
       return datares;
 
@@ -407,5 +422,4 @@ struct P1Parser final {
     return res.until(end);
   }
 };
-
 }

src/dsmr_parser/util.h

@@ -84,19 +84,21 @@ struct ParseResult final : public _ParseResult<ParseResult<T>, T> {
   const char* err = nullptr;
   const char* ctx = nullptr;
 
-  ParseResult& fail(const char* error, const char* context = nullptr) {
+  ParseResult& fail(const char* error, const char* context = nullptr) noexcept {
     this->err = error;
     this->ctx = context;
     return *this;
   }
-  ParseResult& until(const char* nextToken) {
+
+  ParseResult& until(const char* nextToken) noexcept {
     this->next = nextToken;
     return *this;
   }
+
   ParseResult() = default;
 
   template <typename T2>
-  ParseResult(const ParseResult<T2>& other) : next(other.next), err(other.err), ctx(other.ctx) {}
+  ParseResult(const ParseResult<T2>& other) noexcept : next(other.next), err(other.err), ctx(other.ctx) {}
 
   // Returns the error, including context in a fancy multi-line format.
   // The start and end passed are the first and one-past-the-end
@@ -136,10 +138,11 @@ struct ParseResult final : public _ParseResult<ParseResult<T>, T> {
 // An OBIS id is 6 bytes, usually noted as a-b:c.d.e.f. Here we put them in an array for easy parsing.
 struct ObisId final {
   std::array<uint8_t, 6> v{};
-  constexpr ObisId(const uint8_t a, const uint8_t b = 255, const uint8_t c = 255, const uint8_t d = 255, const uint8_t e = 255, const uint8_t f = 255) noexcept
+  constexpr explicit ObisId(const uint8_t a, const uint8_t b = 255, const uint8_t c = 255, const uint8_t d = 255, const uint8_t e = 255,
+                            const uint8_t f = 255) noexcept
       : v{a, b, c, d, e, f} {};
   ObisId() = default;
-  bool operator==(const ObisId&) const = default;
+  auto operator<=>(const ObisId&) const = default;
 };
 
 }

src/test/packet_accumulator_example_test.cpp → tests/packet_accumulator_example_test.cpp

@@ -68,7 +68,7 @@ TEST_CASE("PacketAccumulator example") {
 
       // Parse the packet.
       // Specify `check_crc` as false, since the accumulator already checked the CRC and didn't include it in the packet
-      P1Parser::parse(&data, packet.data(), packet.size(), /* unknown_error */ false, /* check_crc */ false);
+      P1Parser::parse(data, packet.data(), packet.size(), /* unknown_error */ false, /* check_crc */ false);
 
       // Now you can use the parsed data.
       printf("Identification: %s\n", data.identification.c_str());

src/test/parser_include_test.cpp → tests/parser_include_test.cpp

@@ -10,5 +10,5 @@ using namespace fields;
 void P1Parser_some_function() {
   const auto& msg = "";
   ParsedData<identification, p1_version> data;
-  P1Parser::parse(&data, msg, std::size(msg), true);
+  P1Parser::parse(data, msg, std::size(msg), true);
 }