Speed up Pointer::operator<().

Speed is more important than alphabetical order (which makes few sense in
JSON in general, and with pointers especially). The use case is indexing
in std containers, i.e. O(log n) with rbtree, so the faster comparison
the better.

Author: ylavic

include/rapidjson/pointer.h

@@ -366,20 +366,21 @@ class GenericPointer {
         if (!rhs.IsValid())
             return true;
 
-        const Token *lTok = tokens_, *const lEnd = lTok + tokenCount_,
-                    *rTok = rhs.tokens_, *const rEnd = rTok + rhs.tokenCount_;
-        for (; lTok != lEnd && rTok != rEnd; ++lTok, ++rTok) {
-            if (lTok->index != rTok->index)
-                return lTok->index < rTok->index;
-
-            if (lTok->length > rTok->length)
-                return std::memcmp(lTok->name, rTok->name, sizeof(Ch) * rTok->length) < 0;
-
-            int comp = std::memcmp(lTok->name, rTok->name, sizeof(Ch) * lTok->length);
-            if (comp || lTok->length != rTok->length)
-                return comp <= 0;
+        if (tokenCount_ != rhs.tokenCount_)
+            return tokenCount_ < rhs.tokenCount_;
+
+        for (size_t i = 0; i < tokenCount_; i++) {
+            if (tokens_[i].index != rhs.tokens_[i].index)
+                return tokens_[i].index < rhs.tokens_[i].index;
+
+            if (tokens_[i].length != rhs.tokens_[i].length)
+                return tokens_[i].length < rhs.tokens_[i].length;
+
+            if (int cmp = std::memcmp(tokens_[i].name, rhs.tokens_[i].name, sizeof(Ch) * tokens_[i].length))
+                return cmp < 0;
         }
-        return rTok != rEnd;
+
+        return false;
     }
 
     //@}

test/unittest/pointertest.cpp

@@ -17,7 +17,7 @@
 #include "rapidjson/stringbuffer.h"
 #include "rapidjson/ostreamwrapper.h"
 #include <sstream>
-#include <set>
+#include <map>
 
 using namespace rapidjson;
 
@@ -1496,72 +1496,75 @@ TEST(Pointer, Ambiguity) {
 }
 
 TEST(Pointer, LessThan) {
-    static const char *pointers[] = {
+    static const struct {
+        const char *str;
+        bool valid;
+    } pointers[] = {
+        { "/a/b",       true },
+        { "/a",         true },
+        { "/d/1",       true },
+        { "/d/2/z",     true },
+        { "/d/2/3",     true },
+        { "/d/2",       true },
+        { "/a/c",       true },
+        { "/e/f~g",     false },
+        { "/d/2/zz",    true },
+        { "/d/1",       true },
+        { "/d/2/z",     true },
+        { "/e/f~~g",    false },
+        { "/e/f~0g",    true },
+        { "/e/f~1g",    true },
+        { "/e/f.g",     true },
+        { "",           true }
+    };
+    static const char *ordered_pointers[] = {
+        "",
+        "/a",
         "/a/b",
         "/a/c",
-        "/a",
         "/d/1",
-        "/d/2/z",
-        "/d/2/3",
-        "/d/2",
-        "/d/2/zz",
         "/d/1",
-        "/d/2/z",
-        "/e/f~g",
-        "/e/f~0g",
-        "/e/f~1g",
-        "/e/f~~g",
-        "#/e/f%2fg",
+        "/d/2",
         "/e/f.g",
-        ""
-    };
-    static const struct {
-        const char *str;
-        bool valid;
-    } ordered_pointers[] = {
-        { "", true },
-        { "/a", true },
-        { "/a/b", true },
-        { "/a/c", true },
-        { "/d/1", true },
-        { "/d/1", true },
-        { "/d/2", true },
-        { "/d/2/3", true },
-        { "/d/2/z", true },
-        { "/d/2/z", true },
-        { "/d/2/zz", true },
-        { "/e/f.g", true },
-        { "/e/f~1g", true },
-        { "/e/f~1g", true }, // was "#/e/f%2fg"
-        { "/e/f~0g", true },
-        { "/e/f~g", false },
-        { "/e/f~~g", false }
+        "/e/f~1g",
+        "/e/f~0g",
+        "/d/2/3",
+        "/d/2/z",
+        "/d/2/z",
+        "/d/2/zz",
+        NULL,       // was invalid "/e/f~g"
+        NULL        // was invalid "/e/f~~g"
     };
     typedef MemoryPoolAllocator<> AllocatorType;
     typedef GenericPointer<Value, AllocatorType> PointerType;
-    typedef std::multiset<PointerType> PointerSet;
+    typedef std::multimap<PointerType, size_t> PointerMap;
+    PointerMap map;
+    PointerMap::iterator it;
     AllocatorType allocator;
-    PointerSet set;
     size_t i;
 
     EXPECT_EQ(sizeof(pointers) / sizeof(pointers[0]),
               sizeof(ordered_pointers) / sizeof(ordered_pointers[0]));
 
     for (i = 0; i < sizeof(pointers) / sizeof(pointers[0]); ++i) {
-        set.insert(PointerType(pointers[i], &allocator));
+        it = map.insert(PointerMap::value_type(PointerType(pointers[i].str, &allocator), i));
+        if (!it->first.IsValid()) {
+            EXPECT_EQ(++it, map.end());
+        }
     }
 
-    i = 0;
-    for (PointerSet::iterator it = set.begin(); it != set.end(); ++it) {
+    for (i = 0, it = map.begin(); it != map.end(); ++it, ++i) {
+        EXPECT_TRUE(it->second < sizeof(pointers) / sizeof(pointers[0]));
+        EXPECT_EQ(it->first.IsValid(), pointers[it->second].valid);
         EXPECT_TRUE(i < sizeof(ordered_pointers) / sizeof(ordered_pointers[0]));
-        EXPECT_EQ(it->IsValid(), ordered_pointers[i].valid);
-        if (it->IsValid()) {
+        EXPECT_EQ(it->first.IsValid(), !!ordered_pointers[i]);
+        if (it->first.IsValid()) {
             std::stringstream ss;
             OStreamWrapper os(ss);
-            EXPECT_TRUE(it->Stringify(os));
-            EXPECT_EQ(ss.str(), ordered_pointers[i].str);
+            EXPECT_TRUE(it->first.Stringify(os));
+            EXPECT_EQ(ss.str(), pointers[it->second].str);
+            EXPECT_EQ(ss.str(), ordered_pointers[i]);
         }
-        i++;
     }
 }