Merge pull request #2061 from joto/low-level-performance

Improve performance of some low level code

Author: lonvia

src/gen/gen-rivers.cpp

@@ -258,7 +258,7 @@ SELECT "{id_column}", "{width_column}", "{name_column}", "{geom_column}"
             if (!name.empty()) {
                 names.emplace(id, name);
             }
-            auto const geom = ewkb_to_geom(decode_hex(result.get_value(i, 3)));
+            auto const geom = ewkb_to_geom(decode_hex(result.get(i, 3)));
 
             if (geom.is_linestring()) {
                 auto const &ls = geom.get<geom::linestring_t>();

src/gen/gen-tile-builtup.cpp

@@ -173,7 +173,7 @@ static void draw_from_db(double margin, canvas_list_t *canvas_list,
                                 box.max_x(), box.max_y());
 
         for (int n = 0; n < result.num_tuples(); ++n) {
-            auto const geom = ewkb_to_geom(decode_hex(result.get_value(n, 0)));
+            auto const geom = ewkb_to_geom(decode_hex(result.get(n, 0)));
             cc.canvas.draw(geom, tile);
         }
     }

src/gen/gen-tile-raster.cpp

@@ -163,7 +163,7 @@ static void draw_from_db(double margin, unsigned int image_extent,
 
     for (int n = 0; n < result.num_tuples(); ++n) {
         std::string param = result.get_value(n, 1);
-        auto const geom = ewkb_to_geom(decode_hex(result.get_value(n, 0)));
+        auto const geom = ewkb_to_geom(decode_hex(result.get(n, 0)));
 
         auto const [it, success] = canvas_list->try_emplace(
             std::move(param), image_extent, image_buffer);

src/gen/tracer.cpp

@@ -41,21 +41,33 @@ void tracer_t::reset()
     m_num_points = 0;
 }
 
+static potrace_word bit_squeeze(potrace_word w, unsigned char const *d) noexcept
+{
+    return (0x80U & d[0]) | (0x40U & d[1]) | (0x20U & d[2]) | (0x10U & d[3]) |
+           (0x08U & d[4]) | (0x04U & d[5]) | (0x02U & d[6]) | (0x01U & d[7]) |
+           w;
+}
+
+static_assert(sizeof(potrace_word) == 8);
+
 void tracer_t::prepare(canvas_t const &canvas) noexcept
 {
     std::size_t const size = canvas.size();
     assert(size % bits_per_word == 0);
 
     m_bits.reserve((size * size) / bits_per_word);
 
-    unsigned char const *d = canvas.begin();
-    while (d != canvas.end()) {
-        potrace_word w = 0x1U & *d++;
-        for (std::size_t n = 1; n < bits_per_word; ++n) {
-            w <<= 1U;
-            assert(d != canvas.end());
-            w |= 0x1U & *d++;
-        }
+    for (unsigned char const *d = canvas.begin(); d != canvas.end(); d += 8) {
+        auto w = bit_squeeze(0, d);
+
+        w = bit_squeeze(w << 8U, d += 8);
+        w = bit_squeeze(w << 8U, d += 8);
+        w = bit_squeeze(w << 8U, d += 8);
+        w = bit_squeeze(w << 8U, d += 8);
+        w = bit_squeeze(w << 8U, d += 8);
+        w = bit_squeeze(w << 8U, d += 8);
+        w = bit_squeeze(w << 8U, d += 8);
+
         m_bits.push_back(w);
     }
 

src/wkb.cpp

@@ -576,32 +576,35 @@ geom::geometry_t ewkb_to_geom(std::string_view wkb)
     return geom;
 }
 
-unsigned char decode_hex_char(char c)
-{
-    if (c >= '0' && c <= '9') {
-        return c - '0';
-    }
-    if (c >= 'A' && c <= 'F') {
-        return c - 'A' + 10;
-    }
-    if (c >= 'a' && c <= 'f') {
-        return c - 'a' + 10;
-    }
+static constexpr std::array<char, 256> const hex_table = {
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    0, 1, 2, 3,   4, 5, 6, 7,   8, 9, 0, 0,   0, 0, 0, 0,
+
+    0, 10, 11, 12,   13, 14, 15, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    0,  0,  0,  0,    0,  0,  0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    0, 10, 11, 12,   13, 14, 15, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+};
 
-    throw std::runtime_error{"Invalid wkb: Not a hex character"};
+unsigned char decode_hex_char(char c) noexcept
+{
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
+    return hex_table[static_cast<std::size_t>(static_cast<unsigned char>(c))];
 }
 
-std::string decode_hex(char const *hex)
+std::string decode_hex(std::string_view hex_string)
 {
+    if (hex_string.size() % 2 != 0) {
+        throw std::runtime_error{"Invalid wkb: Not a valid hex string"};
+    }
+
     std::string wkb;
+    wkb.reserve(hex_string.size() / 2);
 
-    while (*hex != '\0') {
+    // NOLINTNEXTLINE(llvm-qualified-auto, readability-qualified-auto)
+    for (auto hex = hex_string.begin(); hex != hex_string.end();) {
         unsigned int const c = decode_hex_char(*hex++);
-
-        if (*hex == '\0') {
-            throw std::runtime_error{"Invalid wkb: Not a valid hex string"};
-        }
-
         wkb += static_cast<char>((c << 4U) | decode_hex_char(*hex++));
     }
 

src/wkb.hpp

@@ -41,15 +41,15 @@
 [[nodiscard]] geom::geometry_t ewkb_to_geom(std::string_view wkb);
 
 /**
- * Decode one hex character (0-9A-F or 0-9a-f) and return its value. Throw
- * an exception if not a valid hex character.
+ * Decode one hex character (0-9A-F or 0-9a-f) and return its value.
+ * Returns 0 for characters that are not hex characters.
  */
-[[nodiscard]] unsigned char decode_hex_char(char c);
+[[nodiscard]] unsigned char decode_hex_char(char c) noexcept;
 
 /**
  * Decode a string of hex characters. Throws an exception if the input is not
  * a valid hex encoding.
  */
-[[nodiscard]] std::string decode_hex(char const *hex);
+[[nodiscard]] std::string decode_hex(std::string_view hex);
 
 #endif // OSM2PGSQL_WKB_HPP

tests/test-wkb.cpp

@@ -189,15 +189,19 @@ TEST_CASE("wkb: geometrycollection", "[NoDB]")
 
 TEST_CASE("wkb: invalid", "[NoDB]") { REQUIRE_THROWS(ewkb_to_geom("INVALID")); }
 
-TEST_CASE("wkb hex decode of valid hex characters")
+TEST_CASE("wkb hex decode of valid and invalid hex characters")
 {
     REQUIRE(decode_hex_char('0') == 0);
     REQUIRE(decode_hex_char('9') == 9);
     REQUIRE(decode_hex_char('a') == 0x0a);
     REQUIRE(decode_hex_char('f') == 0x0f);
     REQUIRE(decode_hex_char('A') == 0x0a);
     REQUIRE(decode_hex_char('F') == 0x0f);
-    REQUIRE_THROWS(decode_hex_char('x'));
+    REQUIRE(decode_hex_char('#') == 0);
+    REQUIRE(decode_hex_char('@') == 0);
+    REQUIRE(decode_hex_char('g') == 0);
+    REQUIRE(decode_hex_char('G') == 0);
+    REQUIRE(decode_hex_char(0x7f) == 0);
 }
 
 TEST_CASE("wkb hex decode of valid hex string")
@@ -216,11 +220,6 @@ TEST_CASE("wkb hex decode of valid hex string")
     REQUIRE(result == data);
 }
 
-TEST_CASE("wkb hex decode of invalid hex string")
-{
-    REQUIRE_THROWS(decode_hex("no"));
-}
-
 TEST_CASE("wkb hex decode of empty string is okay")
 {
     std::string const hex{};