Skip to content

Commit

Permalink
Use folly::IsRelocatable in folly::small_vector (#1934)
Browse files Browse the repository at this point in the history
Summary:
The `relocateInlineTrivial` operation here is exactly "move-construct plus destroy," which is the same operation optimized into memcpy by `FBVector::make_window` and `FBVector::reserve`. So we can optimize it into memcpy here for the same set of types, namely, trivially relocatable types.

The existing `kShouldCopyInlineTrivial` is also used in the copy constructor, where we're doing "copy-construct, do not destroy." I believe the place I changed in this patch is the only place where we can use true relocation.

FWIW, I don't understand why this optimization can be disabled depending on the value of `hardware_constructive_interference_size`; it seems to me that no matter what your cache line size is, memcpy will always be superior to any non-trivial `std::copy`. Either both approaches will hit cache effects (and memcpy will be faster), or neither approach will hit cache effects (and memcpy will be faster). But that's tangential to this patch's purpose, so I didn't mess with it.

Pull Request resolved: #1934

Reviewed By: Gownta

Differential Revision: D43401609

Pulled By: Orvid

fbshipit-source-id: 7aa1fd922b0e60b9351fba96449e2ad38745bddf
  • Loading branch information
Quuxplusone authored and facebook-github-bot committed Feb 14, 2024
1 parent 0e81e77 commit b6762ac
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions folly/small_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,11 @@ class small_vector
this->u.setCapacity(o.u.getCapacity());
}
} else {
if (kShouldCopyInlineTrivial) {
copyInlineTrivial<Value>(o);
if constexpr (IsRelocatable<Value>::value) {
// Copy the entire inline storage, instead of just size() values, to
// make the loop fixed-size and unrollable.
std::memcpy(u.buffer(), o.u.buffer(), MaxInline * kSizeOfValue);
this->setSize(o.size());
o.resetSizePolicy();
} else {
auto n = o.size();
Expand Down

0 comments on commit b6762ac

Please sign in to comment.