Skip to content

Commit b6b62c7

Browse files
Abseil Teamderekmauro
authored andcommitted
Export of internal Abseil changes
-- f49e405201d2ffd5955503fa8ad0f08ec0cdfb2b by Martijn Vels <[email protected]>: Add common [container.requirements] type definitions to `CharRange` and `ChunkRange` The presence of these allow these range classes to be used in various utility functions which require some minimum type of container. For example, this change allows tests to use `EXPECT_THAT(cord.Chunks(), ElementsAre(...))` PiperOrigin-RevId: 406941278 -- 0c195f073632e21d9a4bce158047b2ba8551c2d1 by Evan Brown <[email protected]>: Use explicit exponential growth in SubstituteAndAppendArray. PiperOrigin-RevId: 406931952 -- afb043bccd809a55cab78abadb7548a057d9eda0 by Jorg Brown <[email protected]>: Use longer var names in macro to avoid clang-tidy warning PiperOrigin-RevId: 406930978 -- 80397e2604e6b3d929a34742c3a32581b34d3ac4 by Martijn Vels <[email protected]>: Add future kAppendBuffer and kPrependBuffer API trackers for Cordz sampling PiperOrigin-RevId: 406912759 -- e910ce919ef83933f08a690e8b7325c7cc5b6d5d by Martijn Vels <[email protected]>: Implement Prepend(string_view) in terms of PrependArray(string_view, MethodIdentifier). PiperOrigin-RevId: 406891665 -- c9cff43d4c0568ed01f2fca0f6ef038ae03112b5 by Martijn Vels <[email protected]>: Add 'Rebuild' logic to CordRepBtree There are btree hostile scenarios where an application could perform repeated split/insert/merge operations on a cord leading to a tree exceeding the maximum height. While this should be rare in practice, this change adds a Rebuild() method that will rebuild a tree with a 100% fill factor, and we will invoke this rebuild when a tree exceeds the maximum height. This basically follows the similar 'balance' logic in Concat trees (although the latter is common in Concat uses) PiperOrigin-RevId: 406875739 -- 5b2b8fb88f1ebfdc1c670088152da2cb2ea4c376 by Martijn Vels <[email protected]>: Add 'in place' enabled RemoveSuffix An in-place RemoveSuffix is more efficient than SubTree() as it can directly modify privately owned nodes and flats allowing easy re-use of free capacity in right-most flats that may turn into Substring edges when using SubTree. PiperOrigin-RevId: 406431230 -- f09903c0a3d7344f59aaf1380a16ea10829217d4 by Derek Mauro <[email protected]>: Internal change PiperOrigin-RevId: 406430373 -- 9957af575c33bb18dc170572a4ee8cc5901df6b2 by Greg Falcon <[email protected]>: Initial groundwork to allow storing checksum data inside CordRep instances. This uses a RefcountAndFlags bit that was reserved for this purpose, and will be leveraged in a follow-up change to allow attaching checksums to a Cord's value. This change splits RefcountAndFlags::IsOne() into two distinct operations: * IsOne(): This returns true when the associated CordRep is not shared with other threads. This is useful for functions that consume CordRep instances; for example, code that consumes an unshared CordRep can assume ownership of its children without modifying those refcounts. * IsMutable(): This returns true when the associated CordRep reference is not shared with other threads, *and* does not store an associated checksum value. This is useful for functions that modify a CordRep's contents: code may modify the bytes of a mutable-unshared CordRep without fear of races with other threads, or of invalidating a stored checksum. The tricky part of this CL is ensuring that the correct choice between IsMutable() and IsOne() was made at each point. An incorrect application of IsOne() could lead to correctness bugs in the future. Code conditioned on IsOne() may delete the CordRep in question, or assume ownership of its children, but must not modify the CordRep's data without explicitly adjusting the CRC. PiperOrigin-RevId: 406191103 -- 686544814079e5ab6d4593cca0c068b510be400a by Martijn Vels <[email protected]>: Reduce the size in the LargeString test when running with Sanitizers PiperOrigin-RevId: 406186945 -- 735b4490bdb695c35731f06ce4b8de14ce2be6ed by Alex Strelnikov <[email protected]>: Release absl::SimpleHexAtoi. PiperOrigin-RevId: 406143188 GitOrigin-RevId: f49e405201d2ffd5955503fa8ad0f08ec0cdfb2b Change-Id: Ic6527ac40fa03ea02ca813e8bb7868a219544de4
1 parent 022527c commit b6b62c7

