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.

include/rapidjson/pointer.h

@@ -366,20 +366,21 @@ public:
         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 (tokenCount_ != rhs.tokenCount_)
+            return tokenCount_ < rhs.tokenCount_;
 
-            if (lTok->length > rTok->length)
-                return std::memcmp(lTok->name, rTok->name, sizeof(Ch) * rTok->length) < 0;
+        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;
 
-            int comp = std::memcmp(lTok->name, rTok->name, sizeof(Ch) * lTok->length);
-            if (comp || lTok->length != rTok->length)
-                return comp <= 0;
+            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[] = {
-        "/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",
-        "/e/f.g",
-        ""
-    };
     static const struct {
         const char *str;
         bool valid;
-    } ordered_pointers[] = {
-        { "", true },
-        { "/a", true },
+    } pointers[] = {
         { "/a/b",       true },
-        { "/a/c", true },
-        { "/d/1", true },
+        { "/a",         true },
         { "/d/1",       true },
-        { "/d/2", true },
-        { "/d/2/3", true },
-        { "/d/2/z", true },
         { "/d/2/z",     true },
+        { "/d/2/3",     true },
+        { "/d/2",       true },
+        { "/a/c",       true },
+        { "/e/f~g",     false },
         { "/d/2/zz",    true },
-        { "/e/f.g", true },
-        { "/e/f~1g", true },
-        { "/e/f~1g", true }, // was "#/e/f%2fg"
+        { "/d/1",       true },
+        { "/d/2/z",     true },
+        { "/e/f~~g",    false },
         { "/e/f~0g",    true },
-        { "/e/f~g", false },
-        { "/e/f~~g", false }
+        { "/e/f~1g",    true },
+        { "/e/f.g",     true },
+        { "",           true }
+    };
+    static const char *ordered_pointers[] = {
+        "",
+        "/a",
+        "/a/b",
+        "/a/c",
+        "/d/1",
+        "/d/1",
+        "/d/2",
+        "/e/f.g",
+        "/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++;
     }
 }