Commit 51635047 authored by Nick Sukhanov's avatar Nick Sukhanov Committed by Facebook Github Bot

Added comparison with tolerance for doubles.

Summary:
We were generating a lot of errors because of false positives when compare double.

This diffs is an attempt to fix this introducing comparison with tolerance for floating types.

Reviewed By: yfeldblum

Differential Revision: D13639839

fbshipit-source-id: a81381384495c26ee176620a5f61b5c6109cf8b8
parent 6b34b1b7
...@@ -171,7 +171,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate( ...@@ -171,7 +171,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate(
ValueType low; ValueType low;
ValueType high; ValueType high;
if (bucketIdx == 0) { if (bucketIdx == 0) {
if (avg > min_) { if (less(min_, avg)) {
// This normally shouldn't happen. This bucket is only supposed to track // This normally shouldn't happen. This bucket is only supposed to track
// values less than min_. Most likely this means that integer overflow // values less than min_. Most likely this means that integer overflow
// occurred, and the code in avgFromBucket() returned a huge value // occurred, and the code in avgFromBucket() returned a huge value
...@@ -194,7 +194,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate( ...@@ -194,7 +194,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate(
low = std::numeric_limits<ValueType>::min(); low = std::numeric_limits<ValueType>::min();
} }
} else if (bucketIdx == buckets_.size() - 1) { } else if (bucketIdx == buckets_.size() - 1) {
if (avg < max_) { if (less(avg, max_)) {
// Most likely this means integer overflow occurred. See the comments // Most likely this means integer overflow occurred. See the comments
// above in the minimum case. // above in the minimum case.
LOG(ERROR) << "invalid average value in histogram maximum bucket: " << avg LOG(ERROR) << "invalid average value in histogram maximum bucket: " << avg
...@@ -212,7 +212,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate( ...@@ -212,7 +212,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate(
} else { } else {
low = getBucketMin(bucketIdx); low = getBucketMin(bucketIdx);
high = getBucketMax(bucketIdx); high = getBucketMax(bucketIdx);
if (avg < low || avg > high) { if (less(avg, low) || less(high, avg)) {
// Most likely this means an integer overflow occurred. // Most likely this means an integer overflow occurred.
// See the comments above. Return the midpoint between low and high // See the comments above. Return the midpoint between low and high
// as a best guess, since avg is meaningless. // as a best guess, since avg is meaningless.
...@@ -230,7 +230,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate( ...@@ -230,7 +230,7 @@ T HistogramBuckets<T, BucketType>::getPercentileEstimate(
// Assume that the median value in this bucket is the same as the average // Assume that the median value in this bucket is the same as the average
// value. // value.
double medianPct = (lowPct + highPct) / 2.0; double medianPct = (lowPct + highPct) / 2.0;
if (pct < medianPct) { if (less(pct, medianPct)) {
// Assume that the data points lower than the median of this bucket // Assume that the data points lower than the median of this bucket
// are uniformly distributed between low and avg // are uniformly distributed between low and avg
double pctThroughSection = (pct - lowPct) / (medianPct - lowPct); double pctThroughSection = (pct - lowPct) / (medianPct - lowPct);
......
...@@ -16,12 +16,14 @@ ...@@ -16,12 +16,14 @@
#pragma once #pragma once
#include <cmath>
#include <cstddef> #include <cstddef>
#include <cstdint> #include <cstdint>
#include <limits> #include <limits>
#include <ostream> #include <ostream>
#include <stdexcept> #include <stdexcept>
#include <string> #include <string>
#include <type_traits>
#include <vector> #include <vector>
#include <folly/CPortability.h> #include <folly/CPortability.h>
...@@ -221,6 +223,12 @@ class HistogramBuckets { ...@@ -221,6 +223,12 @@ class HistogramBuckets {
} }
private: private:
template <typename V>
bool less(const V& lhs, const V& rhs) const {
using nl = std::numeric_limits<V>;
return lhs < rhs && (nl::is_integer || rhs - lhs > nl::epsilon());
}
ValueType bucketSize_; ValueType bucketSize_;
ValueType min_; ValueType min_;
ValueType max_; ValueType max_;
......
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