Przeglądaj źródła

Merge pull request #6635 from ghalliday/issue12351

HPCC-12351 Use an internal buffer for small dynamic strings

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 10 lat temu
rodzic
commit
988aaaf9a6

+ 1 - 3
common/dllserver/thorplugin.cpp

@@ -63,8 +63,6 @@ const char * SimplePluginCtx::ctxQueryProp(const char *propName) const
 class HelperDll : public CInterface, implements ILoadedDllEntry
 {
     SharedObject so;
-    StringBuffer version;
-    StringBuffer fileLocation;
     StringAttr name;
     Linked<const IFileIO> dllFile;
     bool logLoad;
@@ -144,7 +142,7 @@ bool HelperDll::IsShared()
 
 const char * HelperDll::queryVersion() const
 {
-    return version.str();
+    return "";
 }
 
 void HelperDll::logLoaded()

+ 3 - 2
common/thorhelper/layouttrans.cpp

@@ -75,9 +75,10 @@ MappingLevel::MappingLevel(FieldMapping::List & _mappings) : topLevel(true), map
 
 MappingLevel::MappingLevel(MappingLevel * parent, char const * name, FieldMapping::List & _mappings) : topLevel(false), mappings(_mappings)
 {
+    StringAttrBuilder fullScope(scope);
     if(!parent->topLevel)
-        scope.append(parent->scope).append(scopeSeparator);
-    scope.append(name);
+        fullScope.append(parent->scope).append(scopeSeparator);
+    fullScope.append(name);
 }
 
 void MappingLevel::calculateMappings(IDefRecordElement const * diskRecord, unsigned numKeyedDisk, IDefRecordElement const * activityRecord, unsigned numKeyedActivity)

+ 1 - 1
common/thorhelper/layouttrans.ipp

@@ -90,7 +90,7 @@ private:
 
 private:
     bool topLevel;
-    StringBuffer scope;
+    StringAttr scope;
     FieldMapping::List & mappings;
 };
 

+ 4 - 3
common/thorhelper/roxiedebug.cpp

@@ -870,11 +870,12 @@ DebugActivityRecord::DebugActivityRecord (IActivityBase *_activity, unsigned _it
 {
     localCycles = 0;
     totalCycles = 0;
-    idText.append(activity->queryId());
+    StringAttrBuilder fullId(idText);
+    fullId.append(activity->queryId());
     if (iteration || channel) 
-        idText.appendf(".%d", iteration);
+        fullId.appendf(".%d", iteration);
     if (channel) 
-        idText.appendf("#%d", channel);
+        fullId.appendf("#%d", channel);
 }
 
 void DebugActivityRecord::outputId(IXmlWriter *output, const char *fieldName)

+ 1 - 1
common/thorhelper/roxiedebug.hpp

@@ -173,7 +173,7 @@ public:
     unsigned channel;
     unsigned sequence;
     IArrayOf<IDebugGraphManager> childGraphs;
-    StringBuffer idText;
+    StringAttr idText;
     Owned<IProperties> properties;
 
     IMPLEMENT_IINTERFACE;

+ 7 - 6
common/thorhelper/roxiehelper.cpp

@@ -632,21 +632,22 @@ void CSafeSocket::setHttpMode(const char *queryName, bool arrayMode, TextMarkupF
     assertex(contentHead.length()==0 && contentTail.length()==0);
     if (mlFmt==MarkupFmt_JSON)
     {
-        contentHead.append("{");
-        contentTail.append("}");
+        contentHead.set("{");
+        contentTail.set("}");
     }
     else
     {
-        contentHead.append(
+        StringAttrBuilder headText(contentHead), tailText(contentTail);
+        headText.append(
             "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
             "<soap:Envelope xmlns:soap=\"http://schemas.xmlsoap.org/soap/envelope/\">"
             "<soap:Body>");
         if (arrayMode)
         {
-            contentHead.append("<").append(queryName).append("ResponseArray>");
-            contentTail.append("</").append(queryName).append("ResponseArray>");
+            headText.append("<").append(queryName).append("ResponseArray>");
+            tailText.append("</").append(queryName).append("ResponseArray>");
         }
-        contentTail.append("</soap:Body></soap:Envelope>");
+        tailText.append("</soap:Body></soap:Envelope>");
     }
 }
 

+ 2 - 2
common/thorhelper/roxiehelper.hpp

@@ -90,8 +90,8 @@ protected:
     bool httpMode;
     bool heartbeat;
     TextMarkupFormat mlFmt;
-    StringBuffer contentHead;
-    StringBuffer contentTail;
+    StringAttr contentHead;
+    StringAttr contentTail;
     PointerArray queued;
     UnsignedArray lengths;
     unsigned sent;

+ 6 - 6
common/thorhelper/thorsoapcall.cpp

@@ -83,8 +83,8 @@ class Url : public CInterface, implements IInterface
 public:
     IMPLEMENT_IINTERFACE;
 
-    StringBuffer method;
-    StringBuffer host;
+    StringAttr method;
+    StringAttr host;
     unsigned port;
     StringBuffer path;
     StringBuffer userPasswordPair;
@@ -155,7 +155,7 @@ public:
         {
             *p = 0;
             p += 3; // skip past the colon-slash-slash
-            method.append(urltext);
+            method.set(urltext);
             urltext = p;
         }
         else
@@ -179,7 +179,7 @@ public:
 
             port = atoi(p);
 
-            host.append(urltext);
+            host.set(urltext);
 
             if ((p = strchr(p, '/')) != NULL)
                 path.append(p);
@@ -201,12 +201,12 @@ public:
                 *p = 0;
                 p++;
 
-                host.append(urltext);
+                host.set(urltext);
                 path.append("/").append(p);
             }
             else
             {
-                host.append(urltext);
+                host.set(urltext);
                 path.append("/");
             }
         }

