Commit 7fdf8141 authored by Jim Meyering's avatar Jim Meyering Committed by Viswanath Sivakumar

folly: ProducerConsumerQueue: avoid many -Wsign-compare errors

Summary:
There were many templates instantiated with signed types.
Changing them to unsigned avoids the compilation failures
noted below.  However, we'll have to watch arc-diff-spawned
tests for new failures, just in case.

This is by far the largest undecomposable patch of those
I've written for this task so far.  It is possible that
some subset of these changes accomplishes the same goal,
but once I managed to get a successful compilation, I have
only re-reviewed it once and did not try to shrink it.

Test Plan:
Without these changes, we would get the following 32 errors from gcc-4.9:
https://phabricator.fb.com/P19692642
With them, (and all of the other patches attached to the parent task,
all of fbcode compiles and passes its tests (modulo preexisting
failures and aborts).

Reviewed By: delong.j@fb.com

Subscribers: trunkagent, folly-diffs@

FB internal diff: D1773461

Tasks: 5941250

Signature: t1:1773461:1420821964:61635f3a2efc95e8d4457c36e7a5ee3a0a50df23
parent bc374dcb
...@@ -61,8 +61,8 @@ struct ProducerConsumerQueue : private boost::noncopyable { ...@@ -61,8 +61,8 @@ struct ProducerConsumerQueue : private boost::noncopyable {
// (No real synchronization needed at destructor time: only one // (No real synchronization needed at destructor time: only one
// thread can be doing this.) // thread can be doing this.)
if (!boost::has_trivial_destructor<T>::value) { if (!boost::has_trivial_destructor<T>::value) {
int read = readIndex_; size_t read = readIndex_;
int end = writeIndex_; size_t end = writeIndex_;
while (read != end) { while (read != end) {
records_[read].~T(); records_[read].~T();
if (++read == size_) { if (++read == size_) {
...@@ -168,8 +168,8 @@ private: ...@@ -168,8 +168,8 @@ private:
const uint32_t size_; const uint32_t size_;
T* const records_; T* const records_;
std::atomic<int> readIndex_; std::atomic<unsigned int> readIndex_;
std::atomic<int> writeIndex_; std::atomic<unsigned int> writeIndex_;
}; };
} }
......
...@@ -32,10 +32,10 @@ namespace { ...@@ -32,10 +32,10 @@ namespace {
using namespace folly; using namespace folly;
typedef int ThroughputType; typedef unsigned int ThroughputType;
typedef ProducerConsumerQueue<ThroughputType> ThroughputQueueType; typedef ProducerConsumerQueue<ThroughputType> ThroughputQueueType;
typedef long LatencyType; typedef unsigned long LatencyType;
typedef ProducerConsumerQueue<LatencyType> LatencyQueueType; typedef ProducerConsumerQueue<LatencyType> LatencyQueueType;
template<class QueueType> template<class QueueType>
...@@ -138,7 +138,7 @@ struct LatencyTest { ...@@ -138,7 +138,7 @@ struct LatencyTest {
pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset); pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
} }
for (int i = 0; i < iters_; ++i) { for (int i = 0; i < iters_; ++i) {
long enqueue_nsec; unsigned long enqueue_nsec;
while (!queue_.read(enqueue_nsec)) { while (!queue_.read(enqueue_nsec)) {
} }
......
...@@ -34,7 +34,7 @@ template<class T> struct TestTraits { ...@@ -34,7 +34,7 @@ template<class T> struct TestTraits {
}; };
template<> struct TestTraits<std::string> { template<> struct TestTraits<std::string> {
int limit() const { return 1 << 22; } unsigned int limit() const { return 1 << 22; }
std::string generate() const { return std::string(12, ' '); } std::string generate() const { return std::string(12, ' '); }
}; };
...@@ -61,7 +61,10 @@ struct PerfTest { ...@@ -61,7 +61,10 @@ struct PerfTest {
} }
void producer() { void producer() {
for (int i = 0; i < traits_.limit(); ++i) { // This is written differently than you might expect so that
// it does not run afoul of -Wsign-compare, regardless of the
// signedness of this loop's upper bound.
for (auto i = traits_.limit(); i > 0; --i) {
while (!queue_.write(traits_.generate())) { while (!queue_.write(traits_.generate())) {
} }
} }
...@@ -112,7 +115,7 @@ struct CorrectnessTest { ...@@ -112,7 +115,7 @@ struct CorrectnessTest {
{ {
const size_t testSize = traits_.limit(); const size_t testSize = traits_.limit();
testData_.reserve(testSize); testData_.reserve(testSize);
for (int i = 0; i < testSize; ++i) { for (size_t i = 0; i < testSize; ++i) {
testData_.push_back(traits_.generate()); testData_.push_back(traits_.generate());
} }
} }
...@@ -200,13 +203,13 @@ void correctnessTestType(const std::string& type) { ...@@ -200,13 +203,13 @@ void correctnessTestType(const std::string& type) {
} }
struct DtorChecker { struct DtorChecker {
static int numInstances; static unsigned int numInstances;
DtorChecker() { ++numInstances; } DtorChecker() { ++numInstances; }
DtorChecker(const DtorChecker& o) { ++numInstances; } DtorChecker(const DtorChecker& o) { ++numInstances; }
~DtorChecker() { --numInstances; } ~DtorChecker() { --numInstances; }
}; };
int DtorChecker::numInstances = 0; unsigned int DtorChecker::numInstances = 0;
} }
......
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