Commit 1509cebb authored by Jim Meyering's avatar Jim Meyering Committed by woo

MPMCQueue.h: fix a bug introduced while accommodating -Wsign-compare

Summary:
* folly/MPMCQueue.h (waitForTurn): My changes to accommodate
-Wsign-compare introduced a bug that could cause lots of wasted CPU.
(See @scottf's comments on the blamed patch.)
Subtraction involving an unsigned type will always yield an unsigned
result, and there was at least this one instance where the result
would sometimes have to be a small negative number.  Instead, we'd
get approximately UINT32_MAX.  The fix is to explicitly cast the
difference to a signed type.

Test Plan:
Run this and note there is no error:
(this ensures it remains -Wsign-compare-clean with gcc-4.9)

fbconfig --platform-all=gcc-4.9-glibc-2.20 tao/server && fbmake dbgo

Before this change, running the mpmc_queue_test test would get
timeout errors ~8 in 100 runs.  Confirm that with this patch, now
there is no type of failure. I.e., run these commands and ensure
the grep prints nothing:

fbconfig folly/test:mpmc_queue_test
for i in $(seq 100); do fbmake runtests_dbgo >& makerr-$i; done
grep -E '(TIMEOUT|FATAL|FAIL).* [1-9]' makerr-*

Reviewed By: hans@fb.com

Subscribers: trunkagent, scottf, folly-diffs@

FB internal diff: D1783672

Tasks: 5941250

Signature: t1:1783672:1421350260:d7d306e8cf3ed3ee636d06228e1f24a2c2796b54

Blame Revision: D1766722
parent 6502bca2
...@@ -664,8 +664,9 @@ struct TurnSequencer { ...@@ -664,8 +664,9 @@ struct TurnSequencer {
} else { } else {
// try once, keep moving if CAS fails. Exponential moving average // try once, keep moving if CAS fails. Exponential moving average
// with alpha of 7/8 // with alpha of 7/8
// Be careful that the quantity we add to prevThresh is signed.
spinCutoff.compare_exchange_weak( spinCutoff.compare_exchange_weak(
prevThresh, prevThresh + (target - prevThresh) / 8); prevThresh, prevThresh + int(target - prevThresh) / 8);
} }
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment