Factor out the unique string key pool from OSM::DataSet

Will be useful for optimizing the MapCSS styles as well.

autotests/osmtypetest.cpp

@@ -47,16 +47,16 @@ private Q_SLOTS:
         QCOMPARE(nullKey, ds.tagKey("akey"));
         QCOMPARE(nullKey.isNull(), true);
 
-        const auto key1 = ds.makeTagKey("akey", OSM::DataSet::StringIsPersistent);
-        QCOMPARE(key1, ds.makeTagKey("akey", OSM::DataSet::StringIsPersistent));
+        const auto key1 = ds.makeTagKey("akey", OSM::StringMemory::Persistent);
+        QCOMPARE(key1, ds.makeTagKey("akey", OSM::StringMemory::Persistent));
         QCOMPARE(key1, ds.tagKey("akey"));
         QCOMPARE(key1.isNull(), false);
         QCOMPARE(key1.name(), "akey");
         QVERIFY(key1 != nullKey);
 
-        const auto key2 = ds.makeTagKey("bkey", OSM::DataSet::StringIsTransient);
+        const auto key2 = ds.makeTagKey("bkey", OSM::StringMemory::Transient);
         QVERIFY(key1 != key2);
-        QCOMPARE(key2, ds.makeTagKey("bkey", OSM::DataSet::StringIsTransient));
+        QCOMPARE(key2, ds.makeTagKey("bkey", OSM::StringMemory::Transient));
 
         OSM::Node node;
         OSM::setTagValue(node, key1, "avalue");

src/osm/CMakeLists.txt

