Skip to content

Commit

Permalink
resize list for trivial types only
Browse files Browse the repository at this point in the history
Summary:
Address comments in D56663331, resize trivially_constructible list might not always be optimum (e.g. for list<shared_ptr>).

Quote from ot
```
imagine elem_type is std::shared_ptr, then it is trivially constructible,
but in order to assign to it you first need to read it back to check if it
is non-empty and needs to be destroyed, which is much more expensive
than constructing in-place.
```

Reviewed By: thedavekwon

Differential Revision: D57161529

fbshipit-source-id: 6a95fe6af7ae112034216b5fca68be9953ff4cef
  • Loading branch information
Jialin Ouyang authored and facebook-github-bot committed May 11, 2024
1 parent 0ab2387 commit 69b2a39
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion thrift/lib/cpp2/op/detail/Encode.h
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ struct Decode<type::list<Tag>> {
apache::thrift::detail::pm::detect_resize,
ListType,
decltype(s)> &&
std::is_trivially_constructible_v<typename ListType::value_type>;
std::is_trivial_v<typename ListType::value_type>;
if constexpr (should_resize) {
list.resize(s);
for (auto&& elem : list) {
Expand Down
2 changes: 1 addition & 1 deletion thrift/lib/cpp2/protocol/detail/protocol_methods.h
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ struct protocol_methods<type_class::list<ElemClass>, Type> {
// resize is more performant than reserve.
constexpr auto should_resize =
folly::is_detected_v<detect_resize, Type, decltype(list_size)> &&
std::is_trivially_constructible_v<elem_type>;
std::is_trivial_v<elem_type>;
if constexpr (should_resize) {
out.resize(list_size);
for (auto&& elem : out) {
Expand Down

0 comments on commit 69b2a39

Please sign in to comment.