HPCC-35227 Use peekStringList for PTree attribute binary deserialization

system/jlib/jptree.cpp

@@ -21,6 +21,7 @@
 #include <string>
 #include <tuple>
 #include <algorithm>
+#include <set>
 
 #include "platform.h"
 #include "jarray.hpp"
@@ -3070,39 +3071,50 @@ void PTree::deserializeFromStream(IBufferedSerialInputStream &src)
     }
 }
 
+static std::vector<size32_t> attrStringOffsets = []()
+{
+    std::vector<size32_t> vec;
+    // Reserve capacity to reduce allocations
+    vec.reserve(50);
+    return vec;
+}();
+
 void PTree::deserializeSelf(IBufferedSerialInputStream &src)
 {
-    size32_t len{0};
-    const char *name = queryZeroTerminatedString(src, len);
-    if (len == 0)
+    size32_t skipLen{0};
+    const char *name = queryZeroTerminatedString(src, skipLen);
+    if (skipLen == 0)
         throwUnexpectedX("PTree deserialization error: end of stream, expected name");
     setName(name);
-    src.skip(len + 1); // Skip over name and null terminator
+    src.skip(skipLen + 1); // Skip over name and null terminator
 
     read(src, flags);
 
     // Read attributes until we encounter a zero byte (attribute list terminator)
-    for (;;)
+    attrStringOffsets.clear();
+    const char * base = peekStringList(attrStringOffsets, src, skipLen);
+    if (base) // there is at least one attribute to process
     {
-        NextByteStatus status = isNextByteZero(src);
-        if (status == NextByteStatus::nextByteIsZero)
-        {
-            src.skip(1); // Skip over null terminator.
-            break;
-        }
-        if (status == NextByteStatus::endOfStream)
-            throwUnexpectedX("PTree deserialization error: end of stream, expected attribute name");
-
-        // NextByteStatus::nextByteIsNonZero - read the attribute key-value pair
-        std::pair<const char *, const char *> attrPair = peekKeyValuePair(src, len);
-        if (attrPair.second == nullptr)
+/*
+        size_t numStringOffsets = attrStringOffsets.size();
+        if (numStringOffsets % 2 != 0)
             throwUnexpectedX("PTree deserialization error: end of stream, expected attribute value");
-
+*/
+        setAttribute(base, attrStringOffsets);
+/*
         constexpr bool attributeNameNotEncoded = false; // Deserialized attribute name is in its original unencoded form
-        setAttribute(attrPair.first, attrPair.second, attributeNameNotEncoded);
+        for (size_t i = 0; i < numStringOffsets; i += 2)
+        {
+            const char *attrName = base + attrStringOffsets[i];
+            const char *attrValue = base + attrStringOffsets[i + 1];
+            setAttribute(attrName, attrValue, attributeNameNotEncoded);
+        }
+*/
 
-        src.skip(len + 1); // +1 to skip over second null terminator.
+        src.skip(skipLen); // Skip over all attributes and the terminator
     }
+    else
+        throwUnexpectedX("PTree deserialization error: end of stream, expected attribute name");
 
     if (value)
         delete value;
@@ -3881,6 +3893,59 @@ void LocalPTree::setAttribute(const char *inputkey, const char *val, bool encode
         AttrStr::destroy(goer);
 }
 
+void LocalPTree::setAttribute(const char *base, const std::vector<size32_t> &offsets)
+{
+    size_t numStringOffsets = offsets.size();
+    if (numStringOffsets % 2 != 0)
+        throwUnexpectedX("setAttribute error: offsets vector must contain pairs of key/value offsets");
+
+    for (size_t i = 0; i < numStringOffsets; i += 2)
+    {
+        const char *inputkey = base + offsets[i];
+        const char *val = base + offsets[i + 1];
+
+        if (!inputkey)
+            continue;
+        const char *key = inputkey;
+        // Note: encoded is always false for deserialized attributes
+        if (!validateXMLTag(key+1))
+            throw MakeIPTException(-1, "Invalid xml attribute: %s", key);
+        if (!val)
+            val = "";  // cannot have NULL value
+        AttrValue *v = findAttribute(key);
+        AttrStr *goer = nullptr;
+        if (v)
+        {
+            if (streq(v->value.get(), val))
+                continue;
+            goer = v->value.getPtr();
+        }
+        else
+        {
+            attrs = (AttrValue *)realloc(attrs, (numAttrs+1)*sizeof(AttrValue));
+            v = new(&attrs[numAttrs++]) AttrValue;  // Initialize new AttrValue
+            if (!v->key.set(inputkey)) //AttrStr will not return encoding marker when get() is called
+                v->key.setPtr(isnocase() ? AttrStr::createNC(inputkey) : AttrStr::create(inputkey));
+        }
+        if (arrayOwner)
+        {
+            CQualifierMap *map = arrayOwner->queryMap();
+            if (map)
+            {
+                if (goer)
+                    map->replaceEntryIfMapped(key, v->value.get(), val, this);
+                else
+                    map->insertEntryIfMapped(key, val, this);
+            }
+        }
+
+        if (!v->value.set(val))
+            v->value.setPtr(AttrStr::create(val));
+        if (goer)
+            AttrStr::destroy(goer);
+    }
+}
+
 #ifdef TRACE_STRING_SIZE
 std::atomic<__int64> AttrStr::totsize { 0 };
 std::atomic<__int64> AttrStr::maxsize { 0 };
@@ -4095,6 +4160,214 @@ void CAtomPTree::setAttribute(const char *key, const char *val, bool encoded)
     }
 }
 
