From bce5bec5517f8c9ec448db9c2ffdf977dfff18eb Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 14 Apr 2023 04:27:57 -0700 Subject: Reland "Get rid of tail padding within `absl::Duration`. This reduces memory usage needs when storing duration in containers (e.g. `vector` uses 25% less memory), and allows classes with `absl::Duration` fields to fit other stuff in memory previously used by tail padding (e.g. `std::optional` is now 16 bytes instead of 24)." PiperOrigin-RevId: 524256689 Change-Id: Ibf40d9e5411020179fa34c972349c7b58aa9d908 --- absl/time/duration.cc | 39 +++++++++++++---------- absl/time/duration_test.cc | 8 +++++ absl/time/time.h | 79 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 106 insertions(+), 20 deletions(-) (limited to 'absl/time') diff --git a/absl/time/duration.cc b/absl/time/duration.cc index 26fadb31..634e5d58 100644 --- a/absl/time/duration.cc +++ b/absl/time/duration.cc @@ -400,16 +400,18 @@ int64_t IDivDuration(bool satq, const Duration num, const Duration den, Duration& Duration::operator+=(Duration rhs) { if (time_internal::IsInfiniteDuration(*this)) return *this; if (time_internal::IsInfiniteDuration(rhs)) return *this = rhs; - const int64_t orig_rep_hi = rep_hi_; - rep_hi_ = - DecodeTwosComp(EncodeTwosComp(rep_hi_) + EncodeTwosComp(rhs.rep_hi_)); + const int64_t orig_rep_hi = rep_hi_.Get(); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) + + EncodeTwosComp(rhs.rep_hi_.Get())); if (rep_lo_ >= kTicksPerSecond - rhs.rep_lo_) { - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_) + 1); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) + 1); rep_lo_ -= kTicksPerSecond; } rep_lo_ += rhs.rep_lo_; - if (rhs.rep_hi_ < 0 ? rep_hi_ > orig_rep_hi : rep_hi_ < orig_rep_hi) { - return *this = rhs.rep_hi_ < 0 ? -InfiniteDuration() : InfiniteDuration(); + if (rhs.rep_hi_.Get() < 0 ? rep_hi_.Get() > orig_rep_hi + : rep_hi_.Get() < orig_rep_hi) { + return *this = + rhs.rep_hi_.Get() < 0 ? -InfiniteDuration() : InfiniteDuration(); } return *this; } @@ -417,18 +419,21 @@ Duration& Duration::operator+=(Duration rhs) { Duration& Duration::operator-=(Duration rhs) { if (time_internal::IsInfiniteDuration(*this)) return *this; if (time_internal::IsInfiniteDuration(rhs)) { - return *this = rhs.rep_hi_ >= 0 ? -InfiniteDuration() : InfiniteDuration(); + return *this = rhs.rep_hi_.Get() >= 0 ? -InfiniteDuration() + : InfiniteDuration(); } - const int64_t orig_rep_hi = rep_hi_; - rep_hi_ = - DecodeTwosComp(EncodeTwosComp(rep_hi_) - EncodeTwosComp(rhs.rep_hi_)); + const int64_t orig_rep_hi = rep_hi_.Get(); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) - + EncodeTwosComp(rhs.rep_hi_.Get())); if (rep_lo_ < rhs.rep_lo_) { - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_) - 1); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) - 1); rep_lo_ += kTicksPerSecond; } rep_lo_ -= rhs.rep_lo_; - if (rhs.rep_hi_ < 0 ? rep_hi_ < orig_rep_hi : rep_hi_ > orig_rep_hi) { - return *this = rhs.rep_hi_ >= 0 ? -InfiniteDuration() : InfiniteDuration(); + if (rhs.rep_hi_.Get() < 0 ? rep_hi_.Get() < orig_rep_hi + : rep_hi_.Get() > orig_rep_hi) { + return *this = rhs.rep_hi_.Get() >= 0 ? -InfiniteDuration() + : InfiniteDuration(); } return *this; } @@ -439,7 +444,7 @@ Duration& Duration::operator-=(Duration rhs) { Duration& Duration::operator*=(int64_t r) { if (time_internal::IsInfiniteDuration(*this)) { - const bool is_neg = (r < 0) != (rep_hi_ < 0); + const bool is_neg = (r < 0) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleFixed(*this, r); @@ -447,7 +452,7 @@ Duration& Duration::operator*=(int64_t r) { Duration& Duration::operator*=(double r) { if (time_internal::IsInfiniteDuration(*this) || !IsFinite(r)) { - const bool is_neg = (std::signbit(r) != 0) != (rep_hi_ < 0); + const bool is_neg = std::signbit(r) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleDouble(*this, r); @@ -455,7 +460,7 @@ Duration& Duration::operator*=(double r) { Duration& Duration::operator/=(int64_t r) { if (time_internal::IsInfiniteDuration(*this) || r == 0) { - const bool is_neg = (r < 0) != (rep_hi_ < 0); + const bool is_neg = (r < 0) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleFixed(*this, r); @@ -463,7 +468,7 @@ Duration& Duration::operator/=(int64_t r) { Duration& Duration::operator/=(double r) { if (time_internal::IsInfiniteDuration(*this) || !IsValidDivisor(r)) { - const bool is_neg = (std::signbit(r) != 0) != (rep_hi_ < 0); + const bool is_neg = std::signbit(r) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleDouble(*this, r); diff --git a/absl/time/duration_test.cc b/absl/time/duration_test.cc index 3bb6081d..dcf7aad6 100644 --- a/absl/time/duration_test.cc +++ b/absl/time/duration_test.cc @@ -16,6 +16,7 @@ #include // for timeval #endif +#include #include #include // NOLINT(build/c++11) #include @@ -1826,4 +1827,11 @@ TEST(Duration, AbslStringify) { EXPECT_EQ(absl::StrFormat("%v", d), absl::FormatDuration(d)); } +TEST(Duration, NoPadding) { + // Should match the size of a struct with uint32_t alignment and no padding. + using NoPadding = std::array; + EXPECT_EQ(sizeof(NoPadding), sizeof(absl::Duration)); + EXPECT_EQ(alignof(NoPadding), alignof(absl::Duration)); +} + } // namespace diff --git a/absl/time/time.h b/absl/time/time.h index f255593e..29178c77 100644 --- a/absl/time/time.h +++ b/absl/time/time.h @@ -84,6 +84,7 @@ struct timeval; #include #include +#include "absl/base/config.h" #include "absl/base/macros.h" #include "absl/strings/string_view.h" #include "absl/time/civil_time.h" @@ -219,7 +220,7 @@ class Duration { template friend H AbslHashValue(H h, Duration d) { - return H::combine(std::move(h), d.rep_hi_, d.rep_lo_); + return H::combine(std::move(h), d.rep_hi_.Get(), d.rep_lo_); } private: @@ -228,7 +229,79 @@ class Duration { friend constexpr Duration time_internal::MakeDuration(int64_t hi, uint32_t lo); constexpr Duration(int64_t hi, uint32_t lo) : rep_hi_(hi), rep_lo_(lo) {} - int64_t rep_hi_; + + // We store `rep_hi_` 4-byte rather than 8-byte aligned to avoid 4 bytes of + // tail padding. + class HiRep { + public: + // Default constructor default-initializes `hi_`, which has the same + // semantics as default-initializing an `int64_t` (undetermined value). + HiRep() = default; + + HiRep(const HiRep&) = default; + HiRep& operator=(const HiRep&) = default; + + explicit constexpr HiRep(const int64_t value) + : // C++17 forbids default-initialization in constexpr contexts. We can + // remove this in C++20. +#if defined(ABSL_IS_BIG_ENDIAN) && ABSL_IS_BIG_ENDIAN + hi_(0), + lo_(0) +#else + lo_(0), + hi_(0) +#endif + { + *this = value; + } + + constexpr int64_t Get() const { + const uint64_t unsigned_value = + (static_cast(hi_) << 32) | static_cast(lo_); + // `static_cast(unsigned_value)` is implementation-defined + // before c++20. On all supported platforms the behaviour is that mandated + // by c++20, i.e. "If the destination type is signed, [...] the result is + // the unique value of the destination type equal to the source value + // modulo 2^n, where n is the number of bits used to represent the + // destination type." + static_assert( + (static_cast((std::numeric_limits::max)()) == + int64_t{-1}) && + (static_cast(static_cast( + (std::numeric_limits::max)()) + + 1) == + (std::numeric_limits::min)()), + "static_cast(uint64_t) does not have c++20 semantics"); + return static_cast(unsigned_value); + } + + constexpr HiRep& operator=(const int64_t value) { + // "If the destination type is unsigned, the resulting value is the + // smallest unsigned value equal to the source value modulo 2^n + // where `n` is the number of bits used to represent the destination + // type". + const auto unsigned_value = static_cast(value); + hi_ = static_cast(unsigned_value >> 32); + lo_ = static_cast(unsigned_value); + return *this; + } + + private: + // Notes: + // - Ideally we would use a `char[]` and `std::bitcast`, but the latter + // does not exist (and is not constexpr in `absl`) before c++20. + // - Order is optimized depending on endianness so that the compiler can + // turn `Get()` (resp. `operator=()`) into a single 8-byte load (resp. + // store). +#if defined(ABSL_IS_BIG_ENDIAN) && ABSL_IS_BIG_ENDIAN + uint32_t hi_; + uint32_t lo_; +#else + uint32_t lo_; + uint32_t hi_; +#endif + }; + HiRep rep_hi_; uint32_t rep_lo_; }; @@ -1511,7 +1584,7 @@ ABSL_ATTRIBUTE_CONST_FUNCTION constexpr Duration MakeNormalizedDuration( // Provide access to the Duration representation. ABSL_ATTRIBUTE_CONST_FUNCTION constexpr int64_t GetRepHi(Duration d) { - return d.rep_hi_; + return d.rep_hi_.Get(); } ABSL_ATTRIBUTE_CONST_FUNCTION constexpr uint32_t GetRepLo(Duration d) { return d.rep_lo_; -- cgit v1.2.3