From 642132920ffddc77d1ef7e96e7e599a852145779 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Fri, 31 May 2019 18:16:12 -0400 Subject: [PATCH] Fixing performance regression caused by helpful code contributions that moved inlineable functions into the source file combined with helpful compilers which aren't smart enough to do the inlinining in any case. --- benchmark/distinctuseridcompetition.cpp | 10 +- include/simdjson/parsedjson.h | 72 ++++++++++--- src/parsedjsoniterator.cpp | 128 ++++++------------------ 3 files changed, 93 insertions(+), 117 deletions(-) diff --git a/benchmark/distinctuseridcompetition.cpp b/benchmark/distinctuseridcompetition.cpp index a7bc9225c..9dd187897 100644 --- a/benchmark/distinctuseridcompetition.cpp +++ b/benchmark/distinctuseridcompetition.cpp @@ -35,7 +35,7 @@ void simdjson_traverse(std::vector &answer, ParsedJson::iterator &i) { case '{': if (i.down()) { do { - bool founduser = equals(i.get_string(), "user"); + bool founduser = (i.get_string_length() == 4) && (memcmp(i.get_string(), "user", 4) == 0); i.next(); // move to value if (i.is_object()) { if (founduser && i.move_to_key("id")) { @@ -97,8 +97,11 @@ void sajson_traverse(std::vector &answer, const sajson::value &node) { } case TYPE_OBJECT: { auto length = node.get_length(); + // sajson has O(log n) find_object_key, but we still visit each node anyhow for (auto i = 0u; i < length; ++i) { - if (equals(node.get_object_key(i).data(), "user")) { // found a user!!! + auto key = node.get_object_key(i); // expected: sajson::string + bool founduser = (key.length() == 4) && (memcmp(key.data(), "user", 4) == 0); + if (founduser) { // found a user!!! auto uservalue = node.get_object_value(i); // get the value if (uservalue.get_type() == TYPE_OBJECT) { // the value should be an object @@ -153,7 +156,8 @@ void rapid_traverse(std::vector &answer, const rapidjson::Value &v) { case kObjectType: for (Value::ConstMemberIterator m = v.MemberBegin(); m != v.MemberEnd(); ++m) { - if (equals(m->name.GetString(), "user")) { + bool founduser = (m->name.GetStringLength() == 4) && (memcmp(m->name.GetString(), "user", 4) == 0); + if (founduser) { const rapidjson::Value &child = m->value; if (child.GetType() == kObjectType) { for (Value::ConstMemberIterator k = child.MemberBegin(); diff --git a/include/simdjson/parsedjson.h b/include/simdjson/parsedjson.h index 3dc30fe8b..40ded9b51 100644 --- a/include/simdjson/parsedjson.h +++ b/include/simdjson/parsedjson.h @@ -124,43 +124,85 @@ public: // retrieve the character code of what we're looking at: // [{"sltfn are the possibilities - uint8_t get_type() const; + inline uint8_t get_type() const { + return current_type; // short functions should be inlined! + } // get the int64_t value at this node; valid only if we're at "l" - int64_t get_integer() const; + inline int64_t get_integer() const { + if(location + 1 >= tape_length) { + return 0;// default value in case of error + } + return static_cast(pj.tape[location + 1]); + } // get the string value at this node (NULL ended); valid only if we're at " // note that tabs, and line endings are escaped in the returned value (see print_with_escapes) // return value is valid UTF-8 // It may contain NULL chars within the string: get_string_length determines the true // string length. - const char * get_string() const; + inline const char * get_string() const { + return reinterpret_cast(pj.string_buf + (current_val & JSONVALUEMASK) + sizeof(uint32_t)) ; + } - uint32_t get_string_length() const; + // return the length of the string in bytes + inline uint32_t get_string_length() const { + uint32_t answer; + memcpy(&answer, reinterpret_cast(pj.string_buf + (current_val & JSONVALUEMASK)), sizeof(uint32_t)); + return answer; + } // get the double value at this node; valid only if // we're at "d" - double get_double() const; + inline double get_double() const { + if(location + 1 >= tape_length) { + return NAN;// default value in case of error + } + double answer; + memcpy(&answer, & pj.tape[location + 1], sizeof(answer)); + return answer; + } - bool is_object_or_array() const; - bool is_object() const; + inline bool is_object_or_array() const { + return is_object() || is_array(); + } - bool is_array() const; + inline bool is_object() const { + return get_type() == '{'; + } - bool is_string() const; + inline bool is_array() const { + return get_type() == '['; + } - bool is_integer() const; + inline bool is_string() const { + return get_type() == '"'; + } - bool is_double() const; + inline bool is_integer() const { + return get_type() == 'l'; + } - bool is_true() const; + inline bool is_double() const { + return get_type() == 'd'; + } - bool is_false() const; + inline bool is_true() const { + return get_type() == 't'; + } - bool is_null() const; + inline bool is_false() const { + return get_type() == 'f'; + } - static bool is_object_or_array(uint8_t type); + inline bool is_null() const { + return get_type() == 'n'; + } + + static bool is_object_or_array(uint8_t type) { + return ((type == '[') || (type == '{')); + } // when at {, go one level deep, looking for a given key // if successful, we are left pointing at the value, diff --git a/src/parsedjsoniterator.cpp b/src/parsedjsoniterator.cpp index b8cea9fe4..60286b748 100644 --- a/src/parsedjsoniterator.cpp +++ b/src/parsedjsoniterator.cpp @@ -105,78 +105,6 @@ bool ParsedJson::iterator::move_forward() { return true; } -uint8_t ParsedJson::iterator::get_type() const { - return current_type; -} - - -int64_t ParsedJson::iterator::get_integer() const { - if(location + 1 >= tape_length) { - return 0;// default value in case of error - } - return static_cast(pj.tape[location + 1]); -} - -double ParsedJson::iterator::get_double() const { - if(location + 1 >= tape_length) { - return NAN;// default value in case of error - } - double answer; - memcpy(&answer, & pj.tape[location + 1], sizeof(answer)); - return answer; -} - -const char * ParsedJson::iterator::get_string() const { - return reinterpret_cast(pj.string_buf + (current_val & JSONVALUEMASK) + sizeof(uint32_t)) ; -} - - -uint32_t ParsedJson::iterator::get_string_length() const { - uint32_t answer; - memcpy(&answer, reinterpret_cast(pj.string_buf + (current_val & JSONVALUEMASK)), sizeof(uint32_t)); - return answer; -} - -bool ParsedJson::iterator::is_object_or_array() const { - return is_object_or_array(get_type()); -} - -bool ParsedJson::iterator::is_object() const { - return get_type() == '{'; -} - -bool ParsedJson::iterator::is_array() const { - return get_type() == '['; -} - -bool ParsedJson::iterator::is_string() const { - return get_type() == '"'; -} - -bool ParsedJson::iterator::is_integer() const { - return get_type() == 'l'; -} - -bool ParsedJson::iterator::is_double() const { - return get_type() == 'd'; -} - -bool ParsedJson::iterator::is_true() const { - return get_type() == 't'; -} - -bool ParsedJson::iterator::is_false() const { - return get_type() == 'f'; -} - -bool ParsedJson::iterator::is_null() const { - return get_type() == 'n'; -} - -bool ParsedJson::iterator::is_object_or_array(uint8_t type) { - return (type == '[' || (type == '{')); -} - bool ParsedJson::iterator::move_to_key(const char * key) { if(down()) { do { @@ -195,24 +123,25 @@ bool ParsedJson::iterator::move_to_key(const char * key) { bool ParsedJson::iterator::next() { if ((current_type == '[') || (current_type == '{')){ - // we need to jump - size_t npos = ( current_val & JSONVALUEMASK); - if(npos >= tape_length) { + // we need to jump + size_t npos = ( current_val & JSONVALUEMASK); + if(npos >= tape_length) { return false; // shoud never happen unless at the root - } - uint64_t nextval = pj.tape[npos]; - uint8_t nexttype = (nextval >> 56); - if((nexttype == ']') || (nexttype == '}')) { + } + uint64_t nextval = pj.tape[npos]; + uint8_t nexttype = (nextval >> 56); + if((nexttype == ']') || (nexttype == '}')) { return false; // we reached the end of the scope - } - location = npos; - current_val = nextval; - current_type = nexttype; - return true; + } + location = npos; + current_val = nextval; + current_type = nexttype; + return true; } size_t increment = (current_type == 'd' || current_type == 'l') ? 2 : 1; - if(location + increment >= tape_length) { return false; -} + if(location + increment >= tape_length) { + return false; + } uint64_t nextval = pj.tape[location + increment]; uint8_t nexttype = (nextval >> 56); if((nexttype == ']') || (nexttype == '}')) { @@ -222,25 +151,25 @@ bool ParsedJson::iterator::move_to_key(const char * key) { current_val = nextval; current_type = nexttype; return true; - } bool ParsedJson::iterator::prev() { - if(location - 1 < depthindex[depth].start_of_scope) { return false; -} + if(location - 1 < depthindex[depth].start_of_scope) { + return false; + } location -= 1; current_val = pj.tape[location]; current_type = (current_val >> 56); if ((current_type == ']') || (current_type == '}')){ - // we need to jump - size_t new_location = ( current_val & JSONVALUEMASK); - if(new_location < depthindex[depth].start_of_scope) { + // we need to jump + size_t new_location = ( current_val & JSONVALUEMASK); + if(new_location < depthindex[depth].start_of_scope) { return false; // shoud never happen - } - location = new_location; - current_val = pj.tape[location]; - current_type = (current_val >> 56); + } + location = new_location; + current_val = pj.tape[location]; + current_type = (current_val >> 56); } return true; } @@ -248,7 +177,7 @@ bool ParsedJson::iterator::move_to_key(const char * key) { bool ParsedJson::iterator::up() { if(depth == 1) { - return false; // don't allow moving back to root + return false; // don't allow moving back to root } to_start_scope(); // next we just move to the previous value @@ -261,8 +190,9 @@ bool ParsedJson::iterator::move_to_key(const char * key) { bool ParsedJson::iterator::down() { - if(location + 1 >= tape_length) { return false; -} + if(location + 1 >= tape_length) { + return false; + } if ((current_type == '[') || (current_type == '{')) { size_t npos = (current_val & JSONVALUEMASK); if(npos == location + 2) {