From 7bfa03bdc0b73647ffdbe4b73e5c1649af665fbf Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 1 Jul 2009 14:33:19 +0200 Subject: now put the new property-based methods to good use ... hopefully reducing the number of allocs/frees as well as overall memory usage in a busy system (plus that these shared properties hopefully remain in cache longer than its single-instance counterparts...) --- plugins/imklog/imklog.c | 10 ++++--- plugins/imudp/imudp.c | 12 ++++++-- runtime/msg.c | 58 +++++++++++++++++++++---------------- runtime/msg.h | 4 +-- runtime/prop.c | 76 ++++++++++++++++++++++++++++++++++++++++--------- runtime/prop.h | 2 ++ tcps_sess.c | 1 + tools/syslogd.c | 6 ++-- 8 files changed, 120 insertions(+), 49 deletions(-) diff --git a/plugins/imklog/imklog.c b/plugins/imklog/imklog.c index 269cfee0..2a832bee 100644 --- a/plugins/imklog/imklog.c +++ b/plugins/imklog/imklog.c @@ -84,6 +84,7 @@ int console_log_level = -1; static prop_t *pInputName = NULL; /* there is only one global inputName for all messages generated by this module */ +static prop_t *pLocalHostIP = NULL; /* a pseudo-constant propterty for 127.0.0.1 */ /* enqueue the the kernel message into the message queue. * The provided msg string is not freed - thus must be done @@ -105,7 +106,7 @@ enqMsg(uchar *msg, uchar* pszTag, int iFacility, int iSeverity) MsgSetRawMsgWOSize(pMsg, (char*)msg); MsgSetMSGoffs(pMsg, 0); /* we do not have a header... */ MsgSetRcvFrom(pMsg, glbl.GetLocalHostNameProp()); - MsgSetRcvFromIPStr(pMsg, (uchar*)"127.0.0.1", sizeof("127.0.0.1") - 1); + MsgSetRcvFromIP(pMsg, pLocalHostIP); MsgSetHOSTNAME(pMsg, glbl.GetLocalHostName(), ustrlen(glbl.GetLocalHostName())); MsgSetTAG(pMsg, pszTag, ustrlen(pszTag)); pMsg->iFacility = LOG_FAC(iFacility); @@ -235,9 +236,8 @@ ENDrunInput BEGINwillRun CODESTARTwillRun /* we need to create the inputName property (only once during our lifetime) */ - CHKiRet(prop.Construct(&pInputName)); - CHKiRet(prop.SetString(pInputName, UCHAR_CONSTANT("imklog"), sizeof("imklog") - 1)); - CHKiRet(prop.ConstructFinalize(pInputName)); + CHKiRet(prop.CreateStringProp(&pInputName, UCHAR_CONSTANT("imklog"), sizeof("imklog") - 1)); + CHKiRet(prop.CreateStringProp(&pLocalHostIP, UCHAR_CONSTANT("127.0.0.1"), sizeof("127.0.0.1") - 1)); iRet = klogWillRun(); finalize_it: @@ -250,6 +250,8 @@ CODESTARTafterRun if(pInputName != NULL) prop.Destruct(&pInputName); + if(pLocalHostIP != NULL) + prop.Destruct(&pLocalHostIP); ENDafterRun diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 31a4869f..718c3090 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -191,6 +191,8 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, ssize_t lenRcvBuf; struct sockaddr_storage frominet; msg_t *pMsg; + prop_t *propFromHost = NULL; + prop_t *propFromHostIP = NULL; char errStr[1024]; iNbrTimeUsed = 0; @@ -249,14 +251,18 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, MsgSetFlowControlType(pMsg, eFLOWCTL_NO_DELAY); pMsg->msgFlags = NEEDS_PARSING | PARSE_HOSTNAME; pMsg->bParseHOSTNAME = 1; - MsgSetRcvFromStr(pMsg, fromHost, ustrlen(fromHost)); - CHKiRet(MsgSetRcvFromIPStr(pMsg, fromHostIP, ustrlen(fromHostIP))); + MsgSetRcvFromStr(pMsg, fromHost, ustrlen(fromHost), &propFromHost); + CHKiRet(MsgSetRcvFromIPStr(pMsg, fromHostIP, ustrlen(fromHostIP), &propFromHostIP)); CHKiRet(submitMsg(pMsg)); } } - finalize_it: + if(propFromHost != NULL) + prop.Destruct(&propFromHost); + if(propFromHostIP != NULL) + prop.Destruct(&propFromHostIP); + RETiRet; } diff --git a/runtime/msg.c b/runtime/msg.c index d3645664..6c272d1f 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -1838,21 +1838,22 @@ void MsgSetRcvFrom(msg_t *pThis, prop_t *new) pThis->pRcvFrom = new; } -/* to be removed soon: work-around for those tht can not natively generate a - * property. - * rgerhards, 2009-06-29 + +/* This is used to set the property via a string. This function should not be + * called if there is a reliable way for a caller to make sure that the + * same name can be used across multiple messages. However, if it can not + * ensure that, calling this function is the second best thing, because it + * will re-use the previously created property if it contained the same + * name (but it works only for the immediate previous). + * rgerhards, 2009-06-31 */ -void MsgSetRcvFromStr(msg_t *pThis, uchar *psz, int len) +void MsgSetRcvFromStr(msg_t *pThis, uchar *psz, int len, prop_t **ppProp) { - prop_t *pProp; assert(pThis != NULL); + assert(ppProp != NULL); - /* we need to create a property */ - prop.Construct(&pProp); - prop.SetString(pProp, psz, len); - prop.ConstructFinalize(pProp); - MsgSetRcvFrom(pThis, pProp); - prop.Destruct(&pProp); + prop.CreateOrReuseStringProp(ppProp, psz, len); + MsgSetRcvFrom(pThis, *ppProp); } @@ -1875,24 +1876,27 @@ rsRetVal MsgSetRcvFromIP(msg_t *pThis, prop_t *new) } -/* to be removed soon: work-around for those tht can not natively generate a - * property. - * rgerhards, 2009-06-29 +/* This is used to set the property via a string. This function should not be + * called if there is a reliable way for a caller to make sure that the + * same name can be used across multiple messages. However, if it can not + * ensure that, calling this function is the second best thing, because it + * will re-use the previously created property if it contained the same + * name (but it works only for the immediate previous). + * rgerhards, 2009-06-31 */ -rsRetVal MsgSetRcvFromIPStr(msg_t *pThis, uchar *psz, int len) +rsRetVal MsgSetRcvFromIPStr(msg_t *pThis, uchar *psz, int len, prop_t **ppProp) { - prop_t *pProp; + DEFiRet; assert(pThis != NULL); - /* we need to create a property */ - prop.Construct(&pProp); - prop.SetString(pProp, psz, len); - prop.ConstructFinalize(pProp); - MsgSetRcvFromIP(pThis, pProp); - prop.Destruct(&pProp); - return RS_RET_OK; + CHKiRet(prop.CreateOrReuseStringProp(ppProp, psz, len)); + MsgSetRcvFrom(pThis, *ppProp); + +finalize_it: + RETiRet; } + /* rgerhards 2004-11-09: set HOSTNAME in msg object * rgerhards, 2007-06-21: * Does not return anything. If an error occurs, the hostname is @@ -2901,6 +2905,8 @@ finalize_it: rsRetVal MsgSetProperty(msg_t *pThis, var_t *pProp) { prop_t *myProp; + prop_t *propRcvFrom = NULL; + prop_t *propRcvFromIP = NULL; DEFiRet; ISOBJ_TYPE_assert(pThis, msg); @@ -2935,9 +2941,11 @@ rsRetVal MsgSetProperty(msg_t *pThis, var_t *pProp) MsgSetInputName(pThis, myProp); prop.Destruct(&myProp); } else if(isProp("pszRcvFromIP")) { - MsgSetRcvFromIPStr(pThis, rsCStrGetSzStrNoNULL(pProp->val.pStr), rsCStrLen(pProp->val.pStr)); + MsgSetRcvFromIPStr(pThis, rsCStrGetSzStrNoNULL(pProp->val.pStr), rsCStrLen(pProp->val.pStr), &propRcvFromIP); + prop.Destruct(&propRcvFromIP); } else if(isProp("pszRcvFrom")) { - MsgSetRcvFromStr(pThis, rsCStrGetSzStrNoNULL(pProp->val.pStr), rsCStrLen(pProp->val.pStr)); + MsgSetRcvFromStr(pThis, rsCStrGetSzStrNoNULL(pProp->val.pStr), rsCStrLen(pProp->val.pStr), &propRcvFrom); + prop.Destruct(&propRcvFrom); } else if(isProp("pszHOSTNAME")) { MsgSetHOSTNAME(pThis, rsCStrGetSzStrNoNULL(pProp->val.pStr), rsCStrLen(pProp->val.pStr)); } else if(isProp("pCSStrucData")) { diff --git a/runtime/msg.h b/runtime/msg.h index 43f24435..c20fb005 100644 --- a/runtime/msg.h +++ b/runtime/msg.h @@ -150,9 +150,9 @@ void MsgSetRuleset(msg_t *pMsg, ruleset_t*); rsRetVal MsgSetFlowControlType(msg_t *pMsg, flowControl_t eFlowCtl); rsRetVal MsgSetStructuredData(msg_t *pMsg, char* pszStrucData); void MsgSetRcvFrom(msg_t *pMsg, prop_t*); -void MsgSetRcvFromStr(msg_t *pMsg, uchar* pszRcvFrom, int); +void MsgSetRcvFromStr(msg_t *pMsg, uchar* pszRcvFrom, int, prop_t **); rsRetVal MsgSetRcvFromIP(msg_t *pMsg, prop_t*); -rsRetVal MsgSetRcvFromIPStr(msg_t *pMsg, uchar*, int); +rsRetVal MsgSetRcvFromIPStr(msg_t *pThis, uchar *psz, int len, prop_t **ppProp); void MsgSetHOSTNAME(msg_t *pMsg, uchar* pszHOSTNAME, int lenHOSTNAME); rsRetVal MsgSetAfterPRIOffs(msg_t *pMsg, short offs); void MsgSetMSGoffs(msg_t *pMsg, short offs); diff --git a/runtime/prop.c b/runtime/prop.c index 96ebe212..804f3491 100644 --- a/runtime/prop.c +++ b/runtime/prop.c @@ -41,6 +41,7 @@ #include "rsyslog.h" #include "obj.h" #include "obj-types.h" +#include "unicode-helper.h" #include "atomic.h" #include "prop.h" @@ -54,6 +55,21 @@ BEGINobjConstruct(prop) /* be sure to specify the object type also in END macro! pThis->iRefCount = 1; ENDobjConstruct(prop) + +/* destructor for the prop object */ +BEGINobjDestruct(prop) /* be sure to specify the object type also in END and CODESTART macros! */ + int currRefCount; +CODESTARTobjDestruct(prop) + currRefCount = ATOMIC_DEC_AND_FETCH(pThis->iRefCount); + if(currRefCount == 0) { + /* (only) in this case we need to actually destruct the object */ + if(pThis->len >= CONF_PROP_BUFSIZE) + free(pThis->szVal.psz); + } else { + pThis = NULL; /* tell framework NOT to destructing the object! */ + } +ENDobjDestruct(prop) + /* set string, we make our own private copy! This MUST only be called BEFORE * ConstructFinalize()! */ @@ -90,10 +106,8 @@ static rsRetVal GetString(prop_t *pThis, uchar **ppsz, int *plen) ISOBJ_TYPE_assert(pThis, prop); if(pThis->len < CONF_PROP_BUFSIZE) { *ppsz = pThis->szVal.sz; -RUNLOG; } else { *ppsz = pThis->szVal.psz; -RUNLOG; } *plen = pThis->len; ENDfunc @@ -123,19 +137,53 @@ static rsRetVal AddRef(prop_t *pThis) } -/* destructor for the prop object */ -BEGINobjDestruct(prop) /* be sure to specify the object type also in END and CODESTART macros! */ - int currRefCount; -CODESTARTobjDestruct(prop) - currRefCount = ATOMIC_DEC_AND_FETCH(pThis->iRefCount); - if(currRefCount == 0) { - /* (only) in this case we need to actually destruct the object */ - if(pThis->len >= CONF_PROP_BUFSIZE) - free(pThis->szVal.psz); +/* this is a "do it all in one shot" function that creates a new property, + * assigns the provided string to it and finalizes the property. Among the + * convenience, it is alos (very, very) slightly faster. + * rgerhards, 2009-07-01 + */ +static rsRetVal CreateStringProp(prop_t **ppThis, uchar* psz, int len) +{ + DEFiRet; + propConstruct(ppThis); + SetString(*ppThis, psz, len); + propConstructFinalize(*ppThis); + RETiRet; +} + +/* another one-stop function, quite useful: it takes a property pointer and + * a string. If the string is already contained in the property, nothing happens. + * If the string is different (or the pointer NULL), the current property + * is destructed and a new one created. This can be used to get a specific + * name in those cases where there is a good chance that the property + * immediatly previously processed already contained the value we need - in + * which case we save us all the creation overhead by just reusing the already + * existing property). + * rgerhards, 2009-07-01 + */ +rsRetVal CreateOrReuseStringProp(prop_t **ppThis, uchar *psz, int len) +{ + uchar *pszPrev; + int lenPrev; + DEFiRet; + assert(ppThis != NULL); + + if(*ppThis == NULL) { + /* we need to create a property */ + CHKiRet(CreateStringProp(ppThis, psz, len)); } else { - pThis = NULL; /* tell framework NOT to destructing the object! */ + /* already exists, check if we can re-use it */ + GetString(*ppThis, &pszPrev, &lenPrev); + if(len != lenPrev && ustrcmp(psz, pszPrev)) { + /* different, need to discard old & create new one */ + propDestruct(ppThis); + CHKiRet(CreateStringProp(ppThis, psz, len)); + } /* else we can re-use the existing one! */ } -ENDobjDestruct(prop) + +finalize_it: + RETiRet; +} /* debugprint for the prop object */ @@ -167,6 +215,8 @@ CODESTARTobjQueryInterface(prop) pIf->GetString = GetString; pIf->GetStringLen = GetStringLen; pIf->AddRef = AddRef; + pIf->CreateStringProp = CreateStringProp; + pIf->CreateOrReuseStringProp = CreateOrReuseStringProp; finalize_it: ENDobjQueryInterface(prop) diff --git a/runtime/prop.h b/runtime/prop.h index 62c0d799..e3519664 100644 --- a/runtime/prop.h +++ b/runtime/prop.h @@ -46,6 +46,8 @@ BEGINinterface(prop) /* name must also be changed in ENDinterface macro! */ rsRetVal (*GetString)(prop_t *pThis, uchar** ppsz, int *plen); int (*GetStringLen)(prop_t *pThis); rsRetVal (*AddRef)(prop_t *pThis); + rsRetVal (*CreateStringProp)(prop_t **ppThis, uchar* psz, int len); + rsRetVal (*CreateOrReuseStringProp)(prop_t **ppThis, uchar *psz, int len); ENDinterface(prop) #define propCURR_IF_VERSION 1 /* increment whenever you change the interface structure! */ diff --git a/tcps_sess.c b/tcps_sess.c index f701fe57..8d307380 100644 --- a/tcps_sess.c +++ b/tcps_sess.c @@ -156,6 +156,7 @@ SetHostIP(tcps_sess_t *pThis, uchar *pszHostIP) CHKiRet(prop.SetString(pThis->fromHostIP, pszHostIP, ustrlen(pszHostIP))); finalize_it: + free(pszHostIP); RETiRet; } diff --git a/tools/syslogd.c b/tools/syslogd.c index be5b5aad..f7253a8e 100644 --- a/tools/syslogd.c +++ b/tools/syslogd.c @@ -559,6 +559,8 @@ static inline rsRetVal printline(uchar *hname, uchar *hnameIP, uchar *msg, int f register uchar *p; int pri; msg_t *pMsg; + prop_t *propFromHost = NULL; + prop_t *propFromHostIP = NULL; /* Now it is time to create the message object (rgerhards) */ if(stTime == NULL) { @@ -597,9 +599,9 @@ static inline rsRetVal printline(uchar *hname, uchar *hnameIP, uchar *msg, int f */ if((pMsg->msgFlags & PARSE_HOSTNAME) == 0) MsgSetHOSTNAME(pMsg, hname, ustrlen(hname)); - MsgSetRcvFromStr(pMsg, hname, ustrlen(hname)); + MsgSetRcvFromStr(pMsg, hname, ustrlen(hname), &propFromHost); + CHKiRet(MsgSetRcvFromIPStr(pMsg, hnameIP, ustrlen(hname), &propFromHostIP)); MsgSetAfterPRIOffs(pMsg, p - msg); - CHKiRet(MsgSetRcvFromIPStr(pMsg, hnameIP, ustrlen(hname))); logmsg(pMsg, flags); -- cgit