performance-tuned stringbuf class

6 files changed, 78 insertions, 39 deletions

diff --git a/ChangeLog b/ChangeLog
index 7015922d..8a926268 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,6 @@
 ---------------------------------------------------------------------------
 Version 3.10.1 (rgerhards), 2008-01-??
+- performance-optimized string class, should bring an overall improvement
 ---------------------------------------------------------------------------
 Version 3.10.0 (rgerhards), 2008-01-07
 - implemented input module interface and initial input modules
diff --git a/msg.c b/msg.c
index a224b308..4ed53907 100644
--- a/msg.c
+++ b/msg.c
@@ -387,7 +387,7 @@ static rsRetVal MsgSerialize(msg_t *pThis, rsCStrObj **ppCStr)
 
 	assert(ppCStr != NULL);
 
-	CHKiRet(objBeginSerialize(&pCStr, (obj_t*) pThis));
+	CHKiRet(objBeginSerialize(&pCStr, (obj_t*) pThis, 1024));
 	objSerializeSCALAR(iProtocolVersion, SHORT);
 	objSerializeSCALAR(iSeverity, SHORT);
 	objSerializeSCALAR(iFacility, SHORT);
diff --git a/obj.c b/obj.c
index dc8a1ca6..eb4a7b32 100644
--- a/obj.c
+++ b/obj.c
@@ -102,10 +102,14 @@ rsRetVal objInfoSetMethod(objInfo_t *pThis, objMethod_t objMethod, rsRetVal (*pH
 
 /* begin serialization of an object - this is a very simple hook. It once wrote the wrapper,
  * now it only constructs the string object. We still leave it in here so that we may utilize
- * it in the future (it is a nice abstraction).
+ * it in the future (it is a nice abstraction). iExpcectedObjSize is an optimization setting.
+ * It must contain the size (in characters) that the calling object expects the string 
+ * representation to grow to. Specifying a bit too large size doesn't hurt. A too-small size
+ * does not cause any malfunction, but results in more often memory copies than necessary. So
+ * the caller is advised to be conservative in guessing. Binary multiples are recommended.
  * rgerhards, 2008-01-06
  */
-rsRetVal objBeginSerialize(rsCStrObj **ppCStr, obj_t *pObj)
+rsRetVal objBeginSerialize(rsCStrObj **ppCStr, obj_t *pObj, size_t iExpectedObjSize)
 {
 	DEFiRet;
 
@@ -115,6 +119,8 @@ rsRetVal objBeginSerialize(rsCStrObj **ppCStr, obj_t *pObj)
 	if((*ppCStr = rsCStrConstruct()) == NULL)
 		ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY);
 
+	rsCStrSetAllocIncrement(*ppCStr, iExpectedObjSize);
+
 finalize_it:
 	return iRet;
 }
@@ -205,7 +211,7 @@ finalize_it:
 }
 
 
