[PATCH] caching optimization in share mode locks.

[Stefan (metze) Metzmacher]

Hi,

>> On Sat, Apr 04, 2015 at 07:55:59AM +0200, Volker Lendecke wrote:
>>> On Fri, Apr 03, 2015 at 12:35:50PM -0700, Jeremy Allison wrote:
>>>> History: An OEM came to me with the (attached
>>>> as the .png file) profile from cachegrind on
>>>> Samba 4.1.x.
>>>
>>> Without looking at the code: Did the OEM get back to you
>>> with test results regarding performance?
>>
>> It showed a significant reduction in CPU usage in
>> their test, but not a performance increase in
>> their specific benchmark. Which probably means
>> they weren't correct in identifying exactly
>> what the problem was.
>>
>> But hey - big reduction in CPU :-).
>>
>> It's worth looking at the code I think :-).
>>
> 
> It looks nice.
> 
> My concerns:
> 
> I'm a bit concerned about the use of a 64 bit "keys" and collisions.  128
> bit "keys" should get you clear of the issue.  (Note, this is really not
> something I expect to happen often in practice... but with enough installs
> of enough large systems... It will happen, 128 bits would get us out of the
> issue mostly I think.)
> 
> I don't think the change to 128 bit ids will hurt things much?  It'd be
> interesting to see how it impacts the app.  Note, I wouldn't bother
> changing the algorithm about +1 on the seq numbers to update the "top" 64
> bits...  It isn't worth the bother.
> 
> The rest looks good at first glance, I'll do another pass in a little bit

I'd just use the same key for the cache as for the tdb. The sequence
number check would then
be an additional if statement and we can remove the expired cache entry
if we notice it's not
current. This avoids a lot of expired cache entries for heavily
contended files.

We should also use integer types (hyper/uint64_t) in the idl instead of
an uint8 array.
BVAL() would used to extract the current sequence number and we would
avoid the ugly

+	/* Update the sequence number. */
+	memcpy(&seq, d->sequence_number, 8);
+	seq += 1;
+	memcpy(d->sequence_number, &seq, 8);

and could used use d->sequence_number += 1;

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150407/dc6304b9/attachment.pgp>