@@ -23,6 +23,7 @@ add_library(KOSM
     overpassquery.cpp
     overpassquerymanager.cpp
     pathutil.cpp
+    stringpool.cpp
     xmlparser.cpp
     xmlwriter.cpp
     ztile.cpp
@@ -56,5 +57,6 @@ install(FILES
     ${KOSM_HEADERS}
     ${CMAKE_CURRENT_BINARY_DIR}/kosm_export.h
     internal.h
+    stringpool.h
     DESTINATION ${KDE_INSTALL_INCLUDEDIR}/kosm
 )

src/osm/datatypes.cpp

@@ -10,62 +10,28 @@ using namespace OSM;
 
 DataSet::DataSet() = default;
 DataSet::DataSet(DataSet &&) = default;
-DataSet::~DataSet()
-{
-    std::for_each(m_stringPool.begin(), m_stringPool.end(), free);
-}
+DataSet::~DataSet() = default;
 
 DataSet& DataSet::operator=(DataSet &&) = default;
 
-template<typename T>
-T DataSet::makeStringKey(const char *name, DataSet::StringMemory memOpt, std::vector<T> &registry)
-{
-    const auto it = std::lower_bound(registry.begin(), registry.end(), name, [](T lhs, const char *rhs) {
-        return std::strcmp(lhs.name(), rhs) < 0;
-    });
-    if (it == registry.end() || std::strcmp((*it).name(), name) != 0) {
-        if (memOpt == StringIsTransient) {
-            auto s = strdup(name);
-            m_stringPool.push_back(s);
-            name = s;
-        }
-        T k(name);
-        registry.insert(it, k);
-        return k;
-    }
-    return (*it);
-}
-
-TagKey DataSet::makeTagKey(const char *keyName, DataSet::StringMemory keyMemOpt)
+TagKey DataSet::makeTagKey(const char *keyName, OSM::StringMemory keyMemOpt)
 {
-    return makeStringKey(keyName, keyMemOpt, m_tagKeyRegistry);
+    return m_tagKeyRegistry.makeKey(keyName, keyMemOpt);
 }
 
-Role DataSet::makeRole(const char *roleName, DataSet::StringMemory memOpt)
+Role DataSet::makeRole(const char *roleName, OSM::StringMemory memOpt)
 {
-    return makeStringKey(roleName, memOpt, m_roleRegistry);
-}
-
-template <typename T>
-T DataSet::stringKey(const char *name, const std::vector<T> &registry) const
-{
-    const auto it = std::lower_bound(registry.begin(), registry.end(), name, [](T lhs, const char *rhs) {
-        return std::strcmp(lhs.name(), rhs) < 0;
-    });
-    if (it == registry.end() || std::strcmp((*it).name(), name) != 0) {
-        return {};
-    }
-    return (*it);
+    return m_roleRegistry.makeKey(roleName, memOpt);
 }
 
 TagKey DataSet::tagKey(const char *keyName) const
 {
-    return stringKey(keyName, m_tagKeyRegistry);
+    return m_tagKeyRegistry.key(keyName);
 }
 
 Role DataSet::role(const char *roleName) const
 {
-    return stringKey(roleName, m_roleRegistry);
+    return m_roleRegistry.key(roleName);
 }
 
 const Node* DataSet::node(Id id) const

src/osm/datatypes.h

@@ -9,6 +9,7 @@
 
 #include "kosm_export.h"
 #include "internal.h"
+#include "stringpool.h"
 
 #include <QByteArray>
 #include <QDebug>
@@ -172,37 +173,10 @@ constexpr inline uint32_t longitudeDifference(BoundingBox bbox1, BoundingBox bbo
     return bbox1.max.longitude < bbox2.min.longitude ? bbox2.min.longitude - bbox1.max.longitude : bbox1.min.longitude - bbox2.max.longitude;
 }
 
-/** Base class for unique string keys. */
-class StringKey
-{
-public:
-    constexpr inline const char* name() const { return key; }
-    constexpr inline bool isNull() const { return !key; }
-
-    // yes, pointer compare is enough here
-    inline constexpr bool operator<(StringKey other) const { return key < other.key; }
-    inline constexpr bool operator==(StringKey other) const { return key == other.key; }
-    inline constexpr bool operator!=(StringKey other) const { return key != other.key; }
-
-protected:
-    constexpr inline StringKey() = default;
-    explicit constexpr inline StringKey(const char *keyData) : key(keyData) {}
-
-private:
-    const char* key = nullptr;
-};
-
 /** A key of an OSM tag.
  *  See DataSet::tagKey().
  */
-class TagKey : public StringKey
-{
-public:
-    constexpr inline TagKey() = default;
-private:
-    explicit constexpr inline TagKey(const char *keyData) : StringKey(keyData) {}
-    friend class DataSet;
-};
+class TagKey : public StringKey {};
 
 /** An OSM element tag. */
 class Tag {
@@ -259,7 +233,6 @@ class Role : public StringKey
 public:
     constexpr inline Role() = default;
 private:
-    friend class DataSet;
     friend class Member;
     explicit constexpr inline Role(const char *keyData) : StringKey(keyData) {}
 };
@@ -343,14 +316,13 @@ public:
      */
     TagKey tagKey(const char *keyName) const;
 
-    enum StringMemory { StringIsPersistent, StringIsTransient };
     /** Create a tag key for the given tag name. If none exist yet a new one is created.
      *  Use this for creating tags, not for lookup, prefer tagKey() for that.
      *  @param keyMemOpt specifies whether @p keyName is persisent for the lifetime of this
      *  instance and thus can be used without requiring a copy. If the memory is transient
      *  the string is copied if needed, and released in the DataSet destructor.
      */
-    TagKey makeTagKey(const char *keyName, StringMemory keyMemOpt = StringIsTransient);
+    TagKey makeTagKey(const char *keyName, StringMemory keyMemOpt = StringMemory::Transient);
 
     /** Looks up a role name key.
      *  @see tagKey()
@@ -359,7 +331,7 @@ public:
     /** Creates a role name key.
      *  @see makeTagKey()
      */
-    Role makeRole(const char *roleName, StringMemory memOpt = StringIsTransient);
+    Role makeRole(const char *roleName, StringMemory memOpt = StringMemory::Transient);
 
     /** Create a unique id for internal use (ie. one that will not clash with official OSM ids). */
     Id nextInternalId() const;
@@ -372,9 +344,8 @@ private:
     template <typename T> T stringKey(const char *name, const std::vector<T> &registry) const;
     template <typename T> T makeStringKey(const char *name, StringMemory memOpt, std::vector<T> &registry);
 
-    std::vector<TagKey> m_tagKeyRegistry;
-    std::vector<Role> m_roleRegistry;
-    std::vector<char*> m_stringPool;
+    StringKeyRegistry<TagKey> m_tagKeyRegistry;
+    StringKeyRegistry<Role> m_roleRegistry;
 };
 
 /** Returns the tag value for @p key of @p elem. */

