Skip to content

Commit

Permalink
Let types explicitly indicate support for being passed to ExternalRef
Browse files Browse the repository at this point in the history 
constructors: `RiegeliSupportsExternalRef()` and
`RiegeliSupportsExternalRefWhole()`. They also indicate whether their substrings
are stable when the object is moved, and whether passing a copy of an object is
more efficient than copying the data.

`ExternalRef::From()` are like `ExternalRef` constructors, but
`RiegeliSupportsExternalRef()` or `RiegeliSupportsExternalRefWhole()` is not
needed. The caller is responsible for using an appropriate type of the external
object.

This mechanism replaces `ToExternalRef()` member functions in several classes,
and opts certain types into it. A uniform mechanism allows certain functions to
wrap objects in `ExternalRef` automatically. Also, callers are not forced to
remember whether specifying `substr` is valid for the moved object (this
property differs e.g. between `std::string` and `std::vector<char>`).

Allow types to provide a predicate for objects whose data must be copied, e.g.
because it is stored inline: `RiegeliExternalCopy()`.

Remove `ExternalRef` constructor which always makes a copy. This is no longer
needed given `RiegeliExternalCopy()`.

Call `RiegeliTo{ChainBlock,Cord,ExternalData}(T*, substr)` if defined with
`substr` also when `ExternalRef` constructor was called without `substr`.

Allow types to provide a function which indicates that a subobject should be
wrapped instead: `RiegeliExternalDelegate()`.

Express appending `ExternalRef` to `Chain` as `Chain::Append(ExternalRef)`
instead of `ExternalRef::AppendTo(Chain&)`. Same for prepending. This is
inconsistent with `Cord` but is more natural in isolation, and allows for
automatic wrapping of objects in `ExternalRef` (see below).

Convert data to `ExternalRef` automatically when this is indicated as supported
in `{,Backward}Writer::Write()` and in conversions to `Chain`.

Add `Chain::Reset(ExternalRef)` and optimize the behavior of
`riegeli::Reset(Cord&, ExternalRef)`.

Do not bother including the overhead of wrapper objects when checking whether
wrapping an object would be wasteful, for simplicity. This was inaccurate anyway
when `RiegeliTo{ChainBlock,Cord,ExternalData}()` is used.

Add assertions that the specified substring of an external object is indeed a
substring if the object supports `riegeli::ToStringView()`.

Change `Chain::BlockIterator` to yield a new type `Chain::BlockRef` instead of
`absl::string_view`. This allows conversion to `ExternalRef` to be defined on
`Chain::BlockRef` rather than `Chain::BlockIterator`, which works also when
iterators are implicit in a `for` loop.

PiperOrigin-RevId: 658318102
  • Loading branch information
@QrczakMK
QrczakMK committed Aug 1, 2024
1 parent 31c4dd1 commit d367293
Show file tree
Hide file tree
Showing 47 changed files with 2,150 additions and 1,150 deletions.
27 changes: 21 additions & 6 deletions riegeli/base/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,17 @@ cc_library(
deps = ["@com_google_absl//absl/strings"],
)

cc_library(
name = "external_ref_support",
hdrs = ["external_ref_support.h"],
deps = [
":external_data",
":to_string_view",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/meta:type_traits",
],
)

cc_library(
name = "shared_ptr",
hdrs = [
Expand All @@ -375,6 +386,7 @@ cc_library(
":compare",
":constexpr",
":external_data",
":external_ref_support",
":initializer",
":new_aligned",
":reset",
Expand All @@ -394,7 +406,6 @@ cc_library(
":buffering",
":estimated_allocated_size",
":external_data",
":external_ref",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/strings",
],
Expand All @@ -409,7 +420,6 @@ cc_library(
":assert",
":buffer",
":external_data",
":external_ref",
":initializer",
":shared_ptr",
"@com_google_absl//absl/base:core_headers",
Expand All @@ -425,7 +435,6 @@ cc_library(
":arithmetic",
":assert",
":buffering",
":external_ref",
":shared_buffer",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/strings",
Expand All @@ -449,6 +458,7 @@ cc_library(
":compare",
":cord_utils",
":external_data",
":external_ref_support",
":global",
":initializer",
":memory_estimator",
Expand All @@ -471,13 +481,19 @@ cc_library(
cc_library(
name = "chain",
hdrs = ["chain.h"],
deps = [":chain_and_external_ref"],
deps = [
":chain_and_external_ref",
":external_ref_support",
],
)

cc_library(
name = "external_ref",
hdrs = ["external_ref.h"],
deps = [":chain_and_external_ref"],
deps = [
":chain_and_external_ref",
":external_ref_support",
],
)

cc_library(
Expand All @@ -490,7 +506,6 @@ cc_library(
":compare",
":estimated_allocated_size",
":external_data",
":external_ref",
":new_aligned",
"@com_google_absl//absl/base:config",
"@com_google_absl//absl/base:core_headers",
Expand Down
26 changes: 2 additions & 24 deletions riegeli/base/buffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include <stddef.h>

#include <functional>
#include <iosfwd>
#include <utility>

Expand All @@ -27,7 +26,6 @@
#include "riegeli/base/buffering.h"
#include "riegeli/base/estimated_allocated_size.h"
#include "riegeli/base/external_data.h"
#include "riegeli/base/external_ref.h"

namespace riegeli {

Expand Down Expand Up @@ -61,28 +59,8 @@ class
// Returns the usable data size. It can be greater than the requested size.
size_t capacity() const { return capacity_; }

// Converts a substring of `*this` to `ExternalRef`.
//
// `storage` must outlive usages of the returned `ExternalRef`.
//
// Precondition:
// if `!substr.empty()` then `substr` is a substring of
// [`data()`..`data() + capacity()`).
ExternalRef ToExternalRef(absl::string_view substr,
ExternalRef::StorageSubstr<Buffer&&>&& storage
ABSL_ATTRIBUTE_LIFETIME_BOUND =
ExternalRef::StorageSubstr<Buffer&&>()) && {
if (!substr.empty()) {
RIEGELI_ASSERT(std::greater_equal<>()(substr.data(), data()))
<< "Failed precondition of Buffer::ToExternalRef(): "
"substring not contained in the buffer";
RIEGELI_ASSERT(std::less_equal<>()(substr.data() + substr.size(),
data() + capacity()))
<< "Failed precondition of Buffer::ToExternalRef(): "
"substring not contained in the buffer";
}
return ExternalRef(std::move(*this), substr, std::move(storage));
}
// Indicate support for `ExternalRef(Buffer&&, substr)`.
friend void RiegeliSupportsExternalRef(Buffer*) {}

// Support `ExternalRef`.
friend size_t RiegeliExternalMemory(const Buffer* self) {
Expand Down
87 changes: 16 additions & 71 deletions riegeli/base/chain.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <memory>
#include <ostream>
#include <string>
#include <type_traits>
#include <utility>

#include "absl/base/attributes.h"
Expand Down Expand Up @@ -422,6 +421,17 @@ absl::Cord Chain::Block::ToCord(absl::string_view substr) && {
return absl::MakeCordFromExternal(substr, [block = std::move(block_)] {});
}

absl::Cord Chain::Block::ToCord(absl::string_view substr) const& {
if (const FlatCordBlock* const cord_ptr =
block_->checked_external_object<FlatCordBlock>()) {
if (substr.size() == cord_ptr->src().size()) return cord_ptr->src();
return cord_ptr->src().Subcord(
PtrDistance(absl::string_view(*cord_ptr).data(), substr.data()),
substr.size());
}
return absl::MakeCordFromExternal(substr, [block = block_] {});
}

void Chain::Block::DumpStructure(absl::string_view substr,
std::ostream& out) const {
out << "[block] { offset: "
Expand Down Expand Up @@ -464,24 +474,6 @@ void Chain::Reset(absl::string_view src) {
Initialize(src);
}

template <typename Src,
std::enable_if_t<std::is_same<Src, std::string>::value, int>>
void Chain::Reset(Src&& src) {
size_ = 0;
if (begin_ != end_ && ClearSlow()) {
const size_t size = src.size();
// `std::move(src)` is correct and `std::forward<Src>(src)` is not
// necessary: `Src` is always `std::string`, never an lvalue reference.
Append(std::move(src), Options().set_size_hint(size));
return;
}
// `std::move(src)` is correct and `std::forward<Src>(src)` is not necessary:
// `Src` is always `std::string`, never an lvalue reference.
Initialize(std::move(src));
}

template void Chain::Reset(std::string&& src);

void Chain::Reset(Block src) {
size_ = 0;
UnrefBlocks();
Expand Down Expand Up @@ -523,26 +515,6 @@ void Chain::InitializeSlow(absl::string_view src) {
Append(src, options);
}

template <typename Src,
std::enable_if_t<std::is_same<Src, std::string>::value, int>>
void Chain::InitializeSlow(Src&& src) {
RIEGELI_ASSERT_GT(src.size(), kMaxShortDataSize)
<< "Failed precondition of Chain::InitializeSlow(string&&): "
"string too short, use Initialize() instead";
if (Wasteful(
RawBlock::kExternalAllocatedSize<std::string>() + src.capacity() + 1,
src.size())) {
// Not `std::move(src)`: forward to `InitializeSlow(absl::string_view)`.
InitializeSlow(src);
return;
}
// `std::move(src)` is correct and `std::forward<Src>(src)` is not necessary:
// `Src` is always `std::string`, never an lvalue reference.
Initialize(Block(std::move(src)));
}

template void Chain::InitializeSlow(std::string&& src);

inline void Chain::Initialize(const absl::Cord& src) {
RIEGELI_ASSERT_EQ(size_, 0u)
<< "Failed precondition of Chain::Initialize(const Cord&): "
Expand Down Expand Up @@ -571,8 +543,8 @@ inline void Chain::InitializeFromCord(CordRef&& src) {
return;
}
}
const size_t size = src.size();
AppendCordSlow(std::forward<CordRef>(src), Options().set_size_hint(size));
AppendCordSlow(std::forward<CordRef>(src),
Options().set_size_hint(src.size()));
}

inline void Chain::Initialize(const Chain& src) {
Expand Down Expand Up @@ -1423,16 +1395,6 @@ void Chain::Append(absl::string_view src, Options options) {
}
}

template <typename Src,
std::enable_if_t<std::is_same<Src, std::string>::value, int>>
void Chain::Append(Src&& src, Options options) {
// `std::move(src)` is correct and `std::forward<Src>(src)` is not necessary:
// `Src` is always `std::string`, never an lvalue reference.
ExternalRef(std::move(src)).AppendTo(*this, options);
}

template void Chain::Append(std::string&& src, Options options);

void Chain::Append(const Chain& src, Options options) {
AppendChain<ShareOwnership>(src, options);
}
Expand Down Expand Up @@ -1714,16 +1676,6 @@ void Chain::Prepend(absl::string_view src, Options options) {
}
}

template <typename Src,
std::enable_if_t<std::is_same<Src, std::string>::value, int>>
void Chain::Prepend(Src&& src, Options options) {
// `std::move(src)` is correct and `std::forward<Src>(src)` is not necessary:
// `Src` is always `std::string`, never an lvalue reference.
ExternalRef(std::move(src)).PrependTo(*this, options);
}

template void Chain::Prepend(std::string&& src, Options options);

void Chain::Prepend(const Chain& src, Options options) {
PrependChain<ShareOwnership>(src, options);
}
Expand Down Expand Up @@ -2035,11 +1987,8 @@ void Chain::RemoveSuffix(size_t length, Options options) {
data.remove_suffix(length);
// Compensate for increasing `size_` by `Append()`.
size_ -= data.size();
if (data.size() <= kMaxBytesToCopy) {
Append(data, options);
return;
}
Append(Block(riegeli::Invoker(MakeBlock(), std::move(last)), data), options);
Append(ExternalRef(riegeli::Invoker(MakeBlock(), std::move(last)), data),
options);
}

void Chain::RemovePrefix(size_t length, Options options) {
Expand Down Expand Up @@ -2085,11 +2034,7 @@ void Chain::RemovePrefix(size_t length, Options options) {
data.remove_prefix(length);
// Compensate for increasing `size_` by `Prepend()`.
size_ -= data.size();
if (data.size() <= kMaxBytesToCopy) {
Prepend(data, options);
return;
}
Prepend(Block(riegeli::Invoker(MakeBlock(), std::move(first)), data),
Prepend(ExternalRef(riegeli::Invoker(MakeBlock(), std::move(first)), data),
options);
}

Expand Down
7 changes: 4 additions & 3 deletions riegeli/base/chain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
#ifndef RIEGELI_BASE_CHAIN_H_
#define RIEGELI_BASE_CHAIN_H_

#include "riegeli/base/chain_base.h" // IWYU pragma: export
#include "riegeli/base/chain_details.h" // IWYU pragma: export
#include "riegeli/base/external_ref_base.h" // IWYU pragma: keep
#include "riegeli/base/chain_base.h" // IWYU pragma: export
#include "riegeli/base/chain_details.h" // IWYU pragma: export
#include "riegeli/base/external_ref_base.h" // IWYU pragma: keep
#include "riegeli/base/external_ref_support.h" // IWYU pragma: keep

#endif // RIEGELI_BASE_CHAIN_H_
Loading

0 comments on commit d367293

Please sign in to comment.