Skip to content

Commit

Permalink
Optimize moving or adopting AnyDependency{,Ref} when `inline_size =…
Browse files Browse the repository at this point in the history
…= 0`

by replacing an indirect call to `methods_and_handle_.methods->move()`
with a plain assignment of `methods_and_handle_.handle` and `repr_`.

If `ABSL_ATTRIBUTE_TRIVIAL_ABI` is not available, let `inline_size == 0`
require `std::is_trivially_copyable` (a stronger condition) instead of
`absl::is_trivially_relocatable` (previously this constraint was skipped
in this case). This is relied upon in a new context which is not guarded
with `ABSL_ATTRIBUTE_TRIVIAL_ABI`.

PiperOrigin-RevId: 622825043
  • Loading branch information
QrczakMK committed Apr 8, 2024
1 parent 645e330 commit b2da61e
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 18 deletions.
68 changes: 53 additions & 15 deletions riegeli/base/any_dependency.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <algorithm>
#include <cstddef>
#include <cstring>
#include <memory>
#include <new>
#include <tuple>
Expand Down Expand Up @@ -550,11 +551,21 @@ namespace any_dependency_internal {
template <typename Handle, size_t inline_size, size_t inline_align>
inline AnyDependencyBase<Handle, inline_size, inline_align>::AnyDependencyBase(
AnyDependencyBase&& that) noexcept {
that.methods_and_handle_.handle = SentinelHandle<Handle>();
methods_and_handle_.methods =
std::exchange(that.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.methods->move(repr_.storage, &methods_and_handle_.handle,
that.repr_.storage);
if (inline_size == 0) {
// Replace an indirect call to `methods_and_handle_.methods->move()` with
// a plain assignment of `methods_and_handle_.handle` and `repr_`.
methods_and_handle_.methods =
std::exchange(that.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.handle = std::exchange(that.methods_and_handle_.handle,
SentinelHandle<Handle>());
repr_ = that.repr_;
} else {
that.methods_and_handle_.handle = SentinelHandle<Handle>();
methods_and_handle_.methods =
std::exchange(that.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.methods->move(
repr_.storage, &methods_and_handle_.handle, that.repr_.storage);
}
}

template <typename Handle, size_t inline_size, size_t inline_align>
Expand All @@ -563,11 +574,21 @@ AnyDependencyBase<Handle, inline_size, inline_align>::operator=(
AnyDependencyBase&& that) noexcept {
if (ABSL_PREDICT_TRUE(&that != this)) {
Destroy();
that.methods_and_handle_.handle = SentinelHandle<Handle>();
methods_and_handle_.methods =
std::exchange(that.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.methods->move(
repr_.storage, &methods_and_handle_.handle, that.repr_.storage);
if (inline_size == 0) {
// Replace an indirect call to `methods_and_handle_.methods->move()` with
// a plain assignment of `methods_and_handle_.handle` and `repr_`.
methods_and_handle_.methods = std::exchange(
that.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.handle = std::exchange(
that.methods_and_handle_.handle, SentinelHandle<Handle>());
repr_ = that.repr_;
} else {
that.methods_and_handle_.handle = SentinelHandle<Handle>();
methods_and_handle_.methods = std::exchange(
that.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.methods->move(
repr_.storage, &methods_and_handle_.handle, that.repr_.storage);
}
}
return *this;
}
Expand Down Expand Up @@ -609,11 +630,28 @@ inline void AnyDependencyBase<Handle, inline_size, inline_align>::Initialize(
manager.methods_and_handle_.methods->used_align <=
AvailableAlign<Handle, inline_size, inline_align>())) {
// Adopt `manager` instead of wrapping it.
manager.methods_and_handle_.handle = SentinelHandle<Handle>();
methods_and_handle_.methods = std::exchange(
manager.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.methods->move(
repr_.storage, &methods_and_handle_.handle, manager.repr_.storage);
if (Manager::kAvailableSize == 0 || inline_size == 0) {
// Replace an indirect call to `methods_and_handle_.methods->move()` with
// a plain assignment of `methods_and_handle_.handle` and a memory copy of
// `repr_`.
//
// This would safe whenever
// `manager.methods_and_handle_.methods->used_size == 0`, but this is
// handled specially only if the condition can be determined at compile
// time.
methods_and_handle_.methods = std::exchange(
manager.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.handle = std::exchange(
manager.methods_and_handle_.handle, SentinelHandle<Handle>());
std::memcpy(&repr_, &manager.repr_,
UnsignedMin(sizeof(repr_), sizeof(manager.repr_)));
} else {
manager.methods_and_handle_.handle = SentinelHandle<Handle>();
methods_and_handle_.methods = std::exchange(
manager.methods_and_handle_.methods, &NullMethods::kMethods);
methods_and_handle_.methods->move(
repr_.storage, &methods_and_handle_.handle, manager.repr_.storage);
}
return;
}
methods_and_handle_.methods = &MethodsFor<Manager>::kMethods;
Expand Down
9 changes: 6 additions & 3 deletions riegeli/base/any_dependency_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ using Storage = char[];
//
// If `inline_size == 0`, the dependency is also required to be stable
// (because then `AnyDependency` declares itself stable) and trivially
// relocatable (because then `AnyDependency` declares itself with trivial ABI).
// relocatable (because then `AnyDependency` declares itself with trivial ABI
// and optimizes moving to a plain memory copy of the representation).

// Properties of inline storage in an `AnyDepenency` instance are expressed as
// two numbers: `available_size` and `available_align`, while constraints of a
Expand Down Expand Up @@ -104,9 +105,11 @@ template <typename Handle, typename Manager>
constexpr size_t UsedSize() {
if (sizeof(Dependency<Handle, Manager>) <=
sizeof(Repr<Handle, 0, alignof(Dependency<Handle, Manager>)>) &&
Dependency<Handle, Manager>::kIsStable
Dependency<Handle, Manager>::kIsStable &&
#ifdef ABSL_ATTRIBUTE_TRIVIAL_ABI
&& absl::is_trivially_relocatable<Dependency<Handle, Manager>>::value
absl::is_trivially_relocatable<Dependency<Handle, Manager>>::value
#else
std::is_trivially_copyable<Dependency<Handle, Manager>>::value
#endif
) {
return 0;
Expand Down

0 comments on commit b2da61e

Please sign in to comment.