From eac2f407712046f9a3b91c244301467d31ec3819 Mon Sep 17 00:00:00 2001 From: Christopher Dykes Date: Fri, 16 Dec 2016 00:01:29 -0800 Subject: [PATCH 1/1] Revert D4310312: [Folly] Enable -Wunreachable-code Summary: This reverts commit 8178dacc9268e1001efc5f803a35ef49e23d692a Differential Revision: D4310312 fbshipit-source-id: 7c4b90e834f1a95e51524e1e82ac5294e5ba2dc5 --- folly/Benchmark.cpp | 83 +++++++++++++++++++ folly/dynamic.cpp | 7 +- folly/experimental/DynamicParser.cpp | 1 + .../exception_tracer/ExceptionTracerLib.cpp | 24 +++--- folly/fibers/test/FibersTest.cpp | 2 +- 5 files changed, 99 insertions(+), 18 deletions(-) diff --git a/folly/Benchmark.cpp b/folly/Benchmark.cpp index 6c481335..ccae0344 100644 --- a/folly/Benchmark.cpp +++ b/folly/Benchmark.cpp @@ -139,6 +139,41 @@ meanVariance(const double * begin, const double *const end) { return make_pair(sum / n, sqrt((sum2 - sum * sum / n) / n)); } +/** + * Computes the mode of a sample set through brute force. Assumes + * input is sorted. + */ +static double mode(const double * begin, const double *const end) { + assert(begin < end); + // Lower bound and upper bound for result and their respective + // densities. + auto + result = 0.0, + bestDensity = 0.0; + + // Get the variance so we pass it down to density() + auto const sigma = meanVariance(begin, end).second; + if (!sigma) { + // No variance means constant signal + return *begin; + } + + FOR_EACH_RANGE (i, begin, end) { + assert(i == begin || *i >= i[-1]); + auto candidate = density(begin, end, *i, sigma * sqrt(2.0)); + if (candidate > bestDensity) { + // Found a new best + bestDensity = candidate; + result = *i; + } else { + // Density is decreasing... we could break here if we definitely + // knew this is unimodal. + } + } + + return result; +} + /** * Given a bunch of benchmark samples, estimate the actual run time. */ @@ -147,7 +182,55 @@ static double estimateTime(double * begin, double * end) { // Current state of the art: get the minimum. After some // experimentation, it seems taking the minimum is the best. + return *min_element(begin, end); + + // What follows after estimates the time as the mode of the + // distribution. + + // Select the awesomest (i.e. most frequent) result. We do this by + // sorting and then computing the longest run length. + sort(begin, end); + + // Eliminate outliers. A time much larger than the minimum time is + // considered an outlier. + while (end[-1] > 2.0 * *begin) { + --end; + if (begin == end) { + LOG(INFO) << *begin; + } + assert(begin < end); + } + + double result = 0; + + /* Code used just for comparison purposes */ { + unsigned bestFrequency = 0; + unsigned candidateFrequency = 1; + double candidateValue = *begin; + for (auto current = begin + 1; ; ++current) { + if (current == end || *current != candidateValue) { + // Done with the current run, see if it was best + if (candidateFrequency > bestFrequency) { + bestFrequency = candidateFrequency; + result = candidateValue; + } + if (current == end) { + break; + } + // Start a new run + candidateValue = *current; + candidateFrequency = 1; + } else { + // Cool, inside a run, increase the frequency + ++candidateFrequency; + } + } + } + + result = mode(begin, end); + + return result; } static double runBenchmarkGetNSPerIteration(const BenchmarkFun& fun, diff --git a/folly/dynamic.cpp b/folly/dynamic.cpp index e9a37a99..5e656c5e 100644 --- a/folly/dynamic.cpp +++ b/folly/dynamic.cpp @@ -14,10 +14,8 @@ * limitations under the License. */ -#include - -#include #include +#include #include namespace folly { @@ -279,8 +277,9 @@ std::size_t dynamic::hash() const { const auto& str = getString(); return ::folly::hash::fnv32_buf(str.data(), str.size()); } + default: + CHECK(0); abort(); } - assume_unreachable(); } char const* dynamic::typeName(Type t) { diff --git a/folly/experimental/DynamicParser.cpp b/folly/experimental/DynamicParser.cpp index 734013c9..18dd25fd 100644 --- a/folly/experimental/DynamicParser.cpp +++ b/folly/experimental/DynamicParser.cpp @@ -96,6 +96,7 @@ void DynamicParser::reportError( break; // Continue parsing case OnError::THROW: stack_.throwErrors(); // Package releaseErrors() into an exception. + LOG(FATAL) << "Not reached"; // silence lint false positive default: LOG(FATAL) << "Bad onError_: " << static_cast(onError_); } diff --git a/folly/experimental/exception_tracer/ExceptionTracerLib.cpp b/folly/experimental/exception_tracer/ExceptionTracerLib.cpp index a0a60dd3..2ba97b78 100644 --- a/folly/experimental/exception_tracer/ExceptionTracerLib.cpp +++ b/folly/experimental/exception_tracer/ExceptionTracerLib.cpp @@ -28,12 +28,13 @@ namespace __cxxabiv1 { extern "C" { -[[noreturn]] void __cxa_throw( +void __cxa_throw( void* thrownException, std::type_info* type, - void (*destructor)(void*)); + void (*destructor)(void*)) __attribute__((__noreturn__)); void* __cxa_begin_catch(void* excObj) throw(); -[[noreturn]] void __cxa_rethrow(void); +void __cxa_rethrow(void) __attribute__((__noreturn__)); +void __cxa_rethrow(void); void __cxa_end_catch(void); } @@ -89,17 +90,17 @@ DECLARE_CALLBACK(RethrowException); namespace __cxxabiv1 { -[[noreturn]] void __cxa_throw(void* thrownException, - std::type_info* type, - void (*destructor)(void*)) { +void __cxa_throw(void* thrownException, + std::type_info* type, + void (*destructor)(void*)) { static auto orig_cxa_throw = reinterpret_cast(dlsym(RTLD_NEXT, "__cxa_throw")); getCxaThrowCallbacks().invoke(thrownException, type, destructor); orig_cxa_throw(thrownException, type, destructor); - __builtin_unreachable(); + __builtin_unreachable(); // orig_cxa_throw never returns } -[[noreturn]] void __cxa_rethrow() { +void __cxa_rethrow() { // __cxa_rethrow leaves the current exception on the caught stack, // and __cxa_begin_catch recognizes that case. We could do the same, but // we'll implement something simpler (and slower): we pop the exception from @@ -109,7 +110,7 @@ namespace __cxxabiv1 { dlsym(RTLD_NEXT, "__cxa_rethrow")); getCxaRethrowCallbacks().invoke(); orig_cxa_rethrow(); - __builtin_unreachable(); + __builtin_unreachable(); // orig_cxa_rethrow never returns } void* __cxa_begin_catch(void* excObj) throw() { @@ -142,10 +143,7 @@ void rethrow_exception(std::exception_ptr ep) { "_ZSt17rethrow_exceptionNSt15__exception_ptr13exception_ptrE")); getRethrowExceptionCallbacks().invoke(ep); orig_rethrow_exception(ep); - // Clang knows this is unreachable, but GCC doesn't. -#ifndef __clang__ - __builtin_unreachable(); -#endif + __builtin_unreachable(); // orig_rethrow_exception never returns } } // namespace std diff --git a/folly/fibers/test/FibersTest.cpp b/folly/fibers/test/FibersTest.cpp index 1577b5d3..d1b60cee 100644 --- a/folly/fibers/test/FibersTest.cpp +++ b/folly/fibers/test/FibersTest.cpp @@ -1963,7 +1963,7 @@ TEST(FiberManager, ABD_DispatcherDestroyedBeforeCallingCommit) { dispatchJobs(executor, jobs, results); throw std::runtime_error( "Unexpected exception in user code before commit called"); - // atomicBatchDispatcher.commit(); + atomicBatchDispatcher.commit(); } catch (...) { /* User code handles the exception and does not exit process */ } -- 2.34.1