From: Adam C. Emerson Date: Mon, 14 Mar 2016 20:14:52 +0000 (-0400) Subject: time: Have our time_point and sign the difference too X-Git-Tag: v11.0.0~824^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=93166300779b30bb4423ce2f316772ea7fb0dfbc;p=ceph.git time: Have our time_point and sign the difference too Make operator - between Ceph time points always return a signed duration. This fixes the underflow pointed out by Kefu Chai . Signed-off-by: Adam C. Emerson --- diff --git a/src/common/ceph_time.h b/src/common/ceph_time.h index 6a398fa24394..70b30a9dda41 100644 --- a/src/common/ceph_time.h +++ b/src/common/ceph_time.h @@ -263,6 +263,44 @@ namespace ceph { return time_point(seconds(ts.tv_sec) + nanoseconds(ts.tv_nsec)); } }; + + // So that our subtractions produce negative spans rather than + // arithmetic underflow. + namespace { + template + inline auto difference(std::chrono::duration minuend, + std::chrono::duration subtrahend) + -> typename std::common_type< + std::chrono::duration::type, + Period1>, + std::chrono::duration::type, + Period2> >::type { + // Foo. + using srep = + typename std::common_type< + std::chrono::duration::type, + Period1>, + std::chrono::duration::type, + Period2> >::type; + return srep(srep(minuend).count() - srep(subtrahend).count()); + } + + template + inline auto difference( + typename std::chrono::time_point minuend, + typename std::chrono::time_point subtrahend) + -> typename std::common_type< + std::chrono::duration::type, + typename Duration1::period>, + std::chrono::duration::type, + typename Duration2::period> >::type { + return difference(minuend.time_since_epoch(), + subtrahend.time_since_epoch()); + } + } } // namespace time_detail // duration is the concrete time representation for our code in the @@ -357,16 +395,46 @@ namespace ceph { ceil(timepoint.time_since_epoch(), precision)); } - static inline timespan make_timespan(const double d) { - return std::chrono::duration_cast( - std::chrono::duration(d)); + namespace { + inline timespan make_timespan(const double d) { + return std::chrono::duration_cast( + std::chrono::duration(d)); + } } std::ostream& operator<<(std::ostream& m, const timespan& t); std::ostream& operator<<(std::ostream& m, const real_time& t); std::ostream& operator<<(std::ostream& m, const mono_time& t); - // To gain sane behavior with signs + // The way std::chrono handles the return type of subtraction is not + // wonderful. The difference of two unsigned types SHOULD be signed. + + namespace { + inline signedspan operator -(real_time minuend, + real_time subtrahend) { + return time_detail::difference(minuend, subtrahend); + } + + inline signedspan operator -(coarse_real_time minuend, + coarse_real_time subtrahend) { + return time_detail::difference(minuend, subtrahend); + } + + inline signedspan operator -(mono_time minuend, + mono_time subtrahend) { + return time_detail::difference(minuend, subtrahend); + } + + inline signedspan operator -(coarse_mono_time minuend, + coarse_mono_time subtrahend) { + return time_detail::difference(minuend, subtrahend); + } + } + + // We could add specializations of time_point - duration and + // time_point + duration to assert on overflow, but I don't think we + // should. + } // namespace ceph // We need these definitions to be able to hande ::encode/::decode on @@ -411,6 +479,15 @@ namespace std { ::decode(t, p); } } // namespace chrono + + // An overload of our own + namespace { + inline timespan abs(signedspan z) { + return z > signedspan::zero() ? + std::chrono::duration_cast(z) : + timespan(-z.count()); + } + } } // namespace std #endif // COMMON_CEPH_TIME_H diff --git a/src/test/common/test_time.cc b/src/test/common/test_time.cc index a54fe90b83a3..944492031ff7 100644 --- a/src/test/common/test_time.cc +++ b/src/test/common/test_time.cc @@ -122,9 +122,7 @@ static void system_clock_conversions() { ASSERT_EQ(Clock::to_double(brt), bd); // Fudge factor - ASSERT_LT((Clock::from_double(bd) > brt ? - Clock::from_double(bd) - brt : - brt - Clock::from_double(bd)).count(), 30U); + ASSERT_LT(std::abs((Clock::from_double(bd) - brt).count()), 30); } TEST(RealClock, Sanity) { @@ -144,3 +142,37 @@ TEST(CoarseRealClock, Sanity) { TEST(CoarseRealClock, Conversions) { system_clock_conversions(); } + +TEST(TimePoints, SignedSubtraciton) { + ceph::real_time rta(std::chrono::seconds(3)); + ceph::real_time rtb(std::chrono::seconds(5)); + + ceph::coarse_real_time crta(std::chrono::seconds(3)); + ceph::coarse_real_time crtb(std::chrono::seconds(5)); + + ceph::mono_time mta(std::chrono::seconds(3)); + ceph::mono_time mtb(std::chrono::seconds(5)); + + ceph::coarse_mono_time cmta(std::chrono::seconds(3)); + ceph::coarse_mono_time cmtb(std::chrono::seconds(5)); + + ASSERT_LT(rta - rtb, ceph::signedspan::zero()); + ASSERT_LT((rta - rtb).count(), 0); + ASSERT_GT(rtb - rta, ceph::signedspan::zero()); + ASSERT_GT((rtb - rta).count(), 0); + + ASSERT_LT(crta - crtb, ceph::signedspan::zero()); + ASSERT_LT((crta - crtb).count(), 0); + ASSERT_GT(crtb - crta, ceph::signedspan::zero()); + ASSERT_GT((crtb - crta).count(), 0); + + ASSERT_LT(mta - mtb, ceph::signedspan::zero()); + ASSERT_LT((mta - mtb).count(), 0); + ASSERT_GT(mtb - mta, ceph::signedspan::zero()); + ASSERT_GT((mtb - mta).count(), 0); + + ASSERT_LT(cmta - cmtb, ceph::signedspan::zero()); + ASSERT_LT((cmta - cmtb).count(), 0); + ASSERT_GT(cmtb - cmta, ceph::signedspan::zero()); + ASSERT_GT((cmtb - cmta).count(), 0); +}