src/osm/o5mparser.cpp

@@ -197,7 +197,7 @@ void O5mParser::readTagOrBbox(Elem &e, const uint8_t *&it, const uint8_t *endIt)
     }
 
     OSM::Tag tag;
-    tag.key = m_dataSet->makeTagKey(tagData.first, DataSet::StringIsTransient); // TODO make use of mmap'ed data for this
+    tag.key = m_dataSet->makeTagKey(tagData.first, OSM::StringMemory::Transient); // TODO make use of mmap'ed data for this
     tag.value = QByteArray(tagData.second);
     OSM::setTag(e, std::move(tag));
 }
@@ -218,7 +218,7 @@ void O5mParser::readNode(const uint8_t *begin, const uint8_t *end)
         OSM::Tag tag;
         const auto tagData = readStringPair(it, end);
         if (tagData.first) {
-            tag.key = m_dataSet->makeTagKey(tagData.first, DataSet::StringIsTransient); // TODO use the fact this is mmap'ed data here
+            tag.key = m_dataSet->makeTagKey(tagData.first, OSM::StringMemory::Transient); // TODO use the fact this is mmap'ed data here
             tag.value = QByteArray(tagData.second);
             OSM::setTag(node, std::move(tag));
         }
@@ -282,7 +282,7 @@ void O5mParser::readRelation(const uint8_t *begin, const uint8_t *end)
                 mem.setType(OSM::Type::Relation);
                 break;
         }
-        mem.setRole(m_dataSet->makeRole(typeAndRole + 1, DataSet::StringIsTransient));
+        mem.setRole(m_dataSet->makeRole(typeAndRole + 1, OSM::StringMemory::Transient));
 
         rel.members.push_back(std::move(mem));
     }

src/osm/stringpool.cpp

