From b6363289bfb57a6674d7d37e7b3790bdf414303d Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Tue, 7 Aug 2018 13:30:52 -0700 Subject: [PATCH] Use Json::StaticString for field names Clean up some code relating to unknown fields and avoid allocate/copy/free cycles for Json objects containing serialized field names. --- src/ripple/protocol/SField.h | 33 +++++++++++--------- src/ripple/protocol/impl/SField.cpp | 45 ++++++++++++++------------- src/ripple/protocol/impl/STArray.cpp | 6 +--- src/ripple/protocol/impl/STObject.cpp | 9 +----- 4 files changed, 44 insertions(+), 49 deletions(-) diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 4a6284db2..3742fc39d 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -136,20 +136,20 @@ public: no, yes }; - static IsSigning const notSigning = IsSigning::no; + static IsSigning const notSigning = IsSigning::no; - int const fieldCode; // (type<<16)|index - SerializedTypeID const fieldType; // STI_* - int const fieldValue; // Code number for protocol - std::string fieldName; - int fieldMeta; - int fieldNum; - IsSigning const signingField; - std::string jsonName; + int const fieldCode; // (type<<16)|index + SerializedTypeID const fieldType; // STI_* + int const fieldValue; // Code number for protocol + std::string fieldName; + int fieldMeta; + int fieldNum; + IsSigning const signingField; + Json::StaticString const jsonName; SField(SField const&) = delete; SField& operator=(SField const&) = delete; - SField(SField&&) = default; + SField(SField&&); protected: // These constructors can only be called from FieldNames.cpp @@ -166,18 +166,23 @@ public: { return getField (field_code (type, value)); } + static const SField& getField (SerializedTypeID type, int value) { return getField (field_code (type, value)); } - std::string getName () const; - bool hasName () const + std::string const& getName () const { - return !fieldName.empty (); + return fieldName; } - std::string const& getJsonName () const + bool hasName () const + { + return fieldCode > 0; + } + + Json::StaticString const& getJsonName () const { return jsonName; } diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index f5457d3a8..a2bd61af5 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -277,7 +277,7 @@ SField::SField (SerializedTypeID tid, int fv, const char* fn, , fieldMeta (meta) , fieldNum (++num) , signingField (signing) - , jsonName (getName ()) + , jsonName (fieldName.c_str()) { } @@ -288,7 +288,7 @@ SField::SField (int fc) , fieldMeta (sMD_Never) , fieldNum (++num) , signingField (IsSigning::yes) - , jsonName (getName ()) + , jsonName (fieldName.c_str()) { } @@ -296,18 +296,31 @@ SField::SField (int fc) // This is naturally done with no extra expense // from getField(int code). SField::SField (SerializedTypeID tid, int fv) - : fieldCode (field_code (tid, fv)) - , fieldType (tid) - , fieldValue (fv) - , fieldMeta (sMD_Default) - , fieldNum (++num) - , signingField (IsSigning::yes) + : fieldCode (field_code (tid, fv)) + , fieldType (tid) + , fieldValue (fv) + , fieldName (std::to_string (tid) + '/' + std::to_string (fv)) + , fieldMeta (sMD_Default) + , fieldNum (++num) + , signingField (IsSigning::yes) + , jsonName (fieldName.c_str()) { - fieldName = std::to_string (tid) + '/' + std::to_string (fv); - jsonName = getName (); assert ((fv != 1) || ((tid != STI_ARRAY) && (tid != STI_OBJECT))); } +// we can't use the default move constructor because +// it could leave jsonName referencing a destroyed string +SField::SField (SField &&s) + : fieldCode (s.fieldCode) + , fieldType (s.fieldType) + , fieldValue (s.fieldValue) + , fieldMeta (s.fieldMeta) + , fieldNum (s.fieldNum) + , signingField (s.signingField) + , jsonName (fieldName.c_str()) +{ +} + SField const& SField::getField (int code) { @@ -380,18 +393,6 @@ int SField::compare (SField const& f1, SField const& f2) return 0; } -std::string SField::getName () const -{ - if (!fieldName.empty ()) - return fieldName; - - if (fieldValue == 0) - return ""; - - return std::to_string(safe_cast (fieldType)) + "/" + - std::to_string(fieldValue); -} - SField const& SField::getField (std::string const& fieldName) { diff --git a/src/ripple/protocol/impl/STArray.cpp b/src/ripple/protocol/impl/STArray.cpp index a824163f4..33f9f540d 100644 --- a/src/ripple/protocol/impl/STArray.cpp +++ b/src/ripple/protocol/impl/STArray.cpp @@ -141,16 +141,12 @@ std::string STArray::getText () const Json::Value STArray::getJson (int p) const { Json::Value v = Json::arrayValue; - int index = 1; for (auto const& object: v_) { if (object.getSType () != STI_NOTPRESENT) { Json::Value& inner = v.append (Json::objectValue); - auto const& fname = object.getFName (); - auto k = fname.hasName () ? fname.fieldName : std::to_string(index); - inner[k] = object.getJson (p); - index++; + inner [object.getFName().getJsonName()] = object.getJson (p); } } return v; diff --git a/src/ripple/protocol/impl/STObject.cpp b/src/ripple/protocol/impl/STObject.cpp index 7b954eec4..b91d18c09 100644 --- a/src/ripple/protocol/impl/STObject.cpp +++ b/src/ripple/protocol/impl/STObject.cpp @@ -658,17 +658,10 @@ Json::Value STObject::getJson (int options) const { Json::Value ret (Json::objectValue); - // TODO(tom): this variable is never changed...? - int index = 1; for (auto const& elem : v_) { if (elem->getSType () != STI_NOTPRESENT) - { - auto const& n = elem->getFName (); - auto key = n.hasName () ? std::string(n.getJsonName ()) : - std::to_string (index); - ret[key] = elem->getJson (options); - } + ret [elem->getFName().getJsonName()] = elem->getJson (options); } return ret; }