Commit 322a5469 authored by Brett Wilson's avatar Brett Wilson Committed by Commit Bot

Add base::circular_deque container.

This container behaves like a std::deque (minus random access insert/erase) but uses
an array as the underlying storage. Additional details are documented in the header.

When DCHECK is enabled, the iterators do extra checks to ensure they are not being
used after container mutations.

A new helper class called base::internal::ArrayBuffer is added to be the low-level storage.
This split was inspired by WTF::Deque although the functionality is quite different: The
added one is mostly a holder for template move and destruct helpers, while the WTF one is
to facilitate copy-on-write and buffer sharing (which is not supported by this one).

The base::void_t is moved from base::internal namespace. This C++17 emulation need not
be strictly internal to base and once I understood how to use it, it seems quite useful.

Added base::internal::is_iterator to detect if an item is an iterator. This is required
by one of the STL deque functions but the exact implementation is not specified by the
standard. This implementation checks for the presence of iterator_category in the
iterator traits.

Contains minor fixes to move_only_int.h testing helper and the addition to a non-movable
copy_only_int.h

Related docs by dskiba:
https://docs.google.com/document/d/1PEuPnSW54LaoWpUIEAHobqGt9nbD2DgQ_W5DdlJOkJU
https://docs.google.com/document/d/1YL1FORFMWo0FK0lMg7WsImnjNQ3ZpY0nK0NHGjkeHT4

Future plans:
 1. Replace some std::deque uses around the code base as a smoketest.
 2. Add a presubmit to prevent additions of std::deque and std::queue.
 3. Replace the remaining uses.

