Index: base/json/json_parser.cc |
diff --git a/base/json/json_parser.cc b/base/json/json_parser.cc |
index 801c84a24cad7d344e6266cd2ccf309b4c976c06..c1bcf4a92758ed6dc6dfcbb332ffa4cd3c7a0bee 100644 |
--- a/base/json/json_parser.cc |
+++ b/base/json/json_parser.cc |
@@ -5,10 +5,11 @@ |
#include "base/json/json_parser.h" |
#include <cmath> |
-#include <memory> |
+#include <utility> |
#include "base/logging.h" |
#include "base/macros.h" |
+#include "base/memory/ptr_util.h" |
#include "base/strings/string_number_conversions.h" |
#include "base/strings/string_piece.h" |
#include "base/strings/string_util.h" |
@@ -27,16 +28,19 @@ const int kStackMaxDepth = 100; |
const int32_t kExtendedASCIIStart = 0x80; |
-// This and the class below are used to own the JSON input string for when |
-// string tokens are stored as StringPiece instead of std::string. This |
-// optimization avoids about 2/3rds of string memory copies. The constructor |
-// takes ownership of the input string. The real root value is Swap()ed into |
-// the new instance. |
+// DictionaryHiddenRootValue and ListHiddenRootValue are used in conjunction |
+// with JSONStringValue as an optimization for reducing the number of string |
+// copies. When this optimization is active, the parser uses a hidden root to |
+// keep the original JSON input string live and creates JSONStringValue children |
+// holding StringPiece references to the input string, avoiding about 2/3rds of |
+// string memory copies. The real root value is Swap()ed into the new instance. |
class DictionaryHiddenRootValue : public DictionaryValue { |
public: |
- DictionaryHiddenRootValue(std::string* json, Value* root) : json_(json) { |
+ DictionaryHiddenRootValue(std::unique_ptr<std::string> json, |
+ std::unique_ptr<Value> root) |
+ : json_(std::move(json)) { |
DCHECK(root->IsType(Value::TYPE_DICTIONARY)); |
- DictionaryValue::Swap(static_cast<DictionaryValue*>(root)); |
+ DictionaryValue::Swap(static_cast<DictionaryValue*>(root.get())); |
danakj
2016/04/28 17:39:37
This should be passing the unique_ptr. Can you TOD
dcheng
2016/04/28 18:03:23
Should it though? It's not really moving/taking ow
danakj
2016/04/28 18:09:19
Oh I guess you're right yah, it's swapping stuff i
|
} |
void Swap(DictionaryValue* other) override { |
@@ -44,7 +48,7 @@ class DictionaryHiddenRootValue : public DictionaryValue { |
// First deep copy to convert JSONStringValue to std::string and swap that |
// copy with |other|, which contains the new contents of |this|. |
- std::unique_ptr<DictionaryValue> copy(DeepCopy()); |
+ std::unique_ptr<DictionaryValue> copy(CreateDeepCopy()); |
copy->Swap(other); |
// Then erase the contents of the current dictionary and swap in the |
@@ -71,7 +75,7 @@ class DictionaryHiddenRootValue : public DictionaryValue { |
if (!DictionaryValue::RemoveWithoutPathExpansion(key, &out_owned)) |
return false; |
- out->reset(out_owned->DeepCopy()); |
+ *out = out_owned->CreateDeepCopy(); |
return true; |
} |
@@ -84,9 +88,11 @@ class DictionaryHiddenRootValue : public DictionaryValue { |
class ListHiddenRootValue : public ListValue { |
public: |
- ListHiddenRootValue(std::string* json, Value* root) : json_(json) { |
+ ListHiddenRootValue(std::unique_ptr<std::string> json, |
+ std::unique_ptr<Value> root) |
+ : json_(std::move(json)) { |
DCHECK(root->IsType(Value::TYPE_LIST)); |
- ListValue::Swap(static_cast<ListValue*>(root)); |
+ ListValue::Swap(static_cast<ListValue*>(root.get())); |
danakj
2016/04/28 17:39:37
ditto
dcheng
2016/04/28 18:03:23
See previous.
|
} |
void Swap(ListValue* other) override { |
@@ -94,7 +100,7 @@ class ListHiddenRootValue : public ListValue { |
// First deep copy to convert JSONStringValue to std::string and swap that |
// copy with |other|, which contains the new contents of |this|. |
- std::unique_ptr<ListValue> copy(DeepCopy()); |
+ std::unique_ptr<ListValue> copy(CreateDeepCopy()); |
copy->Swap(other); |
// Then erase the contents of the current list and swap in the new contents, |
@@ -117,7 +123,7 @@ class ListHiddenRootValue : public ListValue { |
if (!ListValue::Remove(index, &out_owned)) |
danakj
2016/04/28 17:39:37
heh, we should just have this return std::unique_p
dcheng
2016/04/28 18:03:23
Acknowledged.
|
return false; |
- out->reset(out_owned->DeepCopy()); |
+ *out = out_owned->CreateDeepCopy(); |
return true; |
} |
@@ -133,10 +139,8 @@ class ListHiddenRootValue : public ListValue { |
// otherwise the referenced string will not be guaranteed to outlive it. |
class JSONStringValue : public Value { |
public: |
- explicit JSONStringValue(const StringPiece& piece) |
- : Value(TYPE_STRING), |
- string_piece_(piece) { |
- } |
+ explicit JSONStringValue(StringPiece piece) |
+ : Value(TYPE_STRING), string_piece_(piece) {} |
danakj
2016/04/28 17:39:37
nit: move piece? (i know its the same as copy atm)
dcheng
2016/04/28 18:03:23
Why though? copy on a StringPiece will be cheaper
jbroman
2016/04/28 18:06:18
nit-nit: base::StringPiece explicitly promises in
danakj
2016/04/28 18:09:19
Ah I see I see. #newtostrings. LGTM
|
// Overridden from Value: |
bool GetAsString(std::string* out_value) const override { |
@@ -203,13 +207,13 @@ JSONParser::JSONParser(int options) |
JSONParser::~JSONParser() { |
} |
-Value* JSONParser::Parse(const StringPiece& input) { |
+std::unique_ptr<Value> JSONParser::Parse(StringPiece input) { |
std::unique_ptr<std::string> input_copy; |
// If the children of a JSON root can be detached, then hidden roots cannot |
// be used, so do not bother copying the input because StringPiece will not |
// be used anywhere. |
if (!(options_ & JSON_DETACHABLE_CHILDREN)) { |
- input_copy.reset(new std::string(input.as_string())); |
+ input_copy = WrapUnique(new std::string(input.as_string())); |
start_pos_ = input_copy->data(); |
} else { |
start_pos_ = input.data(); |
@@ -236,14 +240,14 @@ Value* JSONParser::Parse(const StringPiece& input) { |
// Parse the first and any nested tokens. |
std::unique_ptr<Value> root(ParseNextToken()); |
- if (!root.get()) |
- return NULL; |
+ if (!root) |
+ return nullptr; |
// Make sure the input stream is at an end. |
if (GetNextToken() != T_END_OF_INPUT) { |
if (!CanConsume(1) || (NextChar() && GetNextToken() != T_END_OF_INPUT)) { |
ReportError(JSONReader::JSON_UNEXPECTED_DATA_AFTER_ROOT, 1); |
- return NULL; |
+ return nullptr; |
} |
} |
@@ -251,19 +255,21 @@ Value* JSONParser::Parse(const StringPiece& input) { |
// hidden root. |
if (!(options_ & JSON_DETACHABLE_CHILDREN)) { |
if (root->IsType(Value::TYPE_DICTIONARY)) { |
- return new DictionaryHiddenRootValue(input_copy.release(), root.get()); |
+ return WrapUnique(new DictionaryHiddenRootValue(std::move(input_copy), |
+ std::move(root))); |
} else if (root->IsType(Value::TYPE_LIST)) { |
- return new ListHiddenRootValue(input_copy.release(), root.get()); |
+ return WrapUnique( |
+ new ListHiddenRootValue(std::move(input_copy), std::move(root))); |
} else if (root->IsType(Value::TYPE_STRING)) { |
// A string type could be a JSONStringValue, but because there's no |
// corresponding HiddenRootValue, the memory will be lost. Deep copy to |
// preserve it. |
- return root->DeepCopy(); |
+ return root->CreateDeepCopy(); |
} |
} |
// All other values can be returned directly. |
- return root.release(); |
+ return root; |
} |
JSONReader::JsonParseError JSONParser::error_code() const { |