potential race condition

[tyrellj]

I am only reading the code, and maybe someone can point why I'm wrong, but it seems like there's a potential race condition between ph_counter_block_add() and ph_counter_scope_get()/ph_counter_scope_get_view().

If the thread calling ph_counter_block_add() is switched out before the call to ph_counter_block_record_write(), won't the version be wrong/old and allow inconsistent information to be read? I would think seqno would need incremented before and after the block value update in ph_counter_block_add().

I'm sure I'm probably missing something important here though, I apologize if that is the case.

[tyrellj]

I think I see now. I believe the seq number is mainly for dealing with multiple counter updates (via ph_counter_block_bulk_add). If ph_counter_block_add was modified to increment before and after, there might still be some strange behavior with 3 threads, instead of 2.

[wez]

Hi @tyrellj, thanks for your interest!

The counter stuff works by keeping counter updates local to each thread. Each thread-local counter bump also increments a thread local sequence number.

When a reader thread wants to consume the current counter value, it samples the sequence number of a given thread local counter block before it fetches the counts (non-atomically), and then compares the sequence number again at the end. If there is an inconsistency it means that the counter was bumped during the fetch and will need to be re-fetched.

This technique is similar to sequence locks (http://concurrencykit.org/doc/ck_sequence.html) except that counter updates don't have a write-begin; in practice, the stronger atomics of ck_sequence imposed a performance hit and were not necessary; since the overall sum of counters across threads is an approximation anyway, it is acceptable if we rarely read an individual counter value during a modification cycle and don't notice that the sequence number was bumped.

We moved away from directly using ck_sequence in c2753c2154a0cffc0bd70e8f28dd0ea884aab4fd due to strong evidence of a perf gain in some heavy profiling runs.

[tyrellj]

Thanks for the reply and your time. I missed the thread-local nature of the blocks, but I think I have a good understanding of the counters now. I think this library is pretty cool and I'm enjoying reading through it.