+/*
+void CAtomPTree::setAttribute(const char *base, const std::vector<size32_t> &offsets)
+{
+    size_t numStringOffsets = offsets.size();
+    if (numStringOffsets % 2 != 0)
+        throwUnexpectedX("setAttribute error: offsets vector must contain pairs of key/value offsets");
+
+    CriticalBlock block(hashcrit);
+    for (size_t i = 0; i < numStringOffsets; i += 2)
+    {
+        const char *key = base + offsets[i];
+        const char *val = base + offsets[i + 1];
+
+        if (!key)
+            continue;
+        if (!validateXMLTag(key+1))
+            throw MakeIPTException(-1, "Invalid xml attribute: %s", key);
+        if (!val)
+            val = "";  // cannot have NULL value
+        AttrValue *v = findAttribute(key);
+        if (v)
+        {
+            if (streq(v->value.get(), val))
+                continue;
+            if (arrayOwner)
+            {
+                CQualifierMap *map = arrayOwner->queryMap();
+                if (map)
+                    map->replaceEntryIfMapped(key, v->value.get(), val, this);
+            }
+
+            AttrStr * goer = v->value.getPtr();
+            if (!v->value.set(val))
+            {
+                if (goer)
+                    attrHT->removeval(goer);
+                v->value.setPtr(attrHT->addval(val));
+            }
+            else if (goer)
+            {
+                attrHT->removeval(goer);
+            }
+        }
+        else
+        {
+            AttrValue *newattrs = newAttrArray(numAttrs+1);
+            if (attrs)
+            {
+                memcpy(newattrs, attrs, numAttrs*sizeof(AttrValue));
+                freeAttrArray(attrs, numAttrs);
+            }
+            if (arrayOwner)
+            {
+                CQualifierMap *map = arrayOwner->queryMap();
+                if (map)
+                    map->insertEntryIfMapped(key, val, this);
+            }
+            v = &newattrs[numAttrs];
+            if (!v->key.set(key))
+                v->key.setPtr(attrHT->addkey(key, isnocase()));
+            if (!v->value.set(val))
+                v->value.setPtr(attrHT->addval(val));
+            numAttrs++;
+            attrs = newattrs;
+        }
+    }
+}
+*/
+
+void CAtomPTree::setAttribute(const char *base, const std::vector<size32_t> &offsets)
+{
+    size_t numStringOffsets = offsets.size();
+    if (numStringOffsets == 0)
+        return;
+
+    if (numStringOffsets % 2 != 0)
+        throwUnexpectedX("setAttribute error: offsets vector must contain pairs of key/value offsets");
+
+    CriticalBlock block(hashcrit);
+
+    // Pre-pass for new attributes to allocate them in one call
+    // Count the new attributes
+    std::set<size_t> newAttributesMap;
+    std::unordered_set<size_t> *processed{nullptr};
+    for (size_t i = 0; i < numStringOffsets; i += 2)
+    {
+        const char *key = base + offsets[i];
+        if (!key)
+            continue;
+        if (!validateXMLTag(key+1))
+            throw MakeIPTException(-1, "Invalid xml attribute: %s", key);
+
+        AttrValue *v = findAttribute(key);
+        if (!v)
+            newAttributesMap.emplace(i);
+    }
+    size_t numNewAttributes = newAttributesMap.size();
+    AttrValue *newattrs{nullptr};
+    if (numNewAttributes)
+    {
+        // Allocate the new attributes in one call
+        newattrs = newAttrArray(numAttrs+numNewAttributes);
+        assertex(newattrs);
+        if (attrs)
+        {
+            memcpy(newattrs, attrs, numAttrs*sizeof(AttrValue));
+            freeAttrArray(attrs, numAttrs);
+        }
+
+        // process new additions only
+        size_t newAttributeIndex{0};
+        processed = new std::unordered_set<size_t>();
+        for (size_t i : newAttributesMap)
+        {
+            const char *key = base + offsets[i];
+            const char *val = base + offsets[i + 1];
+
+            if (!val)
+                val = "";  // cannot have NULL value
+
+            if (arrayOwner)
+            {
+                CQualifierMap *map = arrayOwner->queryMap();
+                if (map)
+                    map->insertEntryIfMapped(key, val, this);
+            }
+            AttrValue *v = &newattrs[numAttrs + newAttributeIndex];
+            assertex(v);
+            if (!v->key.set(key))
+                v->key.setPtr(attrHT->addkey(key, isnocase()));
+            if (!v->value.set(val))
+                v->value.setPtr(attrHT->addval(val));
+
+            ++newAttributeIndex;
+            processed->emplace(i);
+        }
+        attrs = newattrs;
+        numAttrs += numNewAttributes;
+    }
+
+    // Process existing attributes
+    for (size_t i = 0; i < numStringOffsets; i += 2)
+    {
+        if (processed && (processed->find(i) != processed->end())) // ignore new attributes
+            continue;
+
+        const char *key = base + offsets[i];
+        const char *val = base + offsets[i + 1];
+
+        if (!key)
+            continue;
+        if (!val)
+            val = "";  // cannot have NULL value
+        AttrValue *v = findAttribute(key);
+        if (v)
+        {
+            if (streq(v->value.get(), val))
+                continue;
+            if (arrayOwner)
+            {
+                CQualifierMap *map = arrayOwner->queryMap();
+                if (map)
+                    map->replaceEntryIfMapped(key, v->value.get(), val, this);
+            }
+
+            AttrStr * goer = v->value.getPtr();
+            if (!v->value.set(val))
+            {
+                if (goer)
+                    attrHT->removeval(goer);
+                v->value.setPtr(attrHT->addval(val));
+            }
+            else if (goer)
+            {
+                attrHT->removeval(goer);
+            }
+        }
+        else
+        {
+            assertex(false);
+
+/*
+            AttrValue *newattrs = newAttrArray(numAttrs+1);
+            if (attrs)
+            {
+                memcpy(newattrs, attrs, numAttrs*sizeof(AttrValue));
+                freeAttrArray(attrs, numAttrs);
+            }
+            if (arrayOwner)
+            {
+                CQualifierMap *map = arrayOwner->queryMap();
+                if (map)
+                    map->insertEntryIfMapped(key, val, this);
+            }
+            v = &newattrs[numAttrs];
+            if (!v->key.set(key))
+                v->key.setPtr(attrHT->addkey(key, isnocase()));
+            if (!v->value.set(val))
+                v->value.setPtr(attrHT->addval(val));
+            numAttrs++;
+            attrs = newattrs;
+*/
+
+        }
+    }
+    delete processed;
+}
+
 bool CAtomPTree::removeAttribute(const char *key)
 {
     AttrValue *del = findAttribute(key);

system/jlib/jptree.ipp

@@ -19,6 +19,8 @@
 #ifndef _PTREE_IPP
 #define _PTREE_IPP
 
+#include <vector>
+
 #include "jarray.hpp"
 #include "jexcept.hpp"
 #include "jhash.hpp"
@@ -701,6 +703,7 @@ public:
         return a->key.isEncoded();
     }
     virtual void setAttribute(const char *attr, const char *val, bool encoded) = 0;
