On Tue, Mar 26, 2013 at 7:16 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:

Le 26/03/13 05:57, Vicente J. Botet Escriba a écrit :

I appreciate your efforts going to a more understandable code. I don't know why HH has written the code like that, but usually he has good reasons.

I guess that I have the answer why HH wrote the code like this. Portability, if unsigned is 64 bits, your bitfield patch can not take advantage of it as you need to give the number of bits

+ n_readers_ : 31;

Or can you?

Yes, the maximum number of simultaneous readers becomes limited to (2^31-1) with my patch, regardless of sizeof(unsigned). That's still a _lot_ of threads. Do you really think that this can be an issue?

In order to ensure the same performance, what about a patch using getters/setters instead of the direct use of the bitfields, and define these functions either using the current code either using the bitfields so that we can have both and compare easily.

Anyway, forget the patch I will do it myself using inline functions.

My impression of bitfields were that they were just "syntactic sugar" to make bitmask logic more readable. You do indeed loose some control over the layout and padding, but that should not be an issue in this situation. The performance should be identical to bitmask operators.

win32/shared_mutex already uses bitfields to fit all state variables within a 32-bit unsigned without and bitmasking. I am merely proposing to do the same also for v2/shared_mutex, so that the code becomes more readable.

Regards,
Fredrik