-static rsRetVal objSerializeHeader(rsCStrObj **ppCStr, obj_t *pObj, rsCStrObj *pCSObjString)
+static rsRetVal objSerializeHeader(rsCStrObj **ppCStr, obj_t *pObj, rsCStrObj *pCSObjString, size_t iAllocIncrement)
 {
 	DEFiRet;
 	rsCStrObj *pCStr;
@@ -215,6 +221,7 @@ static rsRetVal objSerializeHeader(rsCStrObj **ppCStr, obj_t *pObj, rsCStrObj *p
 
 	if((pCStr = rsCStrConstruct()) == NULL)
 		ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY);
+	rsCStrSetAllocIncrement(pCStr, iAllocIncrement);
 
 	/* object cookie and serializer version (so far always 1) */
 	CHKiRet(rsCStrAppendStr(pCStr, (uchar*) "$Obj1"));
@@ -252,7 +259,7 @@ rsRetVal objEndSerialize(rsCStrObj **ppCStr, obj_t *pObj)
 	rsCStrObj *pCStr = NULL;
 
 	assert(ppCStr != NULL);
-	CHKiRet(objSerializeHeader(&pCStr, pObj, *ppCStr));
+	CHKiRet(objSerializeHeader(&pCStr, pObj, *ppCStr, rsCStrGetAllocIncrement(*ppCStr)));
 
 	CHKiRet(rsCStrAppendStrWithLen(pCStr, rsCStrGetBufBeg(*ppCStr), rsCStrLen(*ppCStr)));
 	CHKiRet(rsCStrAppendStr(pCStr, (uchar*) ".\n"));
diff --git a/obj.h b/obj.h
index 09919e2d..6c5fbd12 100644
--- a/obj.h
+++ b/obj.h
@@ -100,7 +100,7 @@ finalize_it: \
 /* prototypes */
 rsRetVal objInfoConstruct(objInfo_t **ppThis, objID_t objID, uchar *pszName, int iObjVers, rsRetVal (*pDestruct)(void *));
 rsRetVal objInfoSetMethod(objInfo_t *pThis, objMethod_t objMethod, rsRetVal (*pHandler)(void*));
-rsRetVal objBeginSerialize(rsCStrObj **ppCStr, obj_t *pObj);
+rsRetVal objBeginSerialize(rsCStrObj **ppCStr, obj_t *pObj, size_t iExpectedObjSize);
 rsRetVal objSerializePsz(rsCStrObj *pCStr, uchar *psz, size_t len);
 rsRetVal objEndSerialize(rsCStrObj **ppCStr, obj_t *pObj);
 rsRetVal objSerializeProp(rsCStrObj *pCStr, uchar *pszPropName, propertyType_t propType, void *pUsr);
diff --git a/stringbuf.c b/stringbuf.c
index 525a65b0..040ede77 100755
--- a/stringbuf.c
+++ b/stringbuf.c
@@ -141,36 +141,73 @@ void rsCStrDestruct(rsCStrObj *pThis)
 }
 
 
+/* extend the string buffer if its size is insufficient.
+ * Param iMinNeeded is the minumum free space needed. If it is larger
+ * than the default alloc increment, space for at least this amount is
+ * allocated. In practice, a bit more is allocated because we envision that
+ * some more characters may be added after these.
+ * rgerhards, 2008-01-07
+ */
+static rsRetVal rsCStrExtendBuf(rsCStrObj *pThis, size_t iMinNeeded)
+{
+	DEFiRet;
+	uchar *pNewBuf;
+	size_t iNewSize;
+
+	/* first compute the new size needed */
+	if(iMinNeeded > pThis->iAllocIncrement) {
+		/* we allocate "n" iAllocIncrements. Usually, that should
+		 * leave some room after the absolutely needed one. It also
+		 * reduces memory fragmentation. Note that all of this are
+		 * integer operations (very important to understand what is
+		 * going on)! Parenthesis are for better readibility.
+		 */
+		iNewSize = ((iMinNeeded / pThis->iAllocIncrement) + 1) * pThis->iAllocIncrement;
+	} else {
+		iNewSize = pThis->iBufSize + pThis->iAllocIncrement;
+	}
+	iNewSize += pThis->iBufSize; /* add current size */
+
+dbgprintf("extending string buffer, old %d, new %d\n", pThis->iBufSize, iNewSize);
+	/* and then allocate and copy over */
+	/* DEV debugging only: dbgprintf("extending string buffer, old %d, new %d\n", pThis->iBufSize, iNewSize); */
+	if((pNewBuf = (uchar*) malloc(iNewSize * sizeof(uchar))) == NULL)
+		ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY);
+	memcpy(pNewBuf, pThis->pBuf, pThis->iBufSize);
+	pThis->iBufSize = iNewSize;
+	if(pThis->pBuf != NULL) {
+		free(pThis->pBuf);
+	}
+	pThis->pBuf = pNewBuf;
+
+finalize_it:
+	return iRet;
+}
+
+
+/* append a string of known length. In this case, we make sure we do at most
+ * one additional memory allocation.
+ * I optimized this function to use memcpy(), among others. Consider it a
+ * rewrite (which may be good to know in case of bugs) -- rgerhards, 2008-01-07
+ */
 rsRetVal rsCStrAppendStrWithLen(rsCStrObj *pThis, uchar* psz, size_t iStrLen)
 {
 	rsRetVal iRet;
-	int iOldAllocInc;
 
 	rsCHECKVALIDOBJECT(pThis, OIDrsCStr);
 	assert(psz != NULL);
 
-	/* we first check if the to-be-added string is larger than the
-	 * alloc increment. If so, we temporarily increase the alloc
-	 * increment to the length of the string. This will ensure that
-	 * one string copy will be needed at most. As this is a very
-	 * costly operation, it outweights the cost of the strlen((char*)) and
-	 * related stuff - at least I think so.
-	 * rgerhards 2005-09-22
-	 */
-	/* We save the current alloc increment in any case, so we can just
-	 * overwrite it below, this is faster than any if-construct.
-	 */
-	iOldAllocInc = pThis->iAllocIncrement;
-	if(iStrLen > pThis->iAllocIncrement) {
-		pThis->iAllocIncrement = iStrLen;
+	/* does the string fit? */
+	if(pThis->iStrLen + iStrLen > pThis->iBufSize) {  
+		CHKiRet(rsCStrExtendBuf(pThis, iStrLen)); /* need more memory! */
 	}
 
-	while(*psz)
-		if((iRet = rsCStrAppendChar(pThis, *psz++)) != RS_RET_OK)
-			return iRet;
+	/* ok, now we always have sufficient continues memory to do a memcpy() */
+	memcpy(pThis->pBuf + pThis->iStrLen, psz, iStrLen);
+	pThis->iStrLen += iStrLen;
 
-	pThis->iAllocIncrement = iOldAllocInc; /* restore */
-	return RS_RET_OK;
+finalize_it:
+	return iRet;
 }
 
 
@@ -181,7 +218,7 @@ rsRetVal rsCStrAppendStrWithLen(rsCStrObj *pThis, uchar* psz, size_t iStrLen)
  */
 rsRetVal rsCStrAppendStr(rsCStrObj *pThis, uchar* psz)
 {
-	return rsCStrAppendStrWithLen(pThis, psz, strlen((char*)(char*) psz));
+	return rsCStrAppendStrWithLen(pThis, psz, strlen((char*) psz));
 }
 
 
@@ -201,20 +238,12 @@ rsRetVal rsCStrAppendInt(rsCStrObj *pThis, long i)
 
 rsRetVal rsCStrAppendChar(rsCStrObj *pThis, uchar c)
 {
-	uchar* pNewBuf;
+	DEFiRet;
 
 	rsCHECKVALIDOBJECT(pThis, OIDrsCStr);
 
-	if(pThis->iStrLen >= pThis->iBufSize)
-	{  /* need more memory! */
-		if((pNewBuf = (uchar*) malloc((pThis->iBufSize + pThis->iAllocIncrement) * sizeof(uchar))) == NULL)
-			return RS_RET_OUT_OF_MEMORY;
-		memcpy(pNewBuf, pThis->pBuf, pThis->iBufSize);
-		pThis->iBufSize += pThis->iAllocIncrement;
-		if(pThis->pBuf != NULL) {
-			free(pThis->pBuf);
-		}
-		pThis->pBuf = pNewBuf;
+	if(pThis->iStrLen >= pThis->iBufSize) {  
+		CHKiRet(rsCStrExtendBuf(pThis, 1)); /* need more memory! */
 	}
 
 	/* ok, when we reach this, we have sufficient memory */
@@ -226,7 +255,8 @@ rsRetVal rsCStrAppendChar(rsCStrObj *pThis, uchar c)
 		pThis->pszBuf = NULL;
 	}
 
-	return RS_RET_OK;
+finalize_it:
+	return iRet;
 }
 
 
diff --git a/stringbuf.h b/stringbuf.h
index b345dd47..59f6e2fc 100755
--- a/stringbuf.h
+++ b/stringbuf.h
@@ -118,6 +118,7 @@ rsRetVal rsCStrAppendStrWithLen(rsCStrObj *pThis, uchar* psz, size_t iStrLen);
  *       if you are very well aware why you are doing it ;)

  */

 void rsCStrSetAllocIncrement(rsCStrObj *pThis, int iNewIncrement);

+#define rsCStrGetAllocIncrement(pThis) ((pThis)->iAllocIncrement)

 

 /**

  * Append an integer to the string. No special formatting is