20 files changed

+890
-100
lines changed

absl/flags/flag.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ ABSL_NAMESPACE_END
241241
/* default value argument. That keeps temporaries alive */ \
242242
/* long enough for NonConst to work correctly. */ \
243243
static constexpr absl::string_view Value( \
244-
absl::string_view v = ABSL_FLAG_IMPL_FLAGHELP(txt)) { \
245-
return v; \
244+
absl::string_view absl_flag_help = ABSL_FLAG_IMPL_FLAGHELP(txt)) { \
245+
return absl_flag_help; \
246246
} \
247247
static std::string NonConst() { return std::string(Value()); } \
248248
}; \
@@ -254,8 +254,8 @@ ABSL_NAMESPACE_END
254254
#define ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \
255255
struct AbslFlagDefaultGenFor##name { \
256256
Type value = absl::flags_internal::InitDefaultValue<Type>(default_value); \
257-
static void Gen(void* p) { \
258-
new (p) Type(AbslFlagDefaultGenFor##name{}.value); \
257+
static void Gen(void* absl_flag_default_loc) { \
258+
new (absl_flag_default_loc) Type(AbslFlagDefaultGenFor##name{}.value); \
259259
} \
260260
};
261261

absl/strings/BUILD.bazel

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,17 @@ cc_library(
305305
],
306306
)
307307

308+
cc_test(
309+
name = "cord_internal_test",
310+
srcs = ["internal/cord_internal_test.cc"],
311+
copts = ABSL_TEST_COPTS,
312+
visibility = ["//visibility:private"],
313+
deps = [
314+
":cord_internal",
315+
"@com_google_googletest//:gtest_main",
316+
],
317+
)
318+
308319
cc_test(
309320
name = "cord_rep_btree_test",
310321
size = "medium",
@@ -684,6 +695,7 @@ cc_test(
684695
"//absl/base:endian",
685696
"//absl/base:raw_logging_internal",
686697
"//absl/container:fixed_array",
698+
"//absl/random",
687699
"@com_google_googletest//:gtest_main",
688700
],
689701
)

absl/strings/CMakeLists.txt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,7 @@ absl_cc_test(
920920
absl::cordz_test_helpers
921921
absl::core_headers
922922
absl::endian
923+
absl::random_random
923924
absl::raw_logging_internal
924925
absl::fixed_array
925926
GTest::gmock_main
@@ -943,6 +944,18 @@ absl_cc_test(
943944
GTest::gmock_main
944945
)
945946

947+
absl_cc_test(
948+
NAME
949+
cord_internal_test
950+
SRCS
951+
"internal/cord_internal_test.cc"
952+
COPTS
953+
${ABSL_TEST_COPTS}
954+
DEPS
955+
absl::cord_internal
956+
GTest::gmock_main
957+
)
958+
946959
absl_cc_test(
947960
NAME
948961
cord_rep_btree_test

absl/strings/cord.cc

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ void Cord::InlineRep::PrependTree(CordRep* tree, MethodIdentifier method) {
419419
// written to region and the actual size increase will be written to size.
420420
static inline bool PrepareAppendRegion(CordRep* root, char** region,
421421
size_t* size, size_t max_length) {
422-
if (root->IsBtree() && root->refcount.IsOne()) {
422+
if (root->IsBtree() && root->refcount.IsMutable()) {
423423
Span<char> span = root->btree()->GetAppendBuffer(max_length);
424424
if (!span.empty()) {
425425
*region = span.data();
@@ -430,11 +430,11 @@ static inline bool PrepareAppendRegion(CordRep* root, char** region,
430430

431431
// Search down the right-hand path for a non-full FLAT node.
432432
CordRep* dst = root;
433-
while (dst->IsConcat() && dst->refcount.IsOne()) {
433+
while (dst->IsConcat() && dst->refcount.IsMutable()) {
434434
dst = dst->concat()->right;
435435
}
436436

437-
if (!dst->IsFlat() || !dst->refcount.IsOne()) {
437+
if (!dst->IsFlat() || !dst->refcount.IsMutable()) {
438438
*region = nullptr;
439439
*size = 0;
440440
return false;
@@ -649,7 +649,7 @@ Cord& Cord::operator=(absl::string_view src) {
649649
if (tree != nullptr) {
650650
CordzUpdateScope scope(contents_.cordz_info(), method);
651651
if (tree->IsFlat() && tree->flat()->Capacity() >= length &&
652-
tree->refcount.IsOne()) {
652+
tree->refcount.IsMutable()) {
653653
// Copy in place if the existing FLAT node is reusable.
654654
memmove(tree->flat()->Data(), data, length);
655655
tree->length = length;
@@ -819,7 +819,7 @@ void Cord::Prepend(const Cord& src) {
819819
return Prepend(src_contents);
820820
}
821821

822-
void Cord::Prepend(absl::string_view src) {
822+
void Cord::PrependArray(absl::string_view src, MethodIdentifier method) {
823823
if (src.empty()) return; // memcpy(_, nullptr, 0) is undefined.
824824
if (!contents_.is_tree()) {
825825
size_t cur_size = contents_.inline_size();
@@ -834,7 +834,7 @@ void Cord::Prepend(absl::string_view src) {
834834
}
835835
}
836836
CordRep* rep = NewTree(src.data(), src.size(), 0);
837-
contents_.PrependTree(rep, CordzUpdateTracker::kPrependString);
837+
contents_.PrependTree(rep, method);
838838
}
839839

840840
template <typename T, Cord::EnableIfString<T>>
@@ -894,7 +894,7 @@ static CordRep* RemoveSuffixFrom(CordRep* node, size_t n) {
894894
if (n >= node->length) return nullptr;
895895
if (n == 0) return CordRep::Ref(node);
896896
absl::InlinedVector<CordRep*, kInlinedVectorSize> lhs_stack;
897-
bool inplace_ok = node->refcount.IsOne();
897+
bool inplace_ok = node->refcount.IsMutable();
898898

899899
while (node->IsConcat()) {
900900
assert(n <= node->length);
@@ -907,7 +907,7 @@ static CordRep* RemoveSuffixFrom(CordRep* node, size_t n) {
907907
n -= node->concat()->right->length;
908908
node = node->concat()->left;
909909
}
910-
inplace_ok = inplace_ok && node->refcount.IsOne();
910+
inplace_ok = inplace_ok && node->refcount.IsMutable();
911911
}
912912
assert(n <= node->length);
913913

@@ -968,9 +968,7 @@ void Cord::RemoveSuffix(size_t n) {
968968
auto constexpr method = CordzUpdateTracker::kRemoveSuffix;
969969
CordzUpdateScope scope(contents_.cordz_info(), method);
970970
if (tree->IsBtree()) {
971-
CordRep* old = tree;
972-
tree = tree->btree()->SubTree(0, tree->length - n);
973-
CordRep::Unref(old);
971+
tree = CordRepBtree::RemoveSuffix(tree->btree(), n);
974972
} else {
975973
CordRep* newrep = RemoveSuffixFrom(tree, n);
976974
CordRep::Unref(tree);

absl/strings/cord.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,16 @@ class Cord {
460460
// `Cord::chunk_begin()` and `Cord::chunk_end()`.
461461
class ChunkRange {
462462
public:
463+
// Fulfill minimum c++ container requirements [container.requirements]
464+
// Theses (partial) container type definitions allow ChunkRange to be used
465+
// in various utilities expecting a subset of [container.requirements].
466+
// For example, the below enables using `::testing::ElementsAre(...)`
467+
using value_type = absl::string_view;
468+
using reference = value_type&;
469+
using const_reference = const value_type&;
470+
using iterator = ChunkIterator;
471+
using const_iterator = ChunkIterator;
472+
463473
explicit ChunkRange(const Cord* cord) : cord_(cord) {}
464474

465475
ChunkIterator begin() const;
@@ -591,6 +601,16 @@ class Cord {
591601
// `Cord::char_begin()` and `Cord::char_end()`.
592602
class CharRange {
593603
public:
604+
// Fulfill minimum c++ container requirements [container.requirements]
605+
// Theses (partial) container type definitions allow CharRange to be used
606+
// in various utilities expecting a subset of [container.requirements].
607+
// For example, the below enables using `::testing::ElementsAre(...)`
608+
using value_type = char;
609+
using reference = value_type&;
610+
using const_reference = const value_type&;
611+
using iterator = CharIterator;
612+
using const_iterator = CharIterator;
613+
594614
explicit CharRange(const Cord* cord) : cord_(cord) {}
595615

596616
CharIterator begin() const;
@@ -881,6 +901,10 @@ class Cord {
881901
template <typename C>
882902
void AppendImpl(C&& src);
883903

904+
// Prepends the provided data to this instance. `method` contains the public
905+
// API method for this action which is tracked for Cordz sampling purposes.
906+
void PrependArray(absl::string_view src, MethodIdentifier method);
907+
884908
// Assigns the value in 'src' to this instance, 'stealing' its contents.
885909
// Requires src.length() > kMaxBytesToCopy.
886910
Cord& AssignLargeString(std::string&& src);
@@ -1220,6 +1244,10 @@ inline void Cord::Append(absl::string_view src) {
12201244
contents_.AppendArray(src, CordzUpdateTracker::kAppendString);
12211245
}
12221246

1247+
inline void Cord::Prepend(absl::string_view src) {
1248+
PrependArray(src, CordzUpdateTracker::kPrependString);
1249+
}
1250+
12231251
extern template void Cord::Append(std::string&& src);
12241252
extern template void Cord::Prepend(std::string&& src);
12251253

absl/strings/cord_test.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "absl/base/internal/raw_logging.h"
3535
#include "absl/base/macros.h"
3636
#include "absl/container/fixed_array.h"
37+
#include "absl/random/random.h"
3738
#include "absl/strings/cord_test_helpers.h"
3839
#include "absl/strings/cordz_test_helpers.h"
3940
#include "absl/strings/str_cat.h"
@@ -1833,6 +1834,48 @@ TEST_P(CordTest, Hardening) {
18331834
EXPECT_DEATH_IF_SUPPORTED(++cord.chunk_end(), "");
18341835
}
18351836

1837+
// This test mimics a specific (and rare) application repeatedly splitting a
1838+
// cord, inserting (overwriting) a string value, and composing a new cord from
1839+
// the three pieces. This is hostile towards a Btree implementation: A split of
1840+
// a node at any level is likely to have the right-most edge of the left split,
1841+
// and the left-most edge of the right split shared. For example, splitting a
1842+
// leaf node with 6 edges will result likely in a 1-6, 2-5, 3-4, etc. split,
1843+
// sharing the 'split node'. When recomposing such nodes, we 'injected' an edge
1844+
// in that node. As this happens with some probability on each level of the
1845+
// tree, this will quickly grow the tree until it reaches maximum height.
1846+
TEST_P(CordTest, BtreeHostileSplitInsertJoin) {
1847+
absl::BitGen bitgen;
1848+
1849+
// Start with about 1GB of data
1850+
std::string data(1 << 10, 'x');
1851+
absl::Cord buffer(data);
1852+
absl::Cord cord;
1853+
for (int i = 0; i < 1000000; ++i) {
1854+
cord.Append(buffer);
1855+
}
1856+
1857+
for (int j = 0; j < 1000; ++j) {
1858+
size_t offset = absl::Uniform(bitgen, 0u, cord.size());
1859+
size_t length = absl::Uniform(bitgen, 100u, data.size());
1860+
if (cord.size() == offset) {
1861+
cord.Append(absl::string_view(data.data(), length));
1862+
} else {
1863+
absl::Cord suffix;
1864+
if (offset + length < cord.size()) {
1865+
suffix = cord;
1866+
suffix.RemovePrefix(offset + length);
1867+
}
1868+
if (cord.size() > offset) {
1869+
cord.RemoveSuffix(cord.size() - offset);
1870+
}
1871+
cord.Append(absl::string_view(data.data(), length));
1872+
if (!suffix.empty()) {
1873+
cord.Append(suffix);
1874+
}
1875+
}
1876+
}
1877+
}
1878+
18361879
class AfterExitCordTester {
18371880
public:
18381881
bool Set(absl::Cord* cord, absl::string_view expected) {

absl/strings/internal/cord_internal.h

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ class RefcountAndFlags {
8787
constexpr RefcountAndFlags() : count_{kRefIncrement} {}
8888
struct Immortal {};
8989
explicit constexpr RefcountAndFlags(Immortal) : count_(kImmortalFlag) {}
90+
struct WithCrc {};
91+
explicit constexpr RefcountAndFlags(WithCrc)
92+
: count_(kCrcFlag | kRefIncrement) {}
9093

9194
// Increments the reference count. Imposes no memory ordering.
9295
inline void Increment() {
@@ -122,14 +125,32 @@ class RefcountAndFlags {
122125
return count_.load(std::memory_order_acquire) >> kNumFlags;
123126
}
124127

125-
// Returns whether the atomic integer is 1.
126-
// If the reference count is used in the conventional way, a
127-
// reference count of 1 implies that the current thread owns the
128-
// reference and no other thread shares it.
129-
// This call performs the test for a reference count of one, and
130-
// performs the memory barrier needed for the owning thread
131-
// to act on the object, knowing that it has exclusive access to the
132-
// object. Always returns false when the immortal bit is set.
128+
// Returns true if the referenced object carries a CRC value.
129+
bool HasCrc() const {
130+
return (count_.load(std::memory_order_relaxed) & kCrcFlag) != 0;
131+
}
132+
133+
// Returns true iff the atomic integer is 1 and this node does not store
134+
// a CRC. When both these conditions are met, the current thread owns
135+
// the reference and no other thread shares it, so its contents may be
136+
// safely mutated.
137+
//
138+
// If the referenced item is shared, carries a CRC, or is immortal,
139+
// it should not be modified in-place, and this function returns false.
140+
//
141+
// This call performs the memory barrier needed for the owning thread
142+
// to act on the object, so that if it returns true, it may safely
143+
// assume exclusive access to the object.
144+
inline bool IsMutable() {
145+
return (count_.load(std::memory_order_acquire)) == kRefIncrement;
146+
}
147+
148+
// Returns whether the atomic integer is 1. Similar to IsMutable(),
149+
// but does not check for a stored CRC. (An unshared node with a CRC is not
150+
// mutable, because changing its data would invalidate the CRC.)
151+
//
152+
// When this returns true, there are no other references, and data sinks
153+
// may safely adopt the children of the CordRep.
133154
inline bool IsOne() {
134155
return (count_.load(std::memory_order_acquire) & kRefcountMask) ==
135156
kRefIncrement;
@@ -149,14 +170,14 @@ class RefcountAndFlags {
149170
kNumFlags = 2,
150171

151172
kImmortalFlag = 0x1,
152-
kReservedFlag = 0x2,
173+
kCrcFlag = 0x2,
153174
kRefIncrement = (1 << kNumFlags),
154175

155176
// Bitmask to use when checking refcount by equality. This masks out
156177
// all flags except kImmortalFlag, which is part of the refcount for
157178
// purposes of equality. (A refcount of 0 or 1 does not count as 0 or 1
158179
// if the immortal bit is set.)
159-
kRefcountMask = ~kReservedFlag,
180+
kRefcountMask = ~kCrcFlag,
160181
};
161182

162183
std::atomic<int32_t> count_;

0 commit comments

Comments
 (0)