TriggerMail / lazylru / 185

86%

master: 98%

push

buildkite

always taking at least a read lock on shouldBubble (#33)

Previously, we were making a dirty read in `Get` when calling `shouldBubble` to see if we should bubble the item up. This creates a race condition with the write path. In cases where it was determined we should bubble up, we would take an exclusive lock, then check again. *Technically*, this is a race condition because we did make that dirty read, though we atoned for our sins with that second read.

Do we care? Well, maybe. The second read should make the action safe enough, but it does trip the race detector. There is no way to tell the race detector that this one is OK, so leaving this race condition in the code means that consumers of the library can't use the race detector for their own code. That's not an acceptable trade-off.

To avoid that, we can take a read lock before checking `shouldBubble`. If the answer is "no," we can just exit the function. If yes, we're going to have to take the exclusive lock anyway, so take the lock, check again, and bubble or not as appropriate. The impact of this lock is <0.5% when the cache is under 75% full and approximately 3% when the cache is over 3% full. That 3% represents only a few nanoseconds, so it's not as bad as it looks at first glance.

16 of 16 new or added lines in 1 file covered. (100.0%)

1410 of 1648 relevant lines covered (85.56%)

1822776.93 hits per line