Skip to content

Commit

Permalink
[base] Specialize __unwrap_iter for CheckedContiguousIterator
Browse files Browse the repository at this point in the history
This change specializes libc++'s __unwrap_iter for base's
CheckedContiguousIterator. This results in speeding up STL
algorithms like std::copy, which now dispatches to a simple
memmove if possible.

Bug: 994174

Change-Id: I4970a7cb420a875a6f323f218448d305ab81d44b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875734
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762698}
  • Loading branch information
jdoerrie authored and Commit Bot committed Apr 26, 2020
1 parent e2362fb commit cbec416
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 3 deletions.
38 changes: 38 additions & 0 deletions base/containers/checked_iterators.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@

namespace base {

#if defined(_LIBCPP_VERSION)
// SFINAE friendly type trait that only enables an overload if the passed in
// type is trivially copy assignable, so that it would be safe to implement
// std::copy as a memmove, as libc++ does for some iterator types.
namespace internal {
template <typename T>
using PointerIfIsTriviallyCopyAssignable = std::enable_if_t<
std::is_trivially_copy_assignable<std::remove_const_t<T>>::value,
T*>;
} // namespace internal
#endif

template <typename T>
class CheckedContiguousIterator {
public:
Expand All @@ -28,6 +40,21 @@ class CheckedContiguousIterator {
friend class CheckedContiguousIterator;

constexpr CheckedContiguousIterator() = default;

#if defined(_LIBCPP_VERSION)
// This constructor is required to be able to use a CheckedContiguousIterator
// as the third argument for the optimized pointer based std::copy version in
// libc++. Do not use this constructor otherwise.
constexpr CheckedContiguousIterator(T* ptr)
: start_(ptr), current_(ptr), end_(ptr) {}

// Friend __unwrap_iter so that it can get unchecked access to the underlying
// pointer.
template <typename U>
friend constexpr internal::PointerIfIsTriviallyCopyAssignable<U>
__unwrap_iter(CheckedContiguousIterator<U> iter);
#endif

constexpr CheckedContiguousIterator(T* start, const T* end)
: CheckedContiguousIterator(start, start, end) {}
constexpr CheckedContiguousIterator(const T* start, T* current, const T* end)
Expand Down Expand Up @@ -204,6 +231,17 @@ class CheckedContiguousIterator {
template <typename T>
using CheckedContiguousConstIterator = CheckedContiguousIterator<const T>;

#if defined(_LIBCPP_VERSION)
// This is a non-portable overload of libc++'s __unwrap_iter. This allows
// treating a CheckedContiguousIterator as a raw pointer within STL algorithms
// enabling performance optimizations that wouldn't be possible otherwise.
template <typename T>
constexpr internal::PointerIfIsTriviallyCopyAssignable<T> __unwrap_iter(
CheckedContiguousIterator<T> iter) {
return iter.current_;
}
#endif

} // namespace base

#endif // BASE_CONTAINERS_CHECKED_ITERATORS_H_
79 changes: 79 additions & 0 deletions base/containers/checked_iterators_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#include "base/containers/checked_iterators.h"

#include <algorithm>
#include <iterator>

#include "testing/gtest/include/gtest/gtest.h"

namespace base {
Expand Down Expand Up @@ -77,4 +80,80 @@ TEST(CheckedContiguousIterator, ConvertingComparisonOperators) {
EXPECT_GE(cbegin, begin);
}

#if defined(_LIBCPP_VERSION)
namespace {

// Helper template that wraps an iterator and disables its dereference and
// increment operations.
template <typename Iterator>
struct DisableDerefAndIncr : Iterator {
using Iterator::Iterator;

void operator*() = delete;
void operator++() = delete;
void operator++(int) = delete;
};

template <typename Iterator>
auto __unwrap_iter(DisableDerefAndIncr<Iterator> iter) {
return __unwrap_iter(static_cast<Iterator>(iter));
}

} // namespace

// Tests that using std::copy with CheckedContiguousIterator<int> results in an
// optimized code-path that does not invoke the iterator's dereference and
// increment operations. This would fail to compile if std::copy was not
// optimized.
TEST(CheckedContiguousIterator, OptimizedCopy) {
using Iter = DisableDerefAndIncr<CheckedContiguousIterator<int>>;
static_assert(std::is_same<int*, decltype(__unwrap_iter(Iter()))>::value,
"Error: Iter should unwrap to int*");

int arr_in[5] = {1, 2, 3, 4, 5};
int arr_out[5];

Iter begin(std::begin(arr_in), std::end(arr_in));
Iter end(std::begin(arr_in), std::end(arr_in), std::end(arr_in));
std::copy(begin, end, arr_out);

EXPECT_TRUE(std::equal(std::begin(arr_in), std::end(arr_in),
std::begin(arr_out), std::end(arr_out)));
}

TEST(CheckedContiguousIterator, UnwrapIter) {
static_assert(
std::is_same<int*, decltype(__unwrap_iter(
CheckedContiguousIterator<int>()))>::value,
"Error: CCI<int> should unwrap to int*");

static_assert(
std::is_same<CheckedContiguousIterator<std::string>,
decltype(__unwrap_iter(
CheckedContiguousIterator<std::string>()))>::value,
"Error: CCI<std::string> should unwrap to CCI<std::string>");
}

// While the result of std::copying into a range via a CCI can't be
// compared to other iterators, it should be possible to re-use it in another
// std::copy expresson.
TEST(CheckedContiguousIterator, ReuseCopyIter) {
using Iter = CheckedContiguousIterator<int>;

int arr_in[5] = {1, 2, 3, 4, 5};
int arr_out[5];

Iter begin(std::begin(arr_in), std::end(arr_in));
Iter end(std::begin(arr_in), std::end(arr_in), std::end(arr_in));
Iter out_begin(std::begin(arr_out), std::end(arr_out));

auto out_middle = std::copy_n(begin, 3, out_begin);
std::copy(begin + 3, end, out_middle);

EXPECT_TRUE(std::equal(std::begin(arr_in), std::end(arr_in),
std::begin(arr_out), std::end(arr_out)));
}

#endif

} // namespace base
5 changes: 2 additions & 3 deletions device/fido/cable/v2_handshake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ bool ConstructNonce(uint32_t counter, base::span<uint8_t, 12> out_nonce) {
// Nonce is just a little-endian counter.
std::array<uint8_t, sizeof(counter)> counter_bytes;
memcpy(counter_bytes.data(), &counter, sizeof(counter));
auto remaining =
std::copy(counter_bytes.begin(), counter_bytes.end(), out_nonce.begin());
std::fill(remaining, out_nonce.end(), 0);
std::copy(counter_bytes.begin(), counter_bytes.end(), out_nonce.begin());
std::fill(out_nonce.begin() + counter_bytes.size(), out_nonce.end(), 0);
return true;
}

Expand Down

0 comments on commit cbec416

Please sign in to comment.