Change-Id: Ic1a5c804da90514782a6eae4984d916da45c0d32
Reviewed-on: https://chromium-review.googlesource.com/582498
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: default avatarVladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491883}
parent 7ba924c0
...@@ -1085,3 +1085,8 @@ template("assert_valid_out_dir") { ...@@ -1085,3 +1085,8 @@ template("assert_valid_out_dir") {
assert_valid_out_dir("_unused") { assert_valid_out_dir("_unused") {
actual_sources = [ "$root_build_dir/foo" ] actual_sources = [ "$root_build_dir/foo" ]
} }
a = "foo"
b = "bar"
c = a + b
print(c)
...@@ -230,6 +230,7 @@ component("base") { ...@@ -230,6 +230,7 @@ component("base") {
"command_line.h", "command_line.h",
"compiler_specific.h", "compiler_specific.h",
"containers/adapters.h", "containers/adapters.h",
"containers/circular_deque.h",
"containers/flat_map.h", "containers/flat_map.h",
"containers/flat_set.h", "containers/flat_set.h",
"containers/flat_tree.h", "containers/flat_tree.h",
...@@ -238,6 +239,7 @@ component("base") { ...@@ -238,6 +239,7 @@ component("base") {
"containers/mru_cache.h", "containers/mru_cache.h",
"containers/small_map.h", "containers/small_map.h",
"containers/stack_container.h", "containers/stack_container.h",
"containers/vector_buffer.h",
"cpu.cc", "cpu.cc",
"cpu.h", "cpu.h",
"critical_closure.h", "critical_closure.h",
...@@ -1977,6 +1979,7 @@ test("base_unittests") { ...@@ -1977,6 +1979,7 @@ test("base_unittests") {
"cancelable_callback_unittest.cc", "cancelable_callback_unittest.cc",
"command_line_unittest.cc", "command_line_unittest.cc",
"containers/adapters_unittest.cc", "containers/adapters_unittest.cc",
"containers/circular_deque_unittest.cc",
"containers/flat_map_unittest.cc", "containers/flat_map_unittest.cc",
"containers/flat_set_unittest.cc", "containers/flat_set_unittest.cc",
"containers/flat_tree_unittest.cc", "containers/flat_tree_unittest.cc",
...@@ -1985,6 +1988,7 @@ test("base_unittests") { ...@@ -1985,6 +1988,7 @@ test("base_unittests") {
"containers/mru_cache_unittest.cc", "containers/mru_cache_unittest.cc",
"containers/small_map_unittest.cc", "containers/small_map_unittest.cc",
"containers/stack_container_unittest.cc", "containers/stack_container_unittest.cc",
"containers/vector_buffer_unittest.cc",
"cpu_unittest.cc", "cpu_unittest.cc",
"debug/activity_analyzer_unittest.cc", "debug/activity_analyzer_unittest.cc",
"debug/activity_tracker_unittest.cc", "debug/activity_tracker_unittest.cc",
......
...@@ -210,6 +210,51 @@ The initial size in the above table is assuming a very small inline table. The ...@@ -210,6 +210,51 @@ The initial size in the above table is assuming a very small inline table. The
actual size will be sizeof(int) + min(sizeof(std::map), sizeof(T) * actual size will be sizeof(int) + min(sizeof(std::map), sizeof(T) *
inline\_size). inline\_size).
# Deque
### Usage advice
Chromium code should always use `base::circular_deque` or `base::queue` in
preference to `std::deque` or `std::queue` due to memory usage and platform
variation.
The `base::deque` implementation (and the `base::queue` which uses it)
provide performance consistent across platforms that better matches most
programmer's expectations on performance (it doesn't waste as much space as
libc++ and doesn't do as many heap allocations as MSVC).
Since `base::deque` does not have stable iterators and it does not support
random-access insert and erase, it may not be appropriate for all uses. If
you need these, consider using a `std::list` which will provide constant
time insert and erase.
### std::deque and std::queue
The implementation of `std::deque` varies considerably which makes it hard to
reason about. All implementations use a sequence of data blocks referenced by
an array of pointers. The standard guarantees random access, amortized
constant operations at the ends, and linear mutations in the middle.
In Microsoft's implementation, each block is 16 bytes or the size of the
contained element. This means in practice that every expansion of the deque
of non-trivial classes requires a heap allocation. libc++ (on Android and Mac)
uses 4K blocks which elimiates the problem of many heap allocations, but
generally wastes a large amount of space (an Android analysis revealed more
than 2.5MB wasted space from deque alone, resulting in some optimizations).
libstdc++ uses an intermediate-size 512 byte buffer.
### base::circular_deque and base::queue
A deque implemented as a circular buffer in an array. The underlying array will
grow like a `std::vector` while the beginning and end of the deque will move
around. The items will wrap around the underlying buffer so the storage will
not be contiguous, but fast random access iterators are still possible.
When the underlying buffer is filled, it will be reallocated and the constents
moved (like a `std::vector`). As a result, iterators are not stable across
mutations. Like a vector, it will not shrink its underlying storage. Consider
calling `shrink_to_fit` if you delete many items that you don't plan to re-add.
## Appendix ## Appendix
### Code for map code size comparison ### Code for map code size comparison
......
This diff is collapsed.
This diff is collapsed.
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef BASE_CONTAINERS_QUEUE_H_
#define BASE_CONTAINERS_QUEUE_H_
#include <queue>
#include "base/containers/circular_deque.h"
namespace base {
// Provides a definition of base::queue that's like std::queue but uses a
// base::circular_queue instead. Since std::queue is just a wraper for an
// underlying type, we can just provide a typedef for it that defaults to the
// base circular_deque.
template <class T, class Container = circular_deque<T>>
using queue = std::queue<T, Container>;
} // namespace base
#endif // BASE_CONTAINERS_QUEUE_H_
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef BASE_CONTAINERS_VECTOR_BUFFERS_H_
#define BASE_CONTAINERS_VECTOR_BUFFERS_H_
#include <stdlib.h>
#include <string.h>
#include <type_traits>
#include <utility>
#include "base/logging.h"
#include "base/macros.h"
namespace base {
namespace internal {
// Internal implementation detail of base/containers.
//
// Implements a vector-like buffer that holds a certain capacity of T. Unlike
// std::vector, VectorBuffer never constructs or destructs its arguments, and
// can't change sizes. But it does implement templates to assist in efficient
// moving and destruction of those items manually.
//
// In particular, the destructor function does not iterate over the items if
// there is no destructor. Moves should be implemented as a memcpy/memmove for
// trivially copyable objects (POD) otherwise, it should be a std::move if
// possible, and as a last resort it falls back to a copy. This behavior is
// similar to std::vector.
//
// No special consideration is done for noexcept move constructors since
// we compile without exceptions.
//
// The current API does not support moving overlapping ranges.
template <typename T>
class VectorBuffer {
public:
VectorBuffer() {}
VectorBuffer(size_t count)
: buffer_(reinterpret_cast<T*>(malloc(sizeof(T) * count))),
capacity_(count) {}
VectorBuffer(VectorBuffer&& other) noexcept
: buffer_(other.buffer_), capacity_(other.capacity_) {
other.buffer_ = nullptr;
other.capacity_ = 0;
}
~VectorBuffer() { free(buffer_); }
VectorBuffer& operator=(VectorBuffer&& other) {
free(buffer_);
buffer_ = other.buffer_;
capacity_ = other.capacity_;
other.buffer_ = nullptr;
other.capacity_ = 0;
return *this;
}
size_t capacity() const { return capacity_; }
T& operator[](size_t i) { return buffer_[i]; }
const T& operator[](size_t i) const { return buffer_[i]; }
T* begin() { return buffer_; }
T* end() { return &buffer_[capacity_]; }
// DestructRange ------------------------------------------------------------
// Trivially destructible objects need not have their destructors called.
template <typename T2 = T,
typename std::enable_if<std::is_trivially_destructible<T2>::value,
int>::type = 0>
void DestructRange(T* begin, T* end) {}
// Non-trivially destructible objects must have their destructors called
// individually.
template <typename T2 = T,
typename std::enable_if<!std::is_trivially_destructible<T2>::value,
int>::type = 0>
void DestructRange(T* begin, T* end) {
while (begin != end) {
begin->~T();
begin++;
}
}
// MoveRange ----------------------------------------------------------------
//
// The destructor will be called (as necessary) for all moved types. The
// ranges must not overlap.
//
// The parameters and begin and end (one past the last) of the input buffer,
// and the address of the first element to copy to. There must be sufficient
// room in the destination for all items in the range [begin, end).
// Trivially copyable types can use memcpy. trivially copyable implies
// that there is a trivial destructor as we don't have to call it.
template <typename T2 = T,
typename std::enable_if<base::is_trivially_copyable<T2>::value,
int>::type = 0>
static void MoveRange(T* from_begin, T* from_end, T* to) {
DCHECK(!RangesOverlap(from_begin, from_end, to));
memcpy(to, from_begin, (from_end - from_begin) * sizeof(T));
}
// Not trivially copyable, but movable: call the move constructor and
// destruct the original.
template <typename T2 = T,
typename std::enable_if<std::is_move_constructible<T2>::value &&
!base::is_trivially_copyable<T2>::value,
int>::type = 0>
static void MoveRange(T* from_begin, T* from_end, T* to) {
DCHECK(!RangesOverlap(from_begin, from_end, to));
while (from_begin != from_end) {
new (to) T(std::move(*from_begin));
from_begin->~T();
from_begin++;
to++;
}
}
// Not movable, not trivially copyable: call the copy constructor and
// destruct the original.
template <typename T2 = T,
typename std::enable_if<!std::is_move_constructible<T2>::value &&
!base::is_trivially_copyable<T2>::value,
int>::type = 0>
static void MoveRange(T* from_begin, T* from_end, T* to) {
DCHECK(!RangesOverlap(from_begin, from_end, to));
while (from_begin != from_end) {
new (to) T(*from_begin);
from_begin->~T();
from_begin++;
to++;
}
}
private:
static bool RangesOverlap(const T* from_begin,
const T* from_end,
const T* to) {
return !(to >= from_end || to + (from_end - from_begin) <= from_begin);
}
T* buffer_ = nullptr;
size_t capacity_ = 0;
DISALLOW_COPY_AND_ASSIGN(VectorBuffer);
};
} // namespace internal
} // namespace base
#endif // BASE_CONTAINERS_VECTOR_BUFFERS_H_
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/containers/vector_buffer.h"
#include "base/test/copy_only_int.h"
#include "base/test/move_only_int.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace base {
namespace internal {
TEST(VectorBuffer, DeletePOD) {
constexpr int size = 10;
VectorBuffer<int> buffer(size);
for (int i = 0; i < size; i++)
buffer.begin()[i] = i + 1;
buffer.DestructRange(buffer.begin(), buffer.end());
// Delete should do nothing.
for (int i = 0; i < size; i++)
EXPECT_EQ(i + 1, buffer.begin()[i]);
}
TEST(VectorBuffer, DeleteMoveOnly) {
constexpr int size = 10;
VectorBuffer<MoveOnlyInt> buffer(size);
for (int i = 0; i < size; i++)
new (buffer.begin() + i) MoveOnlyInt(i + 1);
buffer.DestructRange(buffer.begin(), buffer.end());
// Delete should have reset all of the values to 0.
for (int i = 0; i < size; i++)
EXPECT_EQ(0, buffer.begin()[i].data());
}
TEST(VectorBuffer, PODMove) {
constexpr int size = 10;
VectorBuffer<int> dest(size);
VectorBuffer<int> original(size);
for (int i = 0; i < size; i++)
original.begin()[i] = i + 1;
original.MoveRange(original.begin(), original.end(), dest.begin());
for (int i = 0; i < size; i++)
EXPECT_EQ(i + 1, dest.begin()[i]);
}
TEST(VectorBuffer, MovableMove) {
constexpr int size = 10;
VectorBuffer<MoveOnlyInt> dest(size);
VectorBuffer<MoveOnlyInt> original(size);
for (int i = 0; i < size; i++)
new (original.begin() + i) MoveOnlyInt(i + 1);
original.MoveRange(original.begin(), original.end(), dest.begin());
// Moving from a MoveOnlyInt resets to 0.
for (int i = 0; i < size; i++) {
EXPECT_EQ(0, original.begin()[i].data());
EXPECT_EQ(i + 1, dest.begin()[i].data());
}
}
TEST(VectorBuffer, CopyToMove) {
constexpr int size = 10;
VectorBuffer<CopyOnlyInt> dest(size);
VectorBuffer<CopyOnlyInt> original(size);
for (int i = 0; i < size; i++)
new (original.begin() + i) CopyOnlyInt(i + 1);
original.MoveRange(original.begin(), original.end(), dest.begin());
// The original should have been destructed, which should reset the value to
// 0. Technically this dereferences the destructed object.
for (int i = 0; i < size; i++) {
EXPECT_EQ(0, original.begin()[i].data());
EXPECT_EQ(i + 1, dest.begin()[i].data());
}
}
} // namespace internal
} // namespace base
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <stddef.h> #include <stddef.h>
#include <iosfwd> #include <iosfwd>
#include <iterator>
#include <type_traits> #include <type_traits>
#include <utility> #include <utility>
...@@ -42,18 +43,24 @@ template <class T> struct is_non_const_reference<const T&> : std::false_type {}; ...@@ -42,18 +43,24 @@ template <class T> struct is_non_const_reference<const T&> : std::false_type {};
namespace internal { namespace internal {
// Implementation detail of base::void_t below.
template <typename...> template <typename...>
struct make_void { struct make_void {
using type = void; using type = void;
}; };
// A clone of C++17 std::void_t. } // namespace internal
// Unlike the original version, we need |make_void| as a helper struct to avoid
// a C++14 defect. // base::void_t is an implementation of std::void_t from C++17.
// ref: http://en.cppreference.com/w/cpp/types/void_t //
// ref: http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#1558 // We use |base::internal::make_void| as a helper struct to avoid a C++14
// defect:
// http://en.cppreference.com/w/cpp/types/void_t
// http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#1558
template <typename... Ts> template <typename... Ts>
using void_t = typename make_void<Ts...>::type; using void_t = typename ::base::internal::make_void<Ts...>::type;
namespace internal {
// Uses expression SFINAE to detect whether using operator<< would work. // Uses expression SFINAE to detect whether using operator<< would work.
template <typename T, typename = void> template <typename T, typename = void>
...@@ -64,6 +71,17 @@ struct SupportsOstreamOperator<T, ...@@ -64,6 +71,17 @@ struct SupportsOstreamOperator<T,
<< std::declval<T>()))> << std::declval<T>()))>
: std::true_type {}; : std::true_type {};
// Used to detech whether the given type is an iterator. This is normally used
// with std::enable_if to provide disambiguation for functions that take
// templatzed iterators as input.
template <typename T, typename = void>
struct is_iterator : std::false_type {};
template <typename T>
struct is_iterator<T,
void_t<typename std::iterator_traits<T>::iterator_category>>
: std::true_type {};
} // namespace internal } // namespace internal
// is_trivially_copyable is especially hard to get right. // is_trivially_copyable is especially hard to get right.
......
...@@ -30,6 +30,7 @@ static_library("test_support") { ...@@ -30,6 +30,7 @@ static_library("test_support") {
"android/java_handler_thread_for_testing.h", "android/java_handler_thread_for_testing.h",
"android/test_system_message_handler_link_android.cc", "android/test_system_message_handler_link_android.cc",
"android/test_system_message_handler_link_android.h", "android/test_system_message_handler_link_android.h",
"copy_only_int.h",
"fuzzed_data_provider.cc", "fuzzed_data_provider.cc",
"fuzzed_data_provider.h", "fuzzed_data_provider.h",
"gtest_util.cc", "gtest_util.cc",
...@@ -57,6 +58,7 @@ static_library("test_support") { ...@@ -57,6 +58,7 @@ static_library("test_support") {
"mock_entropy_provider.h", "mock_entropy_provider.h",
"mock_log.cc", "mock_log.cc",
"mock_log.h", "mock_log.h",
"move_only_int.h",
"multiprocess_test.cc", "multiprocess_test.cc",
"multiprocess_test.h", "multiprocess_test.h",
"multiprocess_test_android.cc", "multiprocess_test_android.cc",
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef BASE_TEST_COPY_ONLY_INT_H_
#define BASE_TEST_COPY_ONLY_INT_H_
#include "base/macros.h"
namespace base {
// A copy-only (not moveable) class that holds an integer. This is designed for
// testing containers. See also MoveOnlyInt.
class CopyOnlyInt {
public:
explicit CopyOnlyInt(int data = 1) : data_(data) {}
CopyOnlyInt(const CopyOnlyInt& other) : data_(other.data_) {}
~CopyOnlyInt() { data_ = 0; }
friend bool operator==(const CopyOnlyInt& lhs, const CopyOnlyInt& rhs) {
return lhs.data_ == rhs.data_;
}
friend bool operator!=(const CopyOnlyInt& lhs, const CopyOnlyInt& rhs) {
return !operator==(lhs, rhs);
}
friend bool operator<(const CopyOnlyInt& lhs, const CopyOnlyInt& rhs) {
return lhs.data_ < rhs.data_;
}
friend bool operator>(const CopyOnlyInt& lhs, const CopyOnlyInt& rhs) {
return rhs < lhs;
}
friend bool operator<=(const CopyOnlyInt& lhs, const CopyOnlyInt& rhs) {
return !(rhs < lhs);
}
friend bool operator>=(const CopyOnlyInt& lhs, const CopyOnlyInt& rhs) {
return !(lhs < rhs);
}
int data() const { return data_; }
private:
int data_;
CopyOnlyInt(CopyOnlyInt&&) = delete;
CopyOnlyInt& operator=(CopyOnlyInt&) = delete;
};
} // namespace base
#endif // BASE_TEST_COPY_ONLY_INT_H_
...@@ -2,20 +2,21 @@ ...@@ -2,20 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#ifndef BASE_CONTAINERS_CONTAINER_TEST_UTILS_H_ #ifndef BASE_TEST_MOVE_ONLY_INT_H_
#define BASE_CONTAINERS_CONTAINER_TEST_UTILS_H_ #define BASE_TEST_MOVE_ONLY_INT_H_
// This file contains some helper classes for testing conainer behavior.
#include "base/macros.h" #include "base/macros.h"
namespace base { namespace base {
// A move-only class that holds an integer. // A move-only class that holds an integer. This is designed for testing
// containers. See also CopyOnlyInt.
class MoveOnlyInt { class MoveOnlyInt {
public: public:
explicit MoveOnlyInt(int data = 1) : data_(data) {} explicit MoveOnlyInt(int data = 1) : data_(data) {}
MoveOnlyInt(MoveOnlyInt&& other) : data_(other.data_) { other.data_ = 0; } MoveOnlyInt(MoveOnlyInt&& other) : data_(other.data_) { other.data_ = 0; }
~MoveOnlyInt() { data_ = 0; }
MoveOnlyInt& operator=(MoveOnlyInt&& other) { MoveOnlyInt& operator=(MoveOnlyInt&& other) {
data_ = other.data_; data_ = other.data_;
other.data_ = 0; other.data_ = 0;
...@@ -64,4 +65,4 @@ class MoveOnlyInt { ...@@ -64,4 +65,4 @@ class MoveOnlyInt {
} // namespace base } // namespace base
#endif // BASE_CONTAINERS_CONTAINER_TEST_UTILS_H_ #endif // BASE_TEST_MOVE_ONLY_INT_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment