Simple Efficiency Query

[DavidDobson]

Hi

Can anyone think of a batter way of doing this:

#define NONE       0x00
#define PENDING    0x01
#define REQUESTED  0x02

typedef struct {
	
	uint8_t e2ConfigDispatch;
	uint8_t deviceForceSend;
	uint8_t commAckReq;
	uint8_t commReq;
	uint8_t sumMessageUpdate;
	uint8_t sumMessageForward;
	uint8_t LowPowere2ConfigDispatch;
	uint8_t lowPowerForward;
	uint8_t sendMessage;

} SysServices_t;

SysServices_t sysService;





// This bit:

bool sysServicePending() {

	// If there are any services waiting return true
	if (sysService.deviceForceSend == PENDING ||
	    sysService.commAckReq == PENDING ||
	    sysService.e2ConfigDispatch == PENDING ||
	    sysService.commReq == PENDING || 
	    sysService.sumMessageUpdate == PENDING ||
	    sysService.LowPower2ConfigDispatch == PENDING ||
	    sysService.sendMessage == PENDING ||
            sysService.sumMessageForward == PENDING) {
		
            return true;

	} // if

	return false;

} // sysServicePending

One of the sysService struct variables may be set to PENDING, i don’t need to know which one, I just need to know if there is one pending. Other variables may be randomly set to REQUESTED...

Any ideas would be greatly appreciated

David

[Salem]

If the result is true, how much work do you do as a result?

I'm sure you could save a new nS on this code, but what would be the point if the follow-on work takes mS ?

Leave the micro-optimisation to the compiler, and find more important things to improve by using a profiler.

FWIW, you can improve things slightly by arranging the expression so that the most common things to be true are first.

Other hacks will follow no doubt, once you've justified the necessity.

[cyberfish]

How about adding a "bool hasPending;"?

[DavidDobson]

Thanks both of you, just wanted to make sure I was not missing anything blindingly obvious, ill try re-arranging the if | sequence and look into adding a hasPending bit

Thank you

David

[nonoob]

Your struct has nine (9) flags but your ifs only check for 8. Maybe just a typo.

If you instead palce all flags in a contiguous unsigned char array, you could use

memchr ( const void * ptr, int value, size_t num );

to look for the PENDING code, if any.

[Zlatko]

You could have a single pending integer variable and allocate 1 bit for each service.
Then check if pending is or is not 0.

[Sebastiani]

>> You could have a single pending integer variable and allocate 1 bit for each service.
Then check if pending is or is not 0.

Except that it looks like each variable is actually a tri-state (NONE/PENDING/REQUESTED). Even if it were though, using bitmasks is not always the best way to go. It's much less flexible and more awkward to work with. I'd say unless performance is really an issue here (and I suspect that is not) don't even bother, although having a single boolean flag "hasPending" as cyberfish suggested seems like a cleaner solution altogether.