Wrapping behavior of `impl ClockSequence for Context`

[fef1312]

Wrapping behavior of `impl ClockSequence for Context`

[KodrAus]

Hi @fef1312 👋

Hmm, the result of u16::MAX >> 2 should be 0x3fff rather than 0x1fff right? 🤔 Would a method using a bitwise AND actually have lower entropy than the current method that wraps? Or would they be the same? In the end we're mapping 16 bits down to 14, so if they're the same then an AND definitely sounds preferable to a division.

[fef1312]

Oops, you're right about the 0x3fff. My bad!

What I mean by "lower entropy" is that anything modulo N is always less than N, so the largest value this method will ever return in its current implementation is actually 0x3ffe. In other words, the counter has "only" 16383 rather than 16384 possible states. To wrap at exactly 14 bits, you have two options:

self.count.fetch_add(1, Ordering::AcqRel) % (１ << 14)
self.count.fetch_add(1, Ordering::AcqRel) & (u16::MAX >> 2)

These are mathematically equivalent because it's a power of two, so rustc will pick the AND version either way. But again, this is merely a technicality and a single division here won't make an observable difference in the grand scheme of things. The main reason I brought it up was because it's technically not compliant with the RFC.

[KodrAus]

Aha, thanks for the explanation. It was simply my ignorance that lead to the original implementation and I think your suggested alternative:

self.count.fetch_add(1, Ordering::AcqRel) & (u16::MAX >> 2)

would be a great fix.

Would you like to submit a PR for it?

[fef1312]

Alright, sure!