From bb79e96dc300fa5a2182e7c047afb3b15c5dc870 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 12 May 2009 15:27:40 +0200 Subject: moving to a cleaner implementation of batches ... now that we know what we need from a theoretical POV. --- runtime/batch.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 runtime/batch.h (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h new file mode 100644 index 00000000..cb40cf42 --- /dev/null +++ b/runtime/batch.h @@ -0,0 +1,63 @@ +/* Definition of the batch_t data structure. + * I am not sure yet if this will become a full-blown object. For now, this header just + * includes the object definition and is not accompanied by code. + * + * Copyright 2009 by Rainer Gerhards and Adiscon GmbH. + * + * This file is part of the rsyslog runtime library. + * + * The rsyslog runtime library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * The rsyslog runtime library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with the rsyslog runtime library. If not, see . + * + * A copy of the GPL can be found in the file "COPYING" in this distribution. + * A copy of the LGPL can be found in the file "COPYING.LESSER" in this distribution. + */ + +#ifndef BATCH_H_INCLUDED +#define BATCH_H_INCLUDED + +/* enum for batch states. Actually, we violate a layer here, in that we assume that a batch is used + * for action processing. So far, this seems acceptable, the status is simply ignored inside the + * main message queue. But over time, it could potentially be useful to split the two. + * rgerhad, 2009-05-12 + */ +typedef enum { + BATCH_STATE_RDY = 0, /* object ready for processing */ + BATCH_STATE_BAD = 1, /* unrecoverable failure while processing, do NOT resubmit to same action */ + BATCH_STATE_SUB = 2, /* message submitted for processing, outcome yet unkonwn */ + BATCH_STATE_DISC = 3, /* discarded - processed OK, but do not submit to any other action */ +} batch_state_t; + + +/* an object inside a batch, including any information (state!) needed for it to "life". + */ +struct batch_obj_s { + obj_t *pUsrp; /* pointer to user object (most often message) */ + batch_state_t state; /* associated state */ +}; + +/* the batch + * This object is used to dequeue multiple user pointers which are than handed over + * to processing. The size of elements is fixed after queue creation, but may be + * modified by config variables (better said: queue properties). + * Note that a "user pointer" in rsyslog context so far always is a message + * object. We stick to the more generic term because queues may potentially hold + * other types of objects, too. + * rgerhards, 2009-05-12 + */ +struct batch_s { + int nElem; /* actual number of element in this entry */ + batch_obj_t *pElem; /* batch elements */ +}; + +#endif /* #ifndef BATCH_H_INCLUDED */ -- cgit v1.2.3 From e2b229868955a6f6a6380273314d0d90ddad1273 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 12 May 2009 17:57:04 +0200 Subject: action batch processing implemented ... passed initial tests, but of course more are needed --- runtime/batch.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index cb40cf42..fcbbafce 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -35,7 +35,8 @@ typedef enum { BATCH_STATE_RDY = 0, /* object ready for processing */ BATCH_STATE_BAD = 1, /* unrecoverable failure while processing, do NOT resubmit to same action */ BATCH_STATE_SUB = 2, /* message submitted for processing, outcome yet unkonwn */ - BATCH_STATE_DISC = 3, /* discarded - processed OK, but do not submit to any other action */ + BATCH_STATE_COMM = 3, /* message successfully commited */ + BATCH_STATE_DISC = 4, /* discarded - processed OK, but do not submit to any other action */ } batch_state_t; @@ -57,6 +58,7 @@ struct batch_obj_s { */ struct batch_s { int nElem; /* actual number of element in this entry */ + int iDoneUpTo; /* all messages below this index have state other than RDY */ batch_obj_t *pElem; /* batch elements */ }; -- cgit v1.2.3 From 93f873277bfe5ebb309ff5e92f5dc7244ebd9f1a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 18 May 2009 17:28:34 +0200 Subject: t-delete list implemented, queue store drivers updated... ... on the way to the ultra-reliable queue modes (redesign doc). This version does not really work, but is a good commit point. Next comes queue size calculation. DA mode does not yet work. --- runtime/batch.h | 1 + 1 file changed, 1 insertion(+) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index fcbbafce..eb266b3f 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -59,6 +59,7 @@ struct batch_obj_s { struct batch_s { int nElem; /* actual number of element in this entry */ int iDoneUpTo; /* all messages below this index have state other than RDY */ + qDeqID deqID; /* ID of dequeue operation that generated this batch */ batch_obj_t *pElem; /* batch elements */ }; -- cgit v1.2.3 From 0cf8e88a348dc574244e4f5c2be26f47e8bfff08 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 19 May 2009 18:58:33 +0200 Subject: solved the intended-discard-during-dequeue issue --- runtime/batch.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index eb266b3f..031718a7 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -55,9 +55,15 @@ struct batch_obj_s { * object. We stick to the more generic term because queues may potentially hold * other types of objects, too. * rgerhards, 2009-05-12 + * Note that nElem is not necessarily equal to nElemDeq. This is the case when we + * discard some elements (because of configuration) during dequeue processing. As + * all Elements are only deleted when the batch is processed, we can not immediately + * delete them. So we need to keep their number that we can delete them when the batch + * is completed (else, the whole process does not work correctly). */ struct batch_s { int nElem; /* actual number of element in this entry */ + int nElemDeq; /* actual number of elements dequeued (and thus to be deleted) - see comment above! */ int iDoneUpTo; /* all messages below this index have state other than RDY */ qDeqID deqID; /* ID of dequeue operation that generated this batch */ batch_obj_t *pElem; /* batch elements */ -- cgit v1.2.3 From da53802c96a59a990859706219398dce709ba1b3 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 26 Oct 2009 12:21:07 +0100 Subject: implemented solution for cancel at shutdown/unprocessed entries We do now enqueue those objects that are left unprocessed. This enables us to delete the full batch, what is exactly what we need to do. --- runtime/batch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 031718a7..2b3aa83e 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -34,7 +34,7 @@ typedef enum { BATCH_STATE_RDY = 0, /* object ready for processing */ BATCH_STATE_BAD = 1, /* unrecoverable failure while processing, do NOT resubmit to same action */ - BATCH_STATE_SUB = 2, /* message submitted for processing, outcome yet unkonwn */ + BATCH_STATE_SUB = 2, /* message submitted for processing, outcome yet unknown */ BATCH_STATE_COMM = 3, /* message successfully commited */ BATCH_STATE_DISC = 4, /* discarded - processed OK, but do not submit to any other action */ } batch_state_t; -- cgit v1.2.3 From 74135da95971c8d03e4e45f7d3f703e1d40f76f4 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 10 Jun 2010 12:24:22 +0200 Subject: some cleanup as well as some work in preparation of storing doAction params inside the batch --- runtime/batch.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 2b3aa83e..ec257125 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -45,6 +45,12 @@ typedef enum { struct batch_obj_s { obj_t *pUsrp; /* pointer to user object (most often message) */ batch_state_t state; /* associated state */ + void *pActParams; /* parameters to be passed to action */ + size_t *pLenParams; /* length of the parameter in question */ + void *staticActParams[CONF_OMOD_NUMSTRINGS_BUFSIZE]; + /* a cache to save malloc(), if not absolutely necessary */ + size_t staticLenParams[CONF_OMOD_NUMSTRINGS_BUFSIZE]; + /* and the same for the message length (if used) */ }; /* the batch -- cgit v1.2.3 From fe8d317c1b40fe162891d5ddec1cb7df702bb7fe Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 10 Jun 2010 14:36:49 +0200 Subject: milestone commit(BUGGY): batch is now handed down to rule processing Now, the full batch is passed down to the rule, which then enqueues the elements as single messages. Note that this code has some known defects and needs more changes until it is correct again. This is primarily a commit to be able to return to a known-(somewhat)-good state. --- runtime/batch.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index ec257125..1245df11 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -26,6 +26,8 @@ #ifndef BATCH_H_INCLUDED #define BATCH_H_INCLUDED +#include "msg.h" + /* enum for batch states. Actually, we violate a layer here, in that we assume that a batch is used * for action processing. So far, this seems acceptable, the status is simply ignored inside the * main message queue. But over time, it could potentially be useful to split the two. @@ -45,12 +47,18 @@ typedef enum { struct batch_obj_s { obj_t *pUsrp; /* pointer to user object (most often message) */ batch_state_t state; /* associated state */ + /* work variables for action processing; these are reused for each action (or block of + * actions) + */ + sbool bFilterOK; /* work area for filter processing (per action, reused!) */ + sbool bPrevWasSuspended; void *pActParams; /* parameters to be passed to action */ size_t *pLenParams; /* length of the parameter in question */ void *staticActParams[CONF_OMOD_NUMSTRINGS_BUFSIZE]; /* a cache to save malloc(), if not absolutely necessary */ size_t staticLenParams[CONF_OMOD_NUMSTRINGS_BUFSIZE]; /* and the same for the message length (if used) */ + /* end action work variables */ }; /* the batch @@ -72,7 +80,27 @@ struct batch_s { int nElemDeq; /* actual number of elements dequeued (and thus to be deleted) - see comment above! */ int iDoneUpTo; /* all messages below this index have state other than RDY */ qDeqID deqID; /* ID of dequeue operation that generated this batch */ + int *pbShutdownImmediate;/* end processing of this batch immediately if set to 1 */ + sbool bSingleRuleset; /* do all msgs of this batch use a single ruleset? */ batch_obj_t *pElem; /* batch elements */ }; + +/* some inline functions (we may move this off to an object .. or not) */ +static inline void +batchSetSingleRuleset(batch_t *pBatch, sbool val) { + pBatch->bSingleRuleset = val; +} + +/* get the batches ruleset */ +static inline ruleset_t* +batchGetRuleset(batch_t *pBatch) { + return (pBatch->nElem > 0) ? ((msg_t*) pBatch->pElem[0].pUsrp)->pRuleset : NULL; +} + +/* get number of msgs for this batch */ +static inline int +batchNumMsgs(batch_t *pBatch) { + return pBatch->nElem; +} #endif /* #ifndef BATCH_H_INCLUDED */ -- cgit v1.2.3 From 802f6d8a8f39e5ba578e0183e4500bef8e3a198c Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 15 Jun 2010 14:02:34 +0200 Subject: milestone(BUGGY): batch now pushed down to action at least in important cases (not for non-direct action queues and some other minor things). This version is definitely buggy, but may be tried with success on a non-production system. I will continue to work on the correctness, but needed to commit now to get a baseline. --- runtime/batch.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 1245df11..80621631 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -26,6 +26,7 @@ #ifndef BATCH_H_INCLUDED #define BATCH_H_INCLUDED +#include #include "msg.h" /* enum for batch states. Actually, we violate a layer here, in that we assume that a batch is used @@ -76,6 +77,7 @@ struct batch_obj_s { * is completed (else, the whole process does not work correctly). */ struct batch_s { + int maxElem; /* maximum number of elements that this batch supports */ int nElem; /* actual number of element in this entry */ int nElemDeq; /* actual number of elements dequeued (and thus to be deleted) - see comment above! */ int iDoneUpTo; /* all messages below this index have state other than RDY */ @@ -92,15 +94,84 @@ batchSetSingleRuleset(batch_t *pBatch, sbool val) { pBatch->bSingleRuleset = val; } -/* get the batches ruleset */ +/* get the batches ruleset (if we have a single ruleset) */ static inline ruleset_t* batchGetRuleset(batch_t *pBatch) { return (pBatch->nElem > 0) ? ((msg_t*) pBatch->pElem[0].pUsrp)->pRuleset : NULL; } +/* get the ruleset of a specifc element of the batch (index not verified!) */ +static inline ruleset_t* +batchElemGetRuleset(batch_t *pBatch, int i) { + return ((msg_t*) pBatch->pElem[i].pUsrp)->pRuleset; +} + /* get number of msgs for this batch */ static inline int batchNumMsgs(batch_t *pBatch) { return pBatch->nElem; } + + +/* set the status of the i-th batch element. Note that once the status is + * DISC, it will never be reset. So this function can NOT be used to initialize + * the state table. -- rgerhards, 2010-06-10 + */ +static inline void +batchSetElemState(batch_t *pBatch, int i, batch_state_t newState) { + if(pBatch->pElem[i].state != BATCH_STATE_DISC) + pBatch->pElem[i].state = newState; +} + + +/* check if an element is a valid entry. We do NOT verify if the + * element index is valid. -- rgerhards, 2010-06-10 + */ +static inline int +batchIsValidElem(batch_t *pBatch, int i) { + return(pBatch->pElem[i].bFilterOK && pBatch->pElem[i].state != BATCH_STATE_DISC); +} + + +/* copy one batch element to another. + * This creates a complete duplicate in those cases where + * it is needed. Use duplication only when absolutely necessary! + * rgerhards, 2010-06-10 + */ +static inline void +batchCopyElem(batch_obj_t *pDest, batch_obj_t *pSrc) { + memcpy(pDest, pSrc, sizeof(batch_obj_t)); +} + + +/* free members of a batch "object". Note that we can not do the usual + * destruction as the object typically is allocated on the stack and so the + * object itself cannot be freed! -- rgerhards, 2010-06-15 + */ +static inline void +batchFree(batch_t *pBatch) { + int i; + int j; + for(i = 0 ; i < pBatch->maxElem ; ++i) { + for(j = 0 ; j < CONF_OMOD_NUMSTRINGS_BUFSIZE ; ++j) { + free(pBatch->pElem[i].staticActParams[j]); + } + } + free(pBatch->pElem); +} + + +/* initialiaze a batch "object". The record must already exist, + * we "just" initialize it. The max number of elements must be + * provided. -- rgerhards, 2010-06-15 + */ +static inline rsRetVal +batchInit(batch_t *pBatch, int maxElem) { + DEFiRet; + pBatch->maxElem = maxElem; + CHKmalloc(pBatch->pElem = calloc((size_t)maxElem, sizeof(batch_obj_t))); + // TODO: replace calloc by inidividual writes? +finalize_it: + RETiRet; +} #endif /* #ifndef BATCH_H_INCLUDED */ -- cgit v1.2.3 From 31fae7b93d7aa94b7b3fcbfdf101328230ea6302 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 24 Jun 2010 12:13:48 +0200 Subject: bugfix: "$ActionExecOnlyWhenPreviousIsSuspended on" was broken Note that, as it looks, the directive was already broken in previous v5 versions. So while I solved what looked like a (intentional) regression from the performance tuning, I actually solved a previous regression as well ;) I have also added new test cases to the testbench in order to capture such problems in the future. This version does now look pretty good in shape. --- runtime/batch.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 80621631..b555fc2a 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -174,4 +174,23 @@ batchInit(batch_t *pBatch, int maxElem) { finalize_it: RETiRet; } + + +/* primarily a helper for debug purposes, get human-readble name of state */ +static inline char * +batchState2String(batch_state_t state) { + switch(state) { + case BATCH_STATE_RDY: + return "BATCH_STATE_RDY"; + case BATCH_STATE_BAD: + return "BATCH_STATE_BAD"; + case BATCH_STATE_SUB: + return "BATCH_STATE_SUB"; + case BATCH_STATE_COMM: + return "BATCH_STATE_COMM"; + case BATCH_STATE_DISC: + return "BATCH_STATE_DISC"; + } + return "ERROR, batch state not known!"; +} #endif /* #ifndef BATCH_H_INCLUDED */ -- cgit v1.2.3 From 6e093aa32674235c76410a10d1cc7321d3bf33bc Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 24 Jun 2010 14:05:00 +0200 Subject: made hardcoded max string number persistent see commit for reasoning --- runtime/batch.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index b555fc2a..68f48d8b 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -53,11 +53,9 @@ struct batch_obj_s { */ sbool bFilterOK; /* work area for filter processing (per action, reused!) */ sbool bPrevWasSuspended; - void *pActParams; /* parameters to be passed to action */ - size_t *pLenParams; /* length of the parameter in question */ - void *staticActParams[CONF_OMOD_NUMSTRINGS_BUFSIZE]; + void *staticActParams[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /* a cache to save malloc(), if not absolutely necessary */ - size_t staticLenParams[CONF_OMOD_NUMSTRINGS_BUFSIZE]; + size_t staticLenParams[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /* and the same for the message length (if used) */ /* end action work variables */ }; @@ -153,7 +151,7 @@ batchFree(batch_t *pBatch) { int i; int j; for(i = 0 ; i < pBatch->maxElem ; ++i) { - for(j = 0 ; j < CONF_OMOD_NUMSTRINGS_BUFSIZE ; ++j) { + for(j = 0 ; j < CONF_OMOD_NUMSTRINGS_MAXSIZE ; ++j) { free(pBatch->pElem[i].staticActParams[j]); } } -- cgit v1.2.3 From 2181515805e65c37b9db5e0badef2a0a86164234 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 17 Dec 2010 10:43:55 +0100 Subject: bug fixes in action processing - bugfix: action processor released mememory too early, resulting in potential issue in retry cases (but very unlikely due to another bug, which I also fixed -- only after the fix this problem here became actually visible). - bugfix: batches which had actions in error were not properly retried in all cases --- runtime/batch.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 68f48d8b..d0504f2b 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -53,9 +53,11 @@ struct batch_obj_s { */ sbool bFilterOK; /* work area for filter processing (per action, reused!) */ sbool bPrevWasSuspended; - void *staticActParams[CONF_OMOD_NUMSTRINGS_MAXSIZE]; + /* following are caches to save allocs if not absolutely necessary */ + uchar *staticActStrings[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /**< for strings */ /* a cache to save malloc(), if not absolutely necessary */ - size_t staticLenParams[CONF_OMOD_NUMSTRINGS_MAXSIZE]; + void *staticActParams[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /**< for anything else */ + size_t staticLenStrings[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /* and the same for the message length (if used) */ /* end action work variables */ }; @@ -152,7 +154,10 @@ batchFree(batch_t *pBatch) { int j; for(i = 0 ; i < pBatch->maxElem ; ++i) { for(j = 0 ; j < CONF_OMOD_NUMSTRINGS_MAXSIZE ; ++j) { - free(pBatch->pElem[i].staticActParams[j]); + /* staticActParams MUST be freed immediately (if required), + * so we do not need to do that! + */ + free(pBatch->pElem[i].staticActStrings[j]); } } free(pBatch->pElem); -- cgit v1.2.3 From 1ef709cc97d54f74d3fdeb83788cc4b01f4c6a2a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 25 Feb 2011 14:14:17 +0100 Subject: bugfix: fixed a memory leak and potential abort condition this could happen if multiple rulesets were used and some output batches contained messages belonging to more than one ruleset. fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=226 fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=218 --- runtime/batch.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index d0504f2b..944889bd 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -136,11 +136,16 @@ batchIsValidElem(batch_t *pBatch, int i) { /* copy one batch element to another. * This creates a complete duplicate in those cases where * it is needed. Use duplication only when absolutely necessary! + * Note that all working fields are reset to zeros. If that were + * not done, we would have potential problems with invalid + * or double pointer frees. * rgerhards, 2010-06-10 */ static inline void batchCopyElem(batch_obj_t *pDest, batch_obj_t *pSrc) { - memcpy(pDest, pSrc, sizeof(batch_obj_t)); + memset(pDest, 0, sizeof(batch_obj_t)); + pDest->pUsrp = pSrc->pUsrp; + pDest->state = pSrc->state; } @@ -171,6 +176,7 @@ batchFree(batch_t *pBatch) { static inline rsRetVal batchInit(batch_t *pBatch, int maxElem) { DEFiRet; + pBatch->iDoneUpTo = 0; pBatch->maxElem = maxElem; CHKmalloc(pBatch->pElem = calloc((size_t)maxElem, sizeof(batch_obj_t))); // TODO: replace calloc by inidividual writes? -- cgit v1.2.3 From 99f342e866c6c4a257c0a504ce7e561e21458847 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 12 Sep 2012 11:10:48 +0200 Subject: new ruleengine: fixed action handling in regard to filters This was not yet adapted to the new "active" structure. --- runtime/batch.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 944889bd..fdacb8e2 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -51,7 +51,6 @@ struct batch_obj_s { /* work variables for action processing; these are reused for each action (or block of * actions) */ - sbool bFilterOK; /* work area for filter processing (per action, reused!) */ sbool bPrevWasSuspended; /* following are caches to save allocs if not absolutely necessary */ uchar *staticActStrings[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /**< for strings */ @@ -83,6 +82,7 @@ struct batch_s { int iDoneUpTo; /* all messages below this index have state other than RDY */ qDeqID deqID; /* ID of dequeue operation that generated this batch */ int *pbShutdownImmediate;/* end processing of this batch immediately if set to 1 */ + sbool *active; /* which messages are active for processing, NULL=all */ sbool bSingleRuleset; /* do all msgs of this batch use a single ruleset? */ batch_obj_t *pElem; /* batch elements */ }; @@ -129,7 +129,8 @@ batchSetElemState(batch_t *pBatch, int i, batch_state_t newState) { */ static inline int batchIsValidElem(batch_t *pBatch, int i) { - return(pBatch->pElem[i].bFilterOK && pBatch->pElem[i].state != BATCH_STATE_DISC); + return( (pBatch->active == NULL || pBatch->active[i]) + && pBatch->pElem[i].state != BATCH_STATE_DISC); } -- cgit v1.2.3 From c030585a630902ddd56b303ede1bfd8612036579 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 25 Sep 2012 15:35:08 +0200 Subject: fix invalid state handling during action execution could lead to execution of not-to-be-executed action. very recent regression. --- runtime/batch.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index fdacb8e2..f743c188 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -129,8 +129,8 @@ batchSetElemState(batch_t *pBatch, int i, batch_state_t newState) { */ static inline int batchIsValidElem(batch_t *pBatch, int i) { - return( (pBatch->active == NULL || pBatch->active[i]) - && pBatch->pElem[i].state != BATCH_STATE_DISC); + return( (pBatch->pElem[i].state != BATCH_STATE_DISC) + && (pBatch->active == NULL || pBatch->active[i])); } -- cgit v1.2.3 From c55e0a5a06e69106bc346057dd61dcb98688a4aa Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Sat, 3 Nov 2012 12:32:50 +0100 Subject: queue: change generic msg ptr (pUsr) to be of msg_t type --- runtime/batch.h | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index f743c188..0f19f5bb 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -46,7 +46,7 @@ typedef enum { /* an object inside a batch, including any information (state!) needed for it to "life". */ struct batch_obj_s { - obj_t *pUsrp; /* pointer to user object (most often message) */ + msg_t *pMsg; batch_state_t state; /* associated state */ /* work variables for action processing; these are reused for each action (or block of * actions) @@ -97,13 +97,13 @@ batchSetSingleRuleset(batch_t *pBatch, sbool val) { /* get the batches ruleset (if we have a single ruleset) */ static inline ruleset_t* batchGetRuleset(batch_t *pBatch) { - return (pBatch->nElem > 0) ? ((msg_t*) pBatch->pElem[0].pUsrp)->pRuleset : NULL; + return (pBatch->nElem > 0) ? pBatch->pElem[0].pMsg->pRuleset : NULL; } /* get the ruleset of a specifc element of the batch (index not verified!) */ static inline ruleset_t* batchElemGetRuleset(batch_t *pBatch, int i) { - return ((msg_t*) pBatch->pElem[i].pUsrp)->pRuleset; + return pBatch->pElem[i].pMsg->pRuleset; } /* get number of msgs for this batch */ @@ -134,22 +134,6 @@ batchIsValidElem(batch_t *pBatch, int i) { } -/* copy one batch element to another. - * This creates a complete duplicate in those cases where - * it is needed. Use duplication only when absolutely necessary! - * Note that all working fields are reset to zeros. If that were - * not done, we would have potential problems with invalid - * or double pointer frees. - * rgerhards, 2010-06-10 - */ -static inline void -batchCopyElem(batch_obj_t *pDest, batch_obj_t *pSrc) { - memset(pDest, 0, sizeof(batch_obj_t)); - pDest->pUsrp = pSrc->pUsrp; - pDest->state = pSrc->state; -} - - /* free members of a batch "object". Note that we can not do the usual * destruction as the object typically is allocated on the stack and so the * object itself cannot be freed! -- rgerhards, 2010-06-15 -- cgit v1.2.3 From 62b5ae05d6810bc00092076ccd0b52aab8096e60 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 15 Jan 2013 10:11:38 +0100 Subject: optimize: use fixed size (8 bits) instead of enum looks like GCC, even if optimizing, uses 32 bits - at least this is suggested by the profiler results (both in terms of runtime and cache misses). --- runtime/batch.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 0f19f5bb..097e104a 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -34,13 +34,12 @@ * main message queue. But over time, it could potentially be useful to split the two. * rgerhad, 2009-05-12 */ -typedef enum { - BATCH_STATE_RDY = 0, /* object ready for processing */ - BATCH_STATE_BAD = 1, /* unrecoverable failure while processing, do NOT resubmit to same action */ - BATCH_STATE_SUB = 2, /* message submitted for processing, outcome yet unknown */ - BATCH_STATE_COMM = 3, /* message successfully commited */ - BATCH_STATE_DISC = 4, /* discarded - processed OK, but do not submit to any other action */ -} batch_state_t; +#define BATCH_STATE_RDY 0 /* object ready for processing */ +#define BATCH_STATE_BAD 1 /* unrecoverable failure while processing, do NOT resubmit to same action */ +#define BATCH_STATE_SUB 2 /* message submitted for processing, outcome yet unknown */ +#define BATCH_STATE_COMM 3 /* message successfully commited */ +#define BATCH_STATE_DISC 4 /* discarded - processed OK, but do not submit to any other action */ +typedef unsigned char batch_state_t; /* an object inside a batch, including any information (state!) needed for it to "life". -- cgit v1.2.3 From 9273b4bb4dcb6683cf6825fedd3cb5cd0f59805a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 15 Jan 2013 15:01:16 +0100 Subject: optimize memory layout for much better cache hits Moave element status out of batch_obj_t because we get a *much* better cache hit ratio this way. Note that this is really a HUGE saving, even if it doesn't look so (both profiler data as well as practical tests indicate that!). --- runtime/batch.h | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 097e104a..03122bd0 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -46,7 +46,6 @@ typedef unsigned char batch_state_t; */ struct batch_obj_s { msg_t *pMsg; - batch_state_t state; /* associated state */ /* work variables for action processing; these are reused for each action (or block of * actions) */ @@ -84,6 +83,13 @@ struct batch_s { sbool *active; /* which messages are active for processing, NULL=all */ sbool bSingleRuleset; /* do all msgs of this batch use a single ruleset? */ batch_obj_t *pElem; /* batch elements */ + batch_state_t *eltState;/* state (array!) for individual objects. + NOTE: we have moved this out of batch_obj_t because we + get a *much* better cache hit ratio this way. So do not + move it back into this structure! Note that this is really + a HUGE saving, even if it doesn't look so (both profiler + data as well as practical tests indicate that!). + */ }; @@ -118,8 +124,8 @@ batchNumMsgs(batch_t *pBatch) { */ static inline void batchSetElemState(batch_t *pBatch, int i, batch_state_t newState) { - if(pBatch->pElem[i].state != BATCH_STATE_DISC) - pBatch->pElem[i].state = newState; + if(pBatch->eltState[i] != BATCH_STATE_DISC) + pBatch->eltState[i] = newState; } @@ -128,7 +134,7 @@ batchSetElemState(batch_t *pBatch, int i, batch_state_t newState) { */ static inline int batchIsValidElem(batch_t *pBatch, int i) { - return( (pBatch->pElem[i].state != BATCH_STATE_DISC) + return( (pBatch->eltState[i] != BATCH_STATE_DISC) && (pBatch->active == NULL || pBatch->active[i])); } @@ -163,6 +169,7 @@ batchInit(batch_t *pBatch, int maxElem) { pBatch->iDoneUpTo = 0; pBatch->maxElem = maxElem; CHKmalloc(pBatch->pElem = calloc((size_t)maxElem, sizeof(batch_obj_t))); + CHKmalloc(pBatch->eltState = calloc((size_t)maxElem, sizeof(batch_state_t))); // TODO: replace calloc by inidividual writes? finalize_it: RETiRet; -- cgit v1.2.3 From b69da427ebfeb2bbee2a4e4c086835c0397ed69e Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 18 Jan 2013 16:17:40 +0100 Subject: fix memory leak (regression from batch optimization work - unreleased) --- runtime/batch.h | 1 + 1 file changed, 1 insertion(+) (limited to 'runtime/batch.h') diff --git a/runtime/batch.h b/runtime/batch.h index 03122bd0..2ec07670 100644 --- a/runtime/batch.h +++ b/runtime/batch.h @@ -156,6 +156,7 @@ batchFree(batch_t *pBatch) { } } free(pBatch->pElem); + free(pBatch->eltState); } -- cgit v1.2.3