+ 10 - 6
common/workunit/referencedfilelist.cpp

@@ -127,7 +127,11 @@ public:
     ReferencedFile(const char *lfn, const char *sourceIP, const char *srcCluster, const char *prefix, bool isSubFile, unsigned _flags, const char *_pkgid, bool noDfs, bool calcSize)
     : flags(_flags), pkgid(_pkgid), noDfsResolution(noDfs), calcFileSize(calcSize), fileSize(0), numParts(0)
     {
-        logicalName.set(skipForeign(lfn, &daliip)).toLowerCase();
+        {
+            //Scope ensures strings are assigned
+            StringAttrBuilder logicalNameText(logicalName), daliipText(daliip);
+            logicalNameText.set(skipForeign(lfn, &daliipText)).toLowerCase();
+        }
         if (daliip.length())
             flags |= RefFileForeign;
         else
@@ -157,7 +161,7 @@ public:
     virtual unsigned getFlags() const {return flags;}
     virtual const SocketEndpoint &getForeignIP(SocketEndpoint &ep) const
     {
-        if (flags & RefFileForeign && daliip.length())
+        if ((flags & RefFileForeign) && daliip.length())
             ep.set(daliip.str());
         else
             ep.set(NULL);
@@ -176,10 +180,10 @@ public:
     }
 
 public:
-    StringBuffer logicalName;
+    StringAttr logicalName;
     StringAttr pkgid;
-    StringBuffer daliip;
-    StringBuffer filePrefix;
+    StringAttr daliip;
+    StringAttr filePrefix;
     StringAttr fileSrcCluster;
     __int64 fileSize;
     unsigned numParts;
@@ -347,7 +351,7 @@ IPropertyTree *ReferencedFile::getSpecifiedOrRemoteFileTree(IUserDescriptor *use
     Owned<IPropertyTree> fileTree = getRemoteFileTree(user, remote, remotePrefix);
     if (!fileTree)
         return NULL;
-    remote->endpoint().getUrlStr(daliip);
+    StringAttrBuilder daliipText(daliip);
     filePrefix.set(remotePrefix);
     return fileTree.getClear();
 }

+ 4 - 3
common/workunit/workunit.cpp

@@ -603,8 +603,8 @@ public:
 
 public:
     StringAttr creator;
-    StringBuffer scope;
     StringAttr description;
+    StringBuffer scope;
     StatisticMeasure measure;
     StatisticKind kind;
     StatisticCreatorType creatorType;
@@ -882,7 +882,7 @@ private:
 class CLocalWUAppValue : public CInterface, implements IConstWUAppValue
 {
     Owned<IPropertyTree> p;
-    StringBuffer prop;
+    StringAttr prop;
 public:
     IMPLEMENT_IINTERFACE;
     CLocalWUAppValue(IPropertyTree *p,unsigned child);
@@ -8776,7 +8776,8 @@ void CLocalWUException::setExceptionColumn(unsigned c)
 
 CLocalWUAppValue::CLocalWUAppValue(IPropertyTree *props,unsigned child): p(props)
 {
-    prop.append("*[").append(child).append("]");
+    StringAttrBuilder propPath(prop);
+    propPath.append("*[").append(child).append("]");
 }
 
 IStringVal & CLocalWUAppValue::getApplication(IStringVal & str) const

+ 1 - 1
common/wuwebview/wuwebview.cpp

@@ -58,7 +58,7 @@ public:
             if (queryname)
                 buffer.append(queryname);
             buffer.append("Response");
-            if (flags & WWV_INCL_NAMESPACES && ns.length())
+            if ((flags & WWV_INCL_NAMESPACES) && ns.length())
                 buffer.append(" xmlns=\"").append(ns.str()).append('\"');
             buffer.append('>');
         }

+ 0 - 1
dali/base/dasds.ipp

@@ -591,7 +591,6 @@ class CXPathIterator : public CInterface, implements IPropertyTreeIterator
     UnsignedArray childPositions;
     IPropertyTree *currentChild;
     IPTIteratorCodes flags;
-    StringBuffer currentPath;
     bool validateServerIds;
 public:
     IMPLEMENT_IINTERFACE;

+ 1 - 1
deployment/configgen/main.cpp

@@ -718,7 +718,7 @@ int main(int argc, char** argv)
   const char* out_filename = NULL;
   const char* compName = NULL;
   const char* compType = NULL;
-  StringBuffer ipAddr = NULL;
+  StringBuffer ipAddr;
   bool generateOutput = true;
   bool listComps = false;
   bool verbose = false;

+ 3 - 2
ecl/eclagent/eclagent.ipp

@@ -853,7 +853,7 @@ private:
         unsigned sourceId;
         unsigned outputIndex;
 
-        StringBuffer edgeId;
+        StringAttr edgeId;
 
     public:
         IMPLEMENT_IINTERFACE;
@@ -861,7 +861,8 @@ private:
         LegacyInputProbe(IHThorInput *_in, EclSubGraph *_owner, unsigned _sourceId, int outputidx)
             : in(_in), owner(_owner), sourceId(_sourceId), outputIndex(outputidx)
         {
-            edgeId.append(_sourceId).append("_").append(outputidx);
+            StringAttrBuilder edgeIdText(edgeId);
+            edgeIdText.append(_sourceId).append("_").append(outputidx);
             maxRowSize = 0;
         }
 

+ 5 - 2
ecl/hql/hqlir.cpp

@@ -1128,13 +1128,16 @@ class TextIRBuilder : public CInterfaceOf<IEclBuilder>
         Definition(const char * prefix, id_t _id, bool expandInline) : id(_id)
         {
             if (!expandInline)
-                idText.append("%").append(prefix).append(id);
+            {
+                StringAttrBuilder s(idText);
+                s.append("%").append(prefix).append(id);
+            }
         }
 
         inline bool expandInline() const { return idText.length() == 0; }
 
     public:
-        StringBuffer idText;
+        StringAttr idText;
         id_t id;
     };
 

