os_unix.c: unixShmLock changes, saving cycles

(1) By Martijn (Martijn81) on 2022-03-07 18:18:55 [source]

Hi!

I found two minor changes, to save a few cycles in unixShmLock().

Find:
      for(ii=ofst; ii<ofst+n; ii++){
        if( aLock[ii]>((p->sharedMask & (1<<ii)) ? 1 : 0) ){
          bUnlock = 0;
        }
      }

Replace:
      for(ii=ofst; ii<ofst+n; ii++){
        if( aLock[ii]>((p->sharedMask & (1<<ii)) ? 1 : 0) ){
          bUnlock = 0;
          break;
        }
      }
      
Find:
        for(ii=ofst; ii<ofst+n; ii++){
          aLock[ii] = -1;
        }


Replace:
	memset(&aLock[ofst], -1, sizeof(int)*n);


Martijn.

(2) By Martijn (Martijn81) on 2022-03-11 08:05:38 in reply to 1 [link] [source]

I don't want to be rude or pushy. But above probably has gone by unnoticed.

Or i yet again said a dumb things...

(3) By Warren Young (wyoung) on 2022-03-11 09:01:36 in reply to 2 [link] [source]

"SQLite is open-source but it is not open-contribution."

That said, your first patch seems like an obvious improvement: you can't zero the variable any more zeroed than it's already been zeroed.

As for the second, they're equivalent on GCC under -Os, which means you're probably trying to teach the compiler something it already knows.

Switching to -O1 or higher seems to save a few instructions, but I suspect if you benchmarked it, you'd find that the savings get eaten up by taking 4 or 8 times as many iterations to set the aLock array elements to -1 byte-by-byte.

Note a few minor style improvements: use of pointer arithmetic instead of noisier "address of array element", and use of "sizeof(aLock[0])" instead of "sizeof(int)" to make the code more resilient in the face of data type changes.

Also note that I've switched the code to unsigned size and offset values. This does save a few more instructions since negative sizes and offsets are obvious errors (currently enforced by asserts rather than data types) and informing the compiler that this cannot happen by precluding this in the API allows it to generate better code. Whether this is safe depends on what the calling code actually does.

(4.1) By Martijn (Martijn81) on 2022-03-11 09:24:18 edited from 4.0 in reply to 3 [link] [source]

Thanks!

The first probably iterates over all open connections which is not good. And ii is not used after the loop.

My motivation for second patch, was also to address consistency. Because in the code of unixShmLock() memset() is also used to reset the locks:

memset(&aLock[ofst], 0, sizeof(int)*n);

"sizeof(aLock[0])"

Totally agree!