+    virtual void setAttribute(const char *base, const std::vector<size32_t> &offsets) = 0;
 
 // IPropertyTree impl.
     virtual bool hasProp(const char * xpath) const override;
@@ -864,6 +867,7 @@ public:
     virtual unsigned queryHash() const override;
     virtual void setName(const char *_name) override;
     virtual void setAttribute(const char *attr, const char *val, bool encoded) override;
+    virtual void setAttribute(const char *base, const std::vector<size32_t> &offsets) override;
     virtual bool isEquivalent(IPropertyTree *tree) const override { return (nullptr != QUERYINTERFACE(tree, CAtomPTree)); }
     virtual IPropertyTree *create(const char *name=nullptr, IPTArrayValue *value=nullptr, ChildMap *children=nullptr, bool existing=false) override
     {
@@ -905,6 +909,7 @@ public:
     }
     virtual void setName(const char *_name) override;
     virtual void setAttribute(const char *attr, const char *val, bool encoded) override;
+    virtual void setAttribute(const char *base, const std::vector<size32_t> &offsets) override;
     virtual bool isEquivalent(IPropertyTree *tree) const override { return (nullptr != QUERYINTERFACE(tree, LocalPTree)); }
     virtual IPropertyTree *create(const char *name=nullptr, IPTArrayValue *value=nullptr, ChildMap *children=nullptr, bool existing=false) override
     {