+ 6 - 6
esp/bindings/http/platform/httpbinding.cpp

@@ -120,12 +120,12 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const
         if (!bnd_cfg->getProp("@baseFilesPath", m_filespath))
             m_filespath.append(getCFD()).append("./files/");
 
-        m_host.append(bnd_cfg->queryProp("@netAddress"));
+        m_host.set(bnd_cfg->queryProp("@netAddress"));
         m_port = bnd_cfg->getPropInt("@port");
 
         const char *realm = bnd_cfg->queryProp("@realm");
         if (realm)
-            m_realm.append(realm);
+            m_realm.set(realm);
         else
         {
             StringBuffer xpath;
@@ -136,7 +136,7 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const
                 const char *type = bindings->query().queryProp("@type");
                 if (type && !stricmp(type, "ws_smcSoapBinding"))
                 {
-                    m_realm.append("EclWatch");
+                    m_realm.set("EclWatch");
                     break;
                 }
             }
@@ -156,7 +156,7 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const
                     }
                 }
                 
-                m_realm.append((realm) ? realm : "EspServices");
+                m_realm.set((realm) ? realm : "EspServices");
             }
         }
 
@@ -177,8 +177,8 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const
         Owned<IPropertyTree> authcfg = bnd_cfg->getPropTree("Authenticate");
         if(authcfg != NULL)
         {
-            authcfg->getProp("@type", m_authtype);
-            authcfg->getProp("@method", m_authmethod);
+            m_authtype.set(authcfg->queryProp("@type"));
+            m_authmethod.set(authcfg->queryProp("@method"));
             PROGLOG("Authenticate method=%s", m_authmethod.str());
             if(stricmp(m_authmethod.str(), "LdapSecurity") == 0)
             {

+ 15 - 15
esp/bindings/http/platform/httpbinding.hpp

@@ -30,9 +30,9 @@
 class CMethodInfo : public CInterface
 {
 public:
-    StringBuffer m_label;
-    StringBuffer m_requestLabel;
-    StringBuffer m_responseLabel;
+    StringAttr m_label;
+    StringAttr m_requestLabel;
+    StringAttr m_responseLabel;
 
     //StringBuffer m_securityTag;
     //StringBuffer m_optionalTag;
@@ -43,9 +43,9 @@ public:
     IMPLEMENT_IINTERFACE;
     CMethodInfo(const char * label, const char * req, const char * resp) //, const char *sectag, const char *optag,const char* minver=NULL, const char* maxver=NULL)
     {
-        m_label.append(label);
-        m_requestLabel.append(req);
-        m_responseLabel.append(resp);
+        m_label.set(label);
+        m_requestLabel.set(req);
+        m_responseLabel.set(resp);
         /*
         if (sectag)
             m_securityTag.append(sectag);
@@ -58,9 +58,9 @@ public:
 
     CMethodInfo(CMethodInfo &src)
     {
-        m_label.append(src.m_label);
-        m_requestLabel.append(src.m_requestLabel);
-        m_responseLabel.append(src.m_responseLabel);
+        m_label.set(src.m_label);
+        m_requestLabel.set(src.m_requestLabel);
+        m_responseLabel.set(src.m_responseLabel);
     };
 };
 
@@ -119,8 +119,8 @@ class esp_http_decl EspHttpBinding :
     implements IEspWsdlSections
 {
 private:
-    StringBuffer            m_host;
-    StringBuffer            m_realm;
+    StringAttr              m_host;
+    StringAttr              m_realm;
     unsigned short          m_port;
     bool                    m_viewConfig;
     bool                    m_formOptions;
@@ -129,8 +129,8 @@ private:
 
     HINSTANCE               m_hSecDll;
 
-    StringBuffer            m_authtype;
-    StringBuffer            m_authmethod;
+    StringAttr              m_authtype;
+    StringAttr              m_authmethod;
     StringBuffer            m_reqPath;
     StringBuffer            m_filespath;
     StringBuffer            m_wsdlAddress;
@@ -159,13 +159,13 @@ public:
     const char *getHost(){return m_host.str();}
     unsigned short getPort(){return m_port;}
 
-    void setRealm(const char *realm){m_realm.clear().append((realm) ? realm : "EspService");}
+    void setRealm(const char *realm){m_realm.set((realm) ? realm : "EspService");}
     const char *getRealm(){return m_realm.str();}
     const char* getChallengeRealm() {return m_challenge_realm.str();}
     double getWsdlVersion(){return m_wsdlVer;}
     void setWsdlVersion(double ver){m_wsdlVer=ver;}
     const char *getWsdlAddress(){return m_wsdlAddress.str();}
-    void setWsdlAddress(const char *wsdladdress){m_wsdlAddress.clear().append(wsdladdress);}
+    void setWsdlAddress(const char *wsdladdress){m_wsdlAddress.set(wsdladdress);}
 
     virtual void setRequestPath(const char *path);
     virtual bool rootAuthRequired();

+ 10 - 7
esp/bindings/http/platform/httptransport.cpp

@@ -1087,7 +1087,7 @@ StringBuffer& CHttpRequest::getMethod(StringBuffer & method)
 
 void CHttpRequest::setMethod(const char* method)
 {
-    m_httpMethod.clear().append(method);
+    m_httpMethod.set(method);
 }
 
 StringBuffer& CHttpRequest::getPath(StringBuffer & path)
@@ -1097,7 +1097,7 @@ StringBuffer& CHttpRequest::getPath(StringBuffer & path)
 
 void CHttpRequest::setPath(const char* path)
 {
-    m_httpPath.clear().append(path);
+    m_httpPath.set(path);
 }
 
 void CHttpRequest::parseQueryString(const char* querystr)
@@ -1363,6 +1363,7 @@ void CHttpRequest::parseEspPathInfo()
             m_sstype=(m_queryparams && m_queryparams->hasProp("main")) ? sub_serv_main : sub_serv_root;
         else
         {
+            //MORE: With a couple of changes there would be need to clone pathstr
             char *pathstr = strdup(m_httpPath.str());
             char *finger = pathstr;
             
@@ -1398,23 +1399,25 @@ void CHttpRequest::parseEspPathInfo()
                         if (pathex)
                         {
                             *pathex=0;
-                            m_espPathEx.append(pathex+1);
+                            m_espPathEx.set(pathex+1);
                         }
                             
                         *thumb=0;
-                        m_espMethodName.append(++thumb);
-                        const char *tail = strrchr(thumb, '.');
+                        const char * method = thumb+1;
+                        const char *tail = strrchr(method, '.');
                         ESPSerializationFormat fmt = lookupResponseFormatByExtension(tail);
                         if (fmt!=ESPSerializationANY)
                         {
                             m_context->setResponseFormat(fmt);
-                            m_espMethodName.setLength(tail-thumb);
+                            m_espMethodName.set(method, tail-method);
                         }
+                        else
+                            m_espMethodName.set(method);
                     }
                     else 
                         missingTrailSlash = true; 
 
-                    m_espServiceName.append(finger);
+                    m_espServiceName.set(finger);
                 }
             }
             

+ 6 - 6
esp/bindings/http/platform/httptransport.ipp

@@ -282,13 +282,13 @@ const char* getSubServiceDesc(sub_service stype);
 class CHttpRequest : public CHttpMessage
 {
 private:
-    StringBuffer    m_httpMethod;
-    StringBuffer    m_httpPath;
-    bool            m_pathIsParsed;
-    StringBuffer    m_espServiceName;
-    StringBuffer    m_espMethodName;
-    StringBuffer    m_espPathEx;
+    StringAttr    m_httpMethod;
+    StringAttr    m_httpPath;
+    StringAttr    m_espServiceName;
+    StringAttr    m_espMethodName;
+    StringAttr    m_espPathEx;
     sub_service     m_sstype;
+    bool            m_pathIsParsed;
     bool            m_authrequired;
     int             m_MaxRequestEntityLength;
     ESPSerializationFormat respSerializationFormat;

+ 13 - 20
esp/services/ws_machine/ws_machineService.hpp

@@ -187,25 +187,20 @@ public:
 
 class CProcessData : public CInterface
 {
-    StringBuffer    m_type;
-    StringBuffer    m_name;
-    StringBuffer    m_path;
+    StringAttr    m_type;
+    StringAttr    m_name;
+    StringAttr    m_path;
     unsigned        m_processNumber;
     bool            m_multipleInstances;     //required from ProcessFilter in environment.xml
 
-    StringBuffer    m_pid;
-    StringBuffer    m_upTime;
+    StringAttr    m_pid;
+    StringAttr    m_upTime;
     set<string>     m_dependencies;
 public:
 IMPLEMENT_IINTERFACE;
 
 	CProcessData()
     {
-        m_name.clear();
-        m_type.clear();
-        m_path.clear();
-        m_pid.clear();
-        m_upTime.clear();
         m_processNumber = 0;
         m_multipleInstances = false;
         m_dependencies.clear();
@@ -214,11 +209,9 @@ IMPLEMENT_IINTERFACE;
     CProcessData(const char* name, const char* type, const char* path, unsigned processNumber):
         m_processNumber(processNumber)
     {
-        m_name = name;
-        m_type = type;
-        m_path = path;
-        m_pid.clear();
-        m_upTime.clear();
+        m_name.set(name);
+        m_type.set(type);
+        m_path.set(path);
         m_multipleInstances = false;
         m_dependencies.clear();
     }
@@ -226,7 +219,7 @@ IMPLEMENT_IINTERFACE;
 
     void setName(const char* name)
     {
-        m_name.clear().append(name);
+        m_name.set(name);
     }
 
     const char* getName()
@@ -236,7 +229,7 @@ IMPLEMENT_IINTERFACE;
 
     void setType(const char* type)
     {
-        m_type.clear().append(type);
+        m_type.set(type);
     }
 
     const char* getType()
@@ -246,7 +239,7 @@ IMPLEMENT_IINTERFACE;
 
     void setPath(const char* path)
     {
-        m_path.clear().append(path);
+        m_path.set(path);
     }
 
     const char* getPath()
@@ -256,7 +249,7 @@ IMPLEMENT_IINTERFACE;
 
     void setPID(const char* pid)
     {
-        m_pid.clear().append(pid);
+        m_pid.set(pid);
     }
 
     const char* getPID()
@@ -266,7 +259,7 @@ IMPLEMENT_IINTERFACE;
 
     void setUpTime(const char* upTime)
     {
-        m_upTime.clear().append(upTime);
+        m_upTime.set(upTime);
     }
 
     const char* getUpTime()

+ 180 - 63
system/jlib/jstring.cpp

@@ -87,16 +87,35 @@ StringBuffer::StringBuffer(const StringBuffer & value)
     append(value);
 }
 
-void    StringBuffer::setBuffer(size32_t buffLen, char * newBuff, size32_t strLen)
+StringBuffer::StringBuffer(bool useInternal)
 {
-    assertex(buffLen>0 && newBuff!=NULL && strLen<buffLen);
+    if (useInternal)
+        init();
+    else
+        initNoInternal();
+}
+
+StringBuffer::~StringBuffer()
+{
+    dbgassertex(buffer);
+    freeBuffer();
+}
 
-    if (buffer)
+void StringBuffer::freeBuffer()
+{
+    if (buffer != internalBuffer)
         free(buffer);
+}
+
+void StringBuffer::setBuffer(size32_t buffLen, char * newBuff, size32_t strLen)
+{
+    assertex(newBuff);
+    assertex(buffLen>0 && strLen<buffLen);
 
+    freeBuffer();
     buffer = newBuff;
-    maxLen=buffLen;
-    curLen=strLen;
+    maxLen = buffLen;
+    curLen = strLen;
 }
 
 void StringBuffer::_realloc(size32_t newLen)
@@ -118,13 +137,16 @@ void StringBuffer::_realloc(size32_t newLen)
                 newMax += newMax;
         }
         char * newStr;
-        if(!newMax || !(newStr=(char *)realloc(buffer, newMax)))
+        char * originalBuffer = (buffer == internalBuffer) ? NULL : buffer;
+        if (!newMax || !(newStr=(char *)realloc(originalBuffer, newMax)))
         {
             DBGLOG("StringBuffer::_realloc: Failed to realloc = %d, oldMax = %d", newMax, maxLen);
             PrintStackReport();
             PrintMemoryReport();
             throw MakeStringException(MSGAUD_operator, -1, "StringBuffer::_realloc: Failed to realloc = %d, oldMax = %d", newMax, maxLen);
-        }       
+        }
+        if (useInternal())
+            memcpy(newStr, internalBuffer, curLen);
         buffer = newStr;
         maxLen = newMax;
     }
@@ -133,16 +155,22 @@ void StringBuffer::_realloc(size32_t newLen)
 
 char * StringBuffer::detach()
 {
-    if (buffer)
+    dbgassertex(buffer);
+    char * result;
+    if (buffer == internalBuffer)
+    {
+        result = (char *)malloc(curLen+1);
+        memcpy(result, buffer, curLen);
+    }
+    else
     {
         if (maxLen>curLen+1+DETACH_GRANULARITY)
             buffer = (char *)realloc(buffer,curLen+1); // shrink
-        buffer[curLen] = '\0';          // There is always room for this null
-        char *ret = buffer;
-        init();
-        return ret;
+        result = buffer;
     }
-    return strdup(TheNullStr);
+    result[curLen] = '\0';          // There is always room for this null
+    init();
+    return result;
 }
 
 StringBuffer & StringBuffer::append(char value)
@@ -461,7 +489,7 @@ void StringBuffer::setLength(unsigned len)
     curLen = len;
 }
 
-char *  StringBuffer::reserve(size32_t size)
+char * StringBuffer::reserve(size32_t size)
 {
     ensureCapacity(size);
     char *ret = buffer+curLen;
@@ -469,10 +497,23 @@ char *  StringBuffer::reserve(size32_t size)
     return ret;
 }
 
-char *  StringBuffer::reserveTruncate(size32_t size)
+char * StringBuffer::reserveTruncate(size32_t size)
 {
     size32_t newMax = curLen+size+1;
-    if (newMax != maxLen) {
+    if (buffer == internalBuffer)
+    {
+        if (newMax > InternalBufferSize)
+        {
+            char * newStr = (char *)malloc(newMax);
+            if (!newStr)
+                throw MakeStringException(-1, "StringBuffer::_realloc: Failed to realloc newMax = %d, oldMax = %d", newMax, maxLen);
+            memcpy(newStr, buffer, curLen);
+            buffer = newStr;
+            maxLen = newMax;
+        }
+    }
+    else if (newMax != maxLen)
+    {
         char * newStr = (char *) realloc(buffer, newMax);
         if (!newStr)
             throw MakeStringException(-1, "StringBuffer::_realloc: Failed to realloc newMax = %d, oldMax = %d", newMax, maxLen);
@@ -486,26 +527,84 @@ char *  StringBuffer::reserveTruncate(size32_t size)
 
 void StringBuffer::swapWith(StringBuffer &other)
 {
-    size32_t tmpsz = curLen;
-    curLen = other.curLen;
-    other.curLen = tmpsz;
-    tmpsz = maxLen;
+    //Swap max
+    size_t tempMax = maxLen;
     maxLen = other.maxLen;
-    other.maxLen = tmpsz;
-    char *tmpbuf = buffer;
-    buffer = other.buffer;
-    other.buffer = tmpbuf;
+    other.maxLen = tempMax;
+
+    //Swap lengths
+    size32_t thisLen = curLen;
+    size32_t otherLen = other.curLen;
+    curLen = otherLen;
+    other.curLen = thisLen;
+
+    //Swap buffers
+    char * thisBuffer = buffer;
+    char * otherBuffer = other.buffer;
+    if (useInternal())
+    {
+        if (other.useInternal())
+        {
+            //NOTE: The c++ compiler can generate better code for the fixed size memcpy than it can
+            //      if only the required characters are copied.
+            char temp[InternalBufferSize];
+            memcpy(temp, thisBuffer, InternalBufferSize);
+            memcpy(thisBuffer, otherBuffer, InternalBufferSize);
+            memcpy(otherBuffer, temp, InternalBufferSize);
+            //buffers already point in the correct place
+        }
+        else
+        {
+            memcpy(other.internalBuffer, thisBuffer, InternalBufferSize);
+            buffer = otherBuffer;
+            other.buffer = other.internalBuffer;
+        }
+    }
+    else
+    {
+        if (other.useInternal())
+        {
+            memcpy(internalBuffer, otherBuffer, InternalBufferSize);
+            buffer = internalBuffer;
+            other.buffer = thisBuffer;
+        }
+        else
+        {
+            buffer = otherBuffer;
+            other.buffer = thisBuffer;
+        }
+    }
+}
+
+void StringBuffer::setown(StringBuffer &other)
+{
+    maxLen = other.maxLen;
+    curLen = other.curLen;
+    freeBuffer();
+    if (other.useInternal())
+    {
+        memcpy(internalBuffer, other.internalBuffer, InternalBufferSize);
+        buffer = internalBuffer;
+    }
+    else
+    {
+        buffer = other.buffer;
+    }
+    other.init();
 }
 
 void StringBuffer::kill()
 {
-    if (buffer)
-        free(buffer);
+    freeBuffer();
     init();
 }
 
 void StringBuffer::getChars(int srcBegin, int srcEnd, char * target) const
 {
+    if (srcBegin < 0)
+        srcBegin = 0;
+    if ((unsigned)srcEnd > curLen)
+        srcEnd = curLen;
     const int len = srcEnd - srcBegin;
     if (target && buffer && len > 0)
         memcpy(target, buffer + srcBegin, len);
@@ -760,43 +859,40 @@ void StringBuffer::setCharAt(unsigned offset, char value)
 
 StringBuffer & StringBuffer::toLowerCase()
 {
-    if (buffer)
+    size32_t l = curLen;
+    for (size32_t i = 0; i < l; i++)
     {
-        int l = curLen;
-        for (int i = 0; i < l; i++)
-            if (isupper(buffer[i]))
-                buffer[i] = tolower(buffer[i]);
+        if (isupper(buffer[i]))
+            buffer[i] = tolower(buffer[i]);
     }
     return *this;
 }
 
 StringBuffer & StringBuffer::toUpperCase()
 {
-    if (buffer)
+    size32_t l = curLen;
+    for (size32_t i = 0; i < l; i++)
     {
-        int l = curLen;
-        for (int i = 0; i < l; i++)
-            if (islower(buffer[i]))
-                buffer[i] = toupper(buffer[i]);
+        if (islower(buffer[i]))
+            buffer[i] = toupper(buffer[i]);
     }
     return *this;
 }
 
 StringBuffer & StringBuffer::replace(char oldChar, char newChar)
 {
-    if (buffer)
+    size32_t l = curLen;
+    for (size32_t i = 0; i < l; i++)
     {
-        int l = curLen;
-        for (int i = 0; i < l; i++)
-            if (buffer[i] == oldChar)
-         {
-                buffer[i] = newChar;
+        if (buffer[i] == oldChar)
+        {
+            buffer[i] = newChar;
             if (newChar == '\0')
             {
-                 curLen = i;
-               break;
+                curLen = i;
+                break;
             }
-         }
+        }
     }
     return *this;
 }
@@ -804,7 +900,7 @@ StringBuffer & StringBuffer::replace(char oldChar, char newChar)
 // this method will replace all occurrances of "oldStr" with "newStr"
 StringBuffer & StringBuffer::replaceString(const char* oldStr, const char* newStr)
 {
-    if (buffer)
+    if (curLen)
     {
         const char* s = str();  // get null terminated version of the string
         int left = length();
@@ -841,30 +937,24 @@ StringBuffer & StringBuffer::replaceString(const char* oldStr, const char* newSt
 
 StringBuffer & StringBuffer::stripChar(char oldChar)
 {
-    if (buffer)
+    size32_t delta = 0;
+    size32_t l = curLen;
+    for (size32_t i = 0; i < l; i++)
     {
-        size32_t delta = 0;
-        size32_t l = curLen;
-        for (size32_t i = 0; i < l; i++)
-        {
-            if (buffer[i] == oldChar)
-                delta++;
-            else if (delta)
-                buffer[i-delta] = buffer[i];
-        }
-        curLen = curLen - delta;
+        if (buffer[i] == oldChar)
+            delta++;
+        else if (delta)
+            buffer[i-delta] = buffer[i];
     }
+    if (delta)
+        curLen = curLen - delta;
     return *this;
 }
 
 const char * StringBuffer::toCharArray() const
 {
-    if (buffer)
-    {
-        buffer[curLen] = '\0';          // There is always room for this null
-        return buffer;
-    }
-    return TheNullStr;
+    buffer[curLen] = '\0';          // There is always room for this null
+    return buffer;
 }
 
 //===========================================================================
@@ -879,6 +969,17 @@ VStringBuffer::VStringBuffer(const char* format, ...)
 
 //===========================================================================
 
+StringAttrBuilder::StringAttrBuilder(StringAttr & _target) : StringBuffer(false), target(_target)
+{
+}
+
+StringAttrBuilder::~StringAttrBuilder()
+{
+    target.setown(*this);
+}
+
+//===========================================================================
+
 
 String::String()
 {
@@ -1197,6 +1298,22 @@ void StringAttr::setown(const char * _text)
   text = (char *)_text;
 }
 
+void StringAttr::set(const StringBuffer & source)
+{
+    if (source.length())
+        set(source.str());
+    else
+        clear();
+}
+
+void StringAttr::setown(StringBuffer & source)
+{
+    if (source.length())
+        setown(source.detach());
+    else
+        clear();
+}
+
 void StringAttr::toLowerCase()
 {
     if (text)

+ 28 - 5
system/jlib/jstring.hpp

@@ -33,16 +33,17 @@ interface IFile;
 
 class jlib_decl StringBuffer
 {
+    enum { InternalBufferSize = 16 };
 public:
     StringBuffer();
     StringBuffer(String & value);
     StringBuffer(const char *value);
     StringBuffer(unsigned len, const char *value);
     StringBuffer(const StringBuffer & value);
-    inline ~StringBuffer() { free(buffer); }
+    StringBuffer(bool useInternal);
+    ~StringBuffer();
 
     inline size32_t length() const                      { return curLen; }
-    inline void     Release() const                     { delete this; }    // for consistency even though not link counted
     void            setLength(unsigned len);
     inline void     ensureCapacity(unsigned max)        { if (maxLen <= curLen + max) _realloc(curLen + max); }
     
@@ -53,7 +54,6 @@ public:
     StringBuffer &  append(const IAtom * value);
     StringBuffer &  append(unsigned len, const char * value);
     StringBuffer &  append(const char * value, int offset, int len);
-//  StringBuffer &  append(const unsigned char * value, int offset, int len);
     StringBuffer &  append(double value);
     StringBuffer &  append(float value);
     StringBuffer &  append(int value);
@@ -115,6 +115,7 @@ public:
     StringBuffer &  replaceString(const char* oldStr, const char* newStr);
     char *          reserve(size32_t size);
     char *          reserveTruncate(size32_t size);
+    void            setown(StringBuffer &other);
     StringBuffer &  stripChar(char oldChar);
     void            swapWith(StringBuffer &other);
     void setBuffer(size32_t buffLen, char * newBuff, size32_t strLen);
@@ -130,7 +131,6 @@ public:
         return clear().append(value.str());
     }
 
-
     StringBuffer &  appendlong(long value);
     StringBuffer &  appendulong(unsigned long value);
 private: // long depreciated
@@ -139,17 +139,26 @@ private: // long depreciated
     StringBuffer &  insert(int offset, long value);
 
 protected:
+    inline bool useInternal() const { return buffer == internalBuffer; }
     void init()
     {
+        buffer = internalBuffer;
+        curLen = 0;
+        maxLen = InternalBufferSize;
+    }
+    void initNoInternal()
+    {
         buffer = NULL;
         curLen = 0;
         maxLen = 0;
     }
+    void freeBuffer();
     void _insert(unsigned offset, size32_t insertLen);
     void _realloc(size32_t newLen);
 
 private:    
-    mutable char *  buffer;
+    char                internalBuffer[InternalBufferSize];
+    char *              buffer;
     size32_t            curLen;
     size32_t            maxLen;
 };
@@ -247,11 +256,14 @@ public:
     inline const char * get(void) const         { return text; }
     inline size32_t     length() const          { return text ? (size32_t)strlen(text) : 0; }
     inline bool isEmpty() const                 { return !text||!*text; } // faster than (length==0)
+    inline const char * str(void) const         { return text ? text : ""; } // safe form (doesn't return NULL)
     inline const char * sget(void) const        { return text ? text : ""; } // safe form of get (doesn't return NULL)
 
     void         set(const char * _text);
     void         setown(const char * _text);
     void         set(const char * _text, unsigned _len);
+    void         set(const StringBuffer & source);
+    void         setown(StringBuffer & source);
     void         toLowerCase();
     void         toUpperCase();
     
@@ -262,6 +274,17 @@ private:
     StringAttr &operator = (const StringAttr & from);
 };
 
+
+class jlib_decl StringAttrBuilder : public StringBuffer
+{
+public:
+    StringAttrBuilder(StringAttr & _target);
+    ~StringAttrBuilder();
+
+protected:
+    StringAttr & target;
+};
+
 class jlib_decl StringAttrAdaptor : public CInterface, implements IStringVal
 {
 public:

+ 39 - 0
testing/unittests/jlibtests.cpp

@@ -235,4 +235,43 @@ protected:
 CPPUNIT_TEST_SUITE_REGISTRATION( JlibFileIOTest );
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( JlibFileIOTest, "JlibFileIOTest" );
 
+/* =========================================================== */
+
+class JlibStringBufferTest : public CppUnit::TestFixture
+{
+    CPPUNIT_TEST_SUITE( JlibStringBufferTest );
+        CPPUNIT_TEST(testSwap);
+    CPPUNIT_TEST_SUITE_END();
+
+public:
+    JlibStringBufferTest()
+    {
+    }
+
+    void testSwap()
+    {
+        StringBuffer l;
+        StringBuffer r;
+        for (unsigned len=0; len<40; len++)
+        {
+            const unsigned numIter = 100000000;
+            cycle_t start = get_cycles_now();
+
+            for (unsigned pass=0; pass < numIter; pass++)
+            {
+                l.swapWith(r);
+            }
+            cycle_t elapsed = get_cycles_now() - start;
+            fprintf(stdout, "iterations of size %u took %.2f\n", len, (double)cycle_to_nanosec(elapsed) / numIter);
+            l.append("a");
+            r.append("b");
+        }
+    }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION( JlibStringBufferTest );
+CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( JlibStringBufferTest, "JlibStringBufferTest" );
+
+
 #endif // _USE_CPPUNIT