+/*
+    SPDX-FileCopyrightText: 2020-2021 Volker Krause <vkrause@kde.org>
+
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include "stringpool.h"
+
+#include <algorithm>
+#include <cstring>
+
+OSM::StringKeyRegistryBase::StringKeyRegistryBase() = default;
+OSM::StringKeyRegistryBase::StringKeyRegistryBase(OSM::StringKeyRegistryBase&&) = default;
+OSM::StringKeyRegistryBase& OSM::StringKeyRegistryBase::operator=(OSM::StringKeyRegistryBase&&) = default;
+
+OSM::StringKeyRegistryBase::~StringKeyRegistryBase()
+{
+    std::for_each(m_pool.begin(), m_pool.end(), free);
+}
+
+const char* OSM::StringKeyRegistryBase::makeKeyInternal(const char *name, std::size_t len, OSM::StringMemory memOpt)
+{
+    const auto it = std::lower_bound(m_registry.begin(), m_registry.end(), name, [len](const char *lhs, const char *rhs) {
+        return std::strncmp(lhs, rhs, len) < 0;
+    });
+    if (it == m_registry.end() || std::strncmp((*it), name, len) != 0 || std::strlen(*it) != len) {
+        if (memOpt == OSM::StringMemory::Transient) {
+            auto s = strndup(name, len);
+            m_pool.push_back(s);
+            name = s;
+        }
+        m_registry.insert(it, name);
+        return name;
+    }
+    return (*it);
+}
+
+const char* OSM::StringKeyRegistryBase::keyInternal(const char *name) const
+{
+    const auto it = std::lower_bound(m_registry.begin(), m_registry.end(), name, [](const char *lhs, const char *rhs) {
+        return std::strcmp(lhs, rhs) < 0;
+    });
+    if (it == m_registry.end() || std::strcmp((*it), name) != 0) {
+        return {};
+    }
+    return (*it);
+}

src/osm/stringpool.h

+/*
+    SPDX-FileCopyrightText: 2020-2021 Volker Krause <vkrause@kde.org>
+
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#ifndef KOSM_STRINGPOOL_H
+#define KOSM_STRINGPOOL_H
+
+#include <kosm_export.h>
+
+#include <cstring>
+#include <vector>
+
+namespace OSM {
+
+enum class StringMemory { Persistent, Transient };
+
+/** @internal */
+class KOSM_EXPORT StringKeyRegistryBase
+{
+protected:
+    explicit StringKeyRegistryBase();
+    StringKeyRegistryBase(StringKeyRegistryBase&&);
+    StringKeyRegistryBase& operator=(StringKeyRegistryBase&&);
+    ~StringKeyRegistryBase();
+
+    const char* makeKeyInternal(const char *name, std::size_t len, StringMemory memOpt);
+    const char* keyInternal(const char *name) const;
+
+    std::vector<char*> m_pool;
+    std::vector<const char*> m_registry;
+};
+
+/** Registry of unique string keys.
+ *  @tparam T Sub-classes of StringKey, to have a compile-time check against comparing keys from different pools.
+ */
+template <typename T>
+class StringKeyRegistry : protected StringKeyRegistryBase
+{
+public:
+    explicit StringKeyRegistry() = default;
+    StringKeyRegistry(const StringKeyRegistry&) = delete;
+    StringKeyRegistry(StringKeyRegistry&&) = default;
+    ~StringKeyRegistry() = default;
+    StringKeyRegistry& operator=(const StringKeyRegistry&) = delete;
+    StringKeyRegistry& operator=(StringKeyRegistry&&) = default;
+
+    /** Add a new string to the registry if needed, or returns an existing one if already present. */
+    inline T makeKey(const char *name, StringMemory memOpt)
+    {
+        return makeKey(name, std::strlen(name), memOpt);
+    }
+    inline T makeKey(const char *name, std::size_t len, StringMemory memOpt)
+    {
+        T key;
+        key.key = makeKeyInternal(name, len, memOpt);
+        return key;
+    }
+
+    /** Looks up an existing key, if that doesn't exist an null key is returned. */
+    inline T key(const char *name) const
+    {
+        T key;
+        key.key = keyInternal(name);
+        return key;
+    }
+};
+
+/** Base class for unique string keys. */
+class StringKey
+{
+public:
+    constexpr inline StringKey() = default;
+    constexpr inline const char* name() const { return key; }
+    constexpr inline bool isNull() const { return !key; }
+
+    // yes, pointer compare is enough here
+    inline constexpr bool operator<(StringKey other) const { return key < other.key; }
+    inline constexpr bool operator==(StringKey other) const { return key == other.key; }
+    inline constexpr bool operator!=(StringKey other) const { return key != other.key; }
+
+protected:
+    explicit constexpr inline StringKey(const char *keyData) : key(keyData) {}
+
+private:
+    template <typename T> friend class StringKeyRegistry;
+    const char* key = nullptr;
+};
+
+}
+
+#endif // KOSM_STRINGPOOL_H

src/osm/xmlparser.cpp

@@ -112,7 +112,7 @@ void XmlParser::parseRelation(QXmlStreamReader &reader)
             } else {
                 member.setType(Type::Relation);
             }
-            member.setRole(m_dataSet->makeRole(reader.attributes().value(QLatin1String("role")).toUtf8().constData(), DataSet::StringIsTransient));
+            member.setRole(m_dataSet->makeRole(reader.attributes().value(QLatin1String("role")).toUtf8().constData(), OSM::StringMemory::Transient));
             rel.members.push_back(std::move(member));
         }
         reader.skipCurrentElement();
@@ -124,7 +124,7 @@ void XmlParser::parseRelation(QXmlStreamReader &reader)
 template <typename T>
 void XmlParser::parseTag(QXmlStreamReader &reader, T &elem)
 {
-    const auto key = m_dataSet->makeTagKey(reader.attributes().value(QLatin1String("k")).toString().toUtf8().constData(), OSM::DataSet::StringIsTransient);
+    const auto key = m_dataSet->makeTagKey(reader.attributes().value(QLatin1String("k")).toString().toUtf8().constData(), OSM::StringMemory::Transient);
     OSM::setTagValue(elem, key, reader.attributes().value(QLatin1String("v")).toUtf8());
 }