Repository: mesos Updated Branches: refs/heads/master 997e280b5 -> 7918442b6
Updated json parsing to avoid copying between picojson and stout JSON. When looking at some profiling data, it looks as though we approximately double the cost of JSON parsing due to having to convert from `picojson::value` to `JSON::Value`. Michael Park pointed me to the parsing "context" that's customizable in picojson. This patch replaces our conversion with a parsing context in order to parse directly into JSON::Value and avoid copying. Review: https://reviews.apache.org/r/67861 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7918442b Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7918442b Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7918442b Branch: refs/heads/master Commit: 7918442b6c15ca31a401247850c5ad2d10d12317 Parents: 997e280 Author: Benjamin Mahler <bmah...@apache.org> Authored: Mon Jul 9 18:21:03 2018 -0700 Committer: Benjamin Mahler <bmah...@apache.org> Committed: Tue Jul 10 12:37:08 2018 -0700 ---------------------------------------------------------------------- 3rdparty/stout/include/stout/json.hpp | 142 +++++++++++++++++++++-------- 1 file changed, 106 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/7918442b/3rdparty/stout/include/stout/json.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/json.hpp b/3rdparty/stout/include/stout/json.hpp index c374e29..62a8139 100644 --- a/3rdparty/stout/include/stout/json.hpp +++ b/3rdparty/stout/include/stout/json.hpp @@ -275,7 +275,16 @@ struct Value : internal::Variant bool is() const; template <typename T> - const T& as() const; + const T& as() const &; + + template <typename T> + T& as() &; + + template <typename T> + T&& as() &&; + + template <typename T> + const T&& as() const &&; // Returns true if and only if 'other' is contained by 'this'. // 'Other' is contained by 'this' if the following conditions are @@ -341,14 +350,42 @@ inline bool Value::is<Value>() const template <typename T> -const T& Value::as() const +const T& Value::as() const & { - return *CHECK_NOTNULL(boost::get<T>(this)); + return boost::get<T>(*this); +} + + +template <typename T> +T& Value::as() & +{ + return boost::get<T>(*this); +} + + +template <typename T> +T&& Value::as() && +{ + return std::move(boost::get<T>(*this)); +} + + +template <typename T> +const T&& Value::as() const && +{ + return std::move(boost::get<T>(*this)); } template <> -inline const Value& Value::as<Value>() const +inline const Value& Value::as<Value>() const & +{ + return *this; +} + + +template <> +inline Value& Value::as<Value>() & { return *this; } @@ -852,35 +889,63 @@ inline std::ostream& operator<<(std::ostream& stream, const Null& null) namespace internal { -inline Value convert(const picojson::value& value) -{ - if (value.is<picojson::null>()) { - return Null(); - } else if (value.is<bool>()) { - return Boolean(value.get<bool>()); - } else if (value.is<picojson::value::object>()) { - Object object; - foreachpair (const std::string& name, - const picojson::value& v, - value.get<picojson::value::object>()) { - object.values[name] = convert(v); - } - return object; - } else if (value.is<picojson::value::array>()) { - Array array; - foreach (const picojson::value& v, value.get<picojson::value::array>()) { - array.values.push_back(convert(v)); - } - return array; - } else if (value.is<int64_t>()) { - return Number(value.get<int64_t>()); - } else if (value.is<double>()) { - return Number(value.get<double>()); - } else if (value.is<std::string>()) { - return String(value.get<std::string>()); +// Our implementation of picojson's parsing context that allows +// us to parse directly into our JSON::Value. +// +// https://github.com/kazuho/picojson/blob/v1.3.0/picojson.h#L820-L870 +class ParseContext { +public: + ParseContext(Value* _value) : value(_value) {} + + ParseContext(const ParseContext&) = delete; + ParseContext &operator=(const ParseContext&) = delete; + + bool set_null() { *value = Null(); return true; } + bool set_bool(bool b) { *value = Boolean(b); return true; } + bool set_int64(int64_t i) { *value = Number(i); return true; } + + bool set_number(double f) { + // We take a trip through picojson::value here because it + // is where the validation takes place (i.e. it throws): + // https://github.com/kazuho/picojson/issues/94 + // https://github.com/kazuho/picojson/blob/v1.3.0/picojson.h#L195-L208 + picojson::value v(f); + *value = Number(v.get<double>()); + return true; } - return Null(); -} + + template <typename Iter> + bool parse_string(picojson::input<Iter>& in) { + *value = String(); + return picojson::_parse_string(value->as<String>().value, in); + } + + bool parse_array_start() { *value = Array(); return true; } + + template <typename Iter> + bool parse_array_item(picojson::input<Iter>& in, size_t) { + Array& array = value->as<Array>(); + array.values.push_back(Value()); + ParseContext context(&array.values.back()); + return picojson::_parse(context, in); + } + + bool parse_array_stop(size_t) { return true; } + + bool parse_object_start() { + *value = Object(); + return true; + } + + template <typename Iter> + bool parse_object_item(picojson::input<Iter>& in, const std::string& key) { + Object& object = value->as<Object>(); + ParseContext context(&object.values[key]); + return picojson::_parse(context, in); + } + + Value* value; +}; } // namespace internal { @@ -888,7 +953,7 @@ inline Value convert(const picojson::value& value) inline Try<Value> parse(const std::string& s) { const char* parseBegin = s.c_str(); - picojson::value value; + Value value; std::string error; // Because PicoJson supports repeated parsing of multiple objects/arrays in a @@ -907,8 +972,9 @@ inline Try<Value> parse(const std::string& s) // on parsing, see https://github.com/kazuho/picojson/issues/94 const char* parseEnd; try { + internal::ParseContext context(&value); parseEnd = - picojson::parse(value, parseBegin, parseBegin + s.size(), &error); + picojson::_parse(context, parseBegin, parseBegin + s.size(), &error); } catch (const std::overflow_error&) { return Error("Value out of range"); } catch (...) { @@ -923,7 +989,11 @@ inline Try<Value> parse(const std::string& s) + s.substr(parseEnd - parseBegin, lastVisibleChar + 1 - parseEnd)); } - return internal::convert(value); + // TODO(bmahler): Newer compilers (clang-3.9 and gcc-5.1) can + // perform a move into the resultant Try with optimization on. + // Consider removing the `std::move` when we require these + // compilers. + return std::move(value); } @@ -940,7 +1010,7 @@ Try<T> parse(const std::string& s) return Error("Unexpected JSON type parsed"); } - return value->as<T>(); + return std::move(value->as<T>()); }
