Skip to content

Commit

Permalink
In MemoryRegion, use swap on move-assignment.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 716714484
Change-Id: I309bbfb68f31941f60fad4c01cb62ffe992f8e29
  • Loading branch information
laramiel authored and copybara-github committed Jan 17, 2025
1 parent df80595 commit 2140adf
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 6 deletions.
1 change: 0 additions & 1 deletion tensorstore/internal/flat_cord_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ namespace internal {
/// likely to be retained.
class FlatCordBuilder {
public:
FlatCordBuilder() = default;
explicit FlatCordBuilder(size_t size) : FlatCordBuilder(size, size) {}

explicit FlatCordBuilder(internal_os::MemoryRegion region)
Expand Down
1 change: 1 addition & 0 deletions tensorstore/internal/os/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ tensorstore_cc_test(
srcs = ["memory_region_test.cc"],
deps = [
":memory_region",
"@com_google_absl//absl/strings:cord",
"@com_google_googletest//:gtest_main",
],
)
15 changes: 10 additions & 5 deletions tensorstore/internal/os/memory_region.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,21 @@ class MemoryRegion {
MemoryRegion(const MemoryRegion&) = delete;
MemoryRegion& operator=(const MemoryRegion&) = delete;

MemoryRegion(MemoryRegion&& other) { *this = std::move(other); }
MemoryRegion(MemoryRegion&& other)
: data_(std::exchange(other.data_, nullptr)),
size_(std::exchange(other.size_, 0)),
unmap_fn_(other.unmap_fn_) {}

MemoryRegion& operator=(MemoryRegion&& other) {
data_ = std::exchange(other.data_, nullptr);
size_ = std::exchange(other.size_, 0);
unmap_fn_ = std::exchange(other.unmap_fn_, nullptr);
std::swap(data_, other.data_);
std::swap(size_, other.size_);
std::swap(unmap_fn_, other.unmap_fn_);
return *this;
}

size_t size() const { return size_; }
char* data() const { return data_; }
const char* data() const { return data_; }
char* data() { return data_; }

std::string_view as_string_view() const {
return std::string_view(data_, size_);
Expand Down
20 changes: 20 additions & 0 deletions tensorstore/internal/os/memory_region_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,35 @@

#include <stddef.h>

#include <utility>

#include <gtest/gtest.h>
#include "absl/strings/cord.h"

using ::tensorstore::internal_os::AllocateHeapRegion;

namespace {

TEST(MemoryRegionTest, EmptyRegion) {
auto region = AllocateHeapRegion(0);
EXPECT_EQ(region.as_string_view().size(), 0);
absl::Cord a = std::move(region).as_cord();
}

TEST(MemoryRegionTest, Assignment) {
auto region = AllocateHeapRegion(0);
for (int i = 0; i < 10; ++i) region = AllocateHeapRegion(i);
}

TEST(MemoryRegionTest, AllocateHeapRegion) {
auto region = AllocateHeapRegion(16 * 1024 * 1024);
EXPECT_EQ(region.as_string_view().size(), 16 * 1024 * 1024);

// Verify that assignment doesn't leak.
region = AllocateHeapRegion(16);
EXPECT_EQ(region.as_string_view().size(), 16);

absl::Cord a = std::move(region).as_cord();
}

} // namespace

0 comments on commit 2140adf

Please sign in to comment.