Skip to content

Commit

Permalink
Export of internal Abseil changes
Browse files Browse the repository at this point in the history
--
730bb88bee556aa11fa19aa33e1434cb6fa78985 by Evan Brown <ezb@google.com>:

Support missing allocator-related constructors in b-tree. See [reference](https://en.cppreference.com/w/cpp/container/set/set).

Also use allocator_traits::select_on_container_copy_construction() to get allocator for copy construction.

PiperOrigin-RevId: 339058322

--
b6cc121689ae3e452d1db2d66122cb198d25142b by Derek Mauro <dmauro@google.com>:

Fix more sign-compare warnings

PiperOrigin-RevId: 339057920

--
0e2c62da1dcaf6529abab952bdcc96c6de2d9506 by Abseil Team <absl-team@google.com>:

Add missing <limits> include

PiperOrigin-RevId: 339054753

--
d5a9ec2d1e40fe6359e720942e4955009ee415ec by Derek Mauro <dmauro@google.com>:

Stop disabling sign-compare warnings for non-test targets.
Our users complain about these.

This does not catch issues in header-only libraries (like btree.h)
but we may work on those in the future

PiperOrigin-RevId: 338967089

--
0c062c542a4c61ea0f65d25811827c0858e3adde by Abseil Team <absl-team@google.com>:

Improve cache-locality for ThreadIdentity and PerThreadSynch.

This is a change based on an observation in RPC benchmarks that shows
significant cycles being spent in waking up a thread, 99.8% of which
was on cache misses. Investigating this a bit more, it turns out to
be due to sharing the cache line with the waiter state.

To fix this issue, the following changes are introduced:
- Reorder fields in PerThreadSync so that it fits in a single cache line
  The size of this structure was 80 bytes before this change.
  Note: Manually inspected all booleans to make sure they are not modified by
        multiple threads concurrently.

PiperOrigin-RevId: 338852058

--
a90d6f2b2346385017e32dd8ae1b5ca691a5863f by Derek Mauro <dmauro@google.com>:

Delete GCC 4.9 test script. It is no longer supported

PiperOrigin-RevId: 338779452

--
7274008d4757e88869110be9db39d03d911ae2b5 by Abseil Team <absl-team@google.com>:

Fix the usage example in which SetFlag should take a pointer.

PiperOrigin-RevId: 338744529
GitOrigin-RevId: 730bb88bee556aa11fa19aa33e1434cb6fa78985
Change-Id: Iff99594c4022e60e482a392d334b376c7ae8883e
  • Loading branch information
Abseil Team authored and vslashg committed Oct 26, 2020
1 parent 1e3d25b commit 5bf048b
Show file tree
Hide file tree
Showing 15 changed files with 203 additions and 179 deletions.
79 changes: 39 additions & 40 deletions absl/base/internal/thread_identity.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include "absl/base/config.h"
#include "absl/base/internal/per_thread_tls.h"
#include "absl/base/optimization.h"

namespace absl {
ABSL_NAMESPACE_BEGIN
Expand Down Expand Up @@ -69,30 +70,28 @@ struct PerThreadSynch {
// is using this PerThreadSynch as a terminator. Its
// skip field must not be filled in because the loop
// might then skip over the terminator.

// The wait parameters of the current wait. waitp is null if the
// thread is not waiting. Transitions from null to non-null must
// occur before the enqueue commit point (state = kQueued in
// Enqueue() and CondVarEnqueue()). Transitions from non-null to
// null must occur after the wait is finished (state = kAvailable in
// Mutex::Block() and CondVar::WaitCommon()). This field may be
// changed only by the thread that describes this PerThreadSynch. A
// special case is Fer(), which calls Enqueue() on another thread,
// but with an identical SynchWaitParams pointer, thus leaving the
// pointer unchanged.
SynchWaitParams *waitp;

bool suppress_fatal_errors; // If true, try to proceed even in the face of
// broken invariants. This is used within fatal
// signal handlers to improve the chances of
// debug logging information being output
// successfully.

intptr_t readers; // Number of readers in mutex.
int priority; // Priority of thread (updated every so often).

// When priority will next be read (cycles).
int64_t next_priority_read_cycles;
bool wake; // This thread is to be woken from a Mutex.
// If "x" is on a waiter list for a mutex, "x->cond_waiter" is true iff the
// waiter is waiting on the mutex as part of a CV Wait or Mutex Await.
//
// The value of "x->cond_waiter" is meaningless if "x" is not on a
// Mutex waiter list.
bool cond_waiter;
bool maybe_unlocking; // Valid at head of Mutex waiter queue;
// true if UnlockSlow could be searching
// for a waiter to wake. Used for an optimization
// in Enqueue(). true is always a valid value.
// Can be reset to false when the unlocker or any
// writer releases the lock, or a reader fully
// releases the lock. It may not be set to false
// by a reader that decrements the count to
// non-zero. protected by mutex spinlock
bool suppress_fatal_errors; // If true, try to proceed even in the face
// of broken invariants. This is used within
// fatal signal handlers to improve the
// chances of debug logging information being
// output successfully.
int priority; // Priority of thread (updated every so often).

// State values:
// kAvailable: This PerThreadSynch is available.
Expand All @@ -111,30 +110,30 @@ struct PerThreadSynch {
};
std::atomic<State> state;

bool maybe_unlocking; // Valid at head of Mutex waiter queue;
// true if UnlockSlow could be searching
// for a waiter to wake. Used for an optimization
// in Enqueue(). true is always a valid value.
// Can be reset to false when the unlocker or any
// writer releases the lock, or a reader fully releases
// the lock. It may not be set to false by a reader
// that decrements the count to non-zero.
// protected by mutex spinlock
// The wait parameters of the current wait. waitp is null if the
// thread is not waiting. Transitions from null to non-null must
// occur before the enqueue commit point (state = kQueued in
// Enqueue() and CondVarEnqueue()). Transitions from non-null to
// null must occur after the wait is finished (state = kAvailable in
// Mutex::Block() and CondVar::WaitCommon()). This field may be
// changed only by the thread that describes this PerThreadSynch. A
// special case is Fer(), which calls Enqueue() on another thread,
// but with an identical SynchWaitParams pointer, thus leaving the
// pointer unchanged.
SynchWaitParams* waitp;

bool wake; // This thread is to be woken from a Mutex.
intptr_t readers; // Number of readers in mutex.

// If "x" is on a waiter list for a mutex, "x->cond_waiter" is true iff the
// waiter is waiting on the mutex as part of a CV Wait or Mutex Await.
//
// The value of "x->cond_waiter" is meaningless if "x" is not on a
// Mutex waiter list.
bool cond_waiter;
// When priority will next be read (cycles).
int64_t next_priority_read_cycles;

// Locks held; used during deadlock detection.
// Allocated in Synch_GetAllLocks() and freed in ReclaimThreadIdentity().
SynchLocksHeld *all_locks;
};

// The instances of this class are allocated in NewThreadIdentity() with an
// alignment of PerThreadSynch::kAlignment.
struct ThreadIdentity {
// Must be the first member. The Mutex implementation requires that
// the PerThreadSynch object associated with each thread is
Expand Down
95 changes: 95 additions & 0 deletions absl/container/btree_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2709,6 +2709,101 @@ TEST(Btree, MultiKeyEqualRange) {
}
}

TEST(Btree, AllocConstructor) {
using Alloc = CountingAllocator<int>;
using Set = absl::btree_set<int, std::less<int>, Alloc>;
int64_t bytes_used = 0;
Alloc alloc(&bytes_used);
Set set(alloc);

set.insert({1, 2, 3});

EXPECT_THAT(set, ElementsAre(1, 2, 3));
EXPECT_GT(bytes_used, set.size() * sizeof(int));
}

TEST(Btree, AllocInitializerListConstructor) {
using Alloc = CountingAllocator<int>;
using Set = absl::btree_set<int, std::less<int>, Alloc>;
int64_t bytes_used = 0;
Alloc alloc(&bytes_used);
Set set({1, 2, 3}, alloc);

EXPECT_THAT(set, ElementsAre(1, 2, 3));
EXPECT_GT(bytes_used, set.size() * sizeof(int));
}

TEST(Btree, AllocRangeConstructor) {
using Alloc = CountingAllocator<int>;
using Set = absl::btree_set<int, std::less<int>, Alloc>;
int64_t bytes_used = 0;
Alloc alloc(&bytes_used);
std::vector<int> v = {1, 2, 3};
Set set(v.begin(), v.end(), alloc);

EXPECT_THAT(set, ElementsAre(1, 2, 3));
EXPECT_GT(bytes_used, set.size() * sizeof(int));
}

TEST(Btree, AllocCopyConstructor) {
using Alloc = CountingAllocator<int>;
using Set = absl::btree_set<int, std::less<int>, Alloc>;
int64_t bytes_used1 = 0;
Alloc alloc1(&bytes_used1);
Set set1(alloc1);

set1.insert({1, 2, 3});

int64_t bytes_used2 = 0;
Alloc alloc2(&bytes_used2);
Set set2(set1, alloc2);

EXPECT_THAT(set1, ElementsAre(1, 2, 3));
EXPECT_THAT(set2, ElementsAre(1, 2, 3));
EXPECT_GT(bytes_used1, set1.size() * sizeof(int));
EXPECT_EQ(bytes_used1, bytes_used2);
}

TEST(Btree, AllocMoveConstructor_SameAlloc) {
using Alloc = CountingAllocator<int>;
using Set = absl::btree_set<int, std::less<int>, Alloc>;
int64_t bytes_used = 0;
Alloc alloc(&bytes_used);
Set set1(alloc);

set1.insert({1, 2, 3});

const int64_t original_bytes_used = bytes_used;
EXPECT_GT(original_bytes_used, set1.size() * sizeof(int));

Set set2(std::move(set1), alloc);

EXPECT_THAT(set2, ElementsAre(1, 2, 3));
EXPECT_EQ(bytes_used, original_bytes_used);
}

TEST(Btree, AllocMoveConstructor_DifferentAlloc) {
using Alloc = CountingAllocator<int>;
using Set = absl::btree_set<int, std::less<int>, Alloc>;
int64_t bytes_used1 = 0;
Alloc alloc1(&bytes_used1);
Set set1(alloc1);

set1.insert({1, 2, 3});

const int64_t original_bytes_used = bytes_used1;
EXPECT_GT(original_bytes_used, set1.size() * sizeof(int));

int64_t bytes_used2 = 0;
Alloc alloc2(&bytes_used2);
Set set2(std::move(set1), alloc2);

EXPECT_THAT(set2, ElementsAre(1, 2, 3));
// We didn't free these bytes allocated by `set1` yet.
EXPECT_EQ(bytes_used1, original_bytes_used);
EXPECT_EQ(bytes_used2, original_bytes_used);
}

} // namespace
} // namespace container_internal
ABSL_NAMESPACE_END
Expand Down
42 changes: 23 additions & 19 deletions absl/container/internal/btree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1141,21 +1141,35 @@ class btree {
// before this method is called. This method is used in copy construction,
// copy assignment, and move assignment.
template <typename Btree>
void copy_or_move_values_in_order(Btree *other);
void copy_or_move_values_in_order(Btree &other);

// Validates that various assumptions/requirements are true at compile time.
constexpr static bool static_assert_validation();

public:
btree(const key_compare &comp, const allocator_type &alloc);
btree(const key_compare &comp, const allocator_type &alloc)
: root_(comp, alloc, EmptyNode()), rightmost_(EmptyNode()), size_(0) {}

btree(const btree &other);
btree(const btree &other) : btree(other, other.allocator()) {}
btree(const btree &other, const allocator_type &alloc)
: btree(other.key_comp(), alloc) {
copy_or_move_values_in_order(other);
}
btree(btree &&other) noexcept
: root_(std::move(other.root_)),
rightmost_(absl::exchange(other.rightmost_, EmptyNode())),
size_(absl::exchange(other.size_, 0)) {
other.mutable_root() = EmptyNode();
}
btree(btree &&other, const allocator_type &alloc)
: btree(other.key_comp(), alloc) {
if (alloc == other.allocator()) {
swap(other);
} else {
// Move values from `other` one at a time when allocators are different.
copy_or_move_values_in_order(other);
}
}

~btree() {
// Put static_asserts in destructor to avoid triggering them before the type
Expand Down Expand Up @@ -1851,19 +1865,19 @@ void btree_iterator<N, R, P>::decrement_slow() {
// btree methods
template <typename P>
template <typename Btree>
void btree<P>::copy_or_move_values_in_order(Btree *other) {
void btree<P>::copy_or_move_values_in_order(Btree &other) {
static_assert(std::is_same<btree, Btree>::value ||
std::is_same<const btree, Btree>::value,
"Btree type must be same or const.");
assert(empty());

// We can avoid key comparisons because we know the order of the
// values is the same order we'll store them in.
auto iter = other->begin();
if (iter == other->end()) return;
auto iter = other.begin();
if (iter == other.end()) return;
insert_multi(maybe_move_from_iterator(iter));
++iter;
for (; iter != other->end(); ++iter) {
for (; iter != other.end(); ++iter) {
// If the btree is not empty, we can just insert the new value at the end
// of the tree.
internal_emplace(end(), maybe_move_from_iterator(iter));
Expand Down Expand Up @@ -1901,16 +1915,6 @@ constexpr bool btree<P>::static_assert_validation() {
return true;
}

template <typename P>
btree<P>::btree(const key_compare &comp, const allocator_type &alloc)
: root_(comp, alloc, EmptyNode()), rightmost_(EmptyNode()), size_(0) {}

template <typename P>
btree<P>::btree(const btree &other)
: btree(other.key_comp(), other.allocator()) {
copy_or_move_values_in_order(&other);
}

template <typename P>
template <typename K>
auto btree<P>::equal_range(const K &key) -> std::pair<iterator, iterator> {
Expand Down Expand Up @@ -2068,7 +2072,7 @@ auto btree<P>::operator=(const btree &other) -> btree & {
*mutable_allocator() = other.allocator();
}

copy_or_move_values_in_order(&other);
copy_or_move_values_in_order(other);
}
return *this;
}
Expand Down Expand Up @@ -2098,7 +2102,7 @@ auto btree<P>::operator=(btree &&other) noexcept -> btree & {
// comparator while moving the values so we can't swap the key
// comparators.
*mutable_key_comp() = other.key_comp();
copy_or_move_values_in_order(&other);
copy_or_move_values_in_order(other);
}
}
}
Expand Down
40 changes: 34 additions & 6 deletions absl/container/internal/btree_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "absl/base/internal/throw_delegate.h"
#include "absl/container/internal/btree.h" // IWYU pragma: export
#include "absl/container/internal/common.h"
#include "absl/memory/memory.h"
#include "absl/meta/type_traits.h"

namespace absl {
Expand Down Expand Up @@ -68,8 +69,21 @@ class btree_container {
explicit btree_container(const key_compare &comp,
const allocator_type &alloc = allocator_type())
: tree_(comp, alloc) {}
btree_container(const btree_container &other) = default;
btree_container(btree_container &&other) noexcept = default;
explicit btree_container(const allocator_type &alloc)
: tree_(key_compare(), alloc) {}

btree_container(const btree_container &other)
: btree_container(other, absl::allocator_traits<allocator_type>::
select_on_container_copy_construction(
other.get_allocator())) {}
btree_container(const btree_container &other, const allocator_type &alloc)
: tree_(other.tree_, alloc) {}

btree_container(btree_container &&other) noexcept(
std::is_nothrow_move_constructible<Tree>::value) = default;
btree_container(btree_container &&other, const allocator_type &alloc)
: tree_(std::move(other.tree_), alloc) {}

btree_container &operator=(const btree_container &other) = default;
btree_container &operator=(btree_container &&other) noexcept(
std::is_nothrow_move_assignable<Tree>::value) = default;
Expand Down Expand Up @@ -234,20 +248,27 @@ class btree_set_container : public btree_container<Tree> {
using super_type::super_type;
btree_set_container() {}

// Range constructor.
// Range constructors.
template <class InputIterator>
btree_set_container(InputIterator b, InputIterator e,
const key_compare &comp = key_compare(),
const allocator_type &alloc = allocator_type())
: super_type(comp, alloc) {
insert(b, e);
}
template <class InputIterator>
btree_set_container(InputIterator b, InputIterator e,
const allocator_type &alloc)
: btree_set_container(b, e, key_compare(), alloc) {}

// Initializer list constructor.
// Initializer list constructors.
btree_set_container(std::initializer_list<init_type> init,
const key_compare &comp = key_compare(),
const allocator_type &alloc = allocator_type())
: btree_set_container(init.begin(), init.end(), comp, alloc) {}
btree_set_container(std::initializer_list<init_type> init,
const allocator_type &alloc)
: btree_set_container(init.begin(), init.end(), alloc) {}

// Lookup routines.
template <typename K = key_type>
Expand Down Expand Up @@ -535,20 +556,27 @@ class btree_multiset_container : public btree_container<Tree> {
using super_type::super_type;
btree_multiset_container() {}

// Range constructor.
// Range constructors.
template <class InputIterator>
btree_multiset_container(InputIterator b, InputIterator e,
const key_compare &comp = key_compare(),
const allocator_type &alloc = allocator_type())
: super_type(comp, alloc) {
insert(b, e);
}
template <class InputIterator>
btree_multiset_container(InputIterator b, InputIterator e,
const allocator_type &alloc)
: btree_multiset_container(b, e, key_compare(), alloc) {}

// Initializer list constructor.
// Initializer list constructors.
btree_multiset_container(std::initializer_list<init_type> init,
const key_compare &comp = key_compare(),
const allocator_type &alloc = allocator_type())
: btree_multiset_container(init.begin(), init.end(), comp, alloc) {}
btree_multiset_container(std::initializer_list<init_type> init,
const allocator_type &alloc)
: btree_multiset_container(init.begin(), init.end(), alloc) {}

// Lookup routines.
template <typename K = key_type>
Expand Down
Loading

0 comments on commit 5bf048b

Please sign in to comment.