From c8b946aa94251a93fc934d8e3e875f5f26fcfcfd Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 7 Jan 2008 08:38:14 +0000 Subject: performance-tuned stringbuf class --- ChangeLog | 1 + msg.c | 2 +- obj.c | 15 +++++++--- obj.h | 2 +- stringbuf.c | 96 ++++++++++++++++++++++++++++++++++++++++--------------------- stringbuf.h | 1 + 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 -- cgit