Revert "Add lazy_clone_storage to create COW storages (pytorch#110192…

…)" This reverts commit 1c30814. Reverted pytorch#110192 on behalf of https://github.com/jeanschmidt due to Breaking internal builds, @ezyang please support the author providing further details ([comment](pytorch#110192 (comment)))
kurtamohler · Oct 16, 2023 · 97a513e · 97a513e
1 parent c271df9
commit 97a513e
Show file tree

Hide file tree

Showing 5 changed files with 2 additions and 248 deletions.
diff --git a/c10/core/build.bzl b/c10/core/build.bzl
@@ -58,22 +58,22 @@ def define_targets(rules):
             [
                 "*.cpp",
                 "impl/*.cpp",
+                "impl/cow/*.cpp",
             ],
             exclude = [
                 "CPUAllocator.cpp",
                 "impl/alloc_cpu.cpp",
-                "impl/cow/*.cpp",
             ],
         ),
         hdrs = rules.glob(
             [
                 "*.h",
                 "impl/*.h",
+                "impl/cow/*.h",
             ],
             exclude = [
                 "CPUAllocator.h",
                 "impl/alloc_cpu.h",
-                "impl/cow/*.h",
             ],
         ),
         linkstatic = True,
@@ -92,22 +92,6 @@ def define_targets(rules):
         alwayslink = True,
     )
 
-    rules.cc_library(
-        name = "impl_cow",
-        srcs = rules.glob([
-            "impl/cow/*.cpp",
-        ]),
-        hdrs = rules.glob([
-            "impl/cow/*.h",
-        ]),
-        deps = [
-            ":base",
-            ":CPUAllocator",
-        ],
-        visibility = ["//c10/test:__pkg__"],
-
-    )
-
     rules.filegroup(
         name = "headers",
         srcs = rules.glob(

diff --git a/c10/core/impl/cow/COW.cpp b/c10/core/impl/cow/COW.cpp
diff --git a/c10/core/impl/cow/COW.h b/c10/core/impl/cow/COW.h
diff --git a/c10/test/build.bzl b/c10/test/build.bzl
@@ -21,7 +21,6 @@ def define_targets(rules):
             "//c10/core:base",
             "//c10/util:base",
             "//c10/core:CPUAllocator",
-            "//c10/core:impl_cow",
             "@com_google_googletest//:gtest_main",
         ],
     )

diff --git a/c10/test/core/impl/cow_test.cpp b/c10/test/core/impl/cow_test.cpp
@@ -1,4 +1,3 @@
-#include <c10/core/impl/cow/COW.h>
 #include <c10/core/impl/cow/COWDeleter.h>
 
 #include <c10/core/CPUAllocator.h>
@@ -79,94 +78,6 @@ TEST_F(ContextTest, cow_deleter) {
   ASSERT_THAT(delete_count(), testing::Eq(1));
 }
 
-MATCHER(is_copy_on_write, "") {
-  const c10::StorageImpl& storage = std::ref(arg);
-  return cow::is_cow_data_ptr(storage.data_ptr());
-}
-
-TEST(lazy_clone_storage_test, no_context) {
-  StorageImpl original_storage(
-      {}, /*size_bytes=*/7, GetDefaultCPUAllocator(), /*resizable=*/false);
-  ASSERT_THAT(original_storage, testing::Not(is_copy_on_write()));
-  ASSERT_TRUE(cow::has_simple_data_ptr(original_storage));
-
-  intrusive_ptr<StorageImpl> new_storage =
-      cow::lazy_clone_storage(original_storage);
-  ASSERT_THAT(new_storage.get(), testing::NotNull());
-
-  // The original storage was modified in-place to now hold a copy on
-  // write context.
-  ASSERT_THAT(original_storage, is_copy_on_write());
-
-  // The result is a different storage impl.
-  ASSERT_THAT(&*new_storage, testing::Ne(&original_storage));
-  // But it is also copy-on-write.
-  ASSERT_THAT(*new_storage, is_copy_on_write());
-  // But they share the same data!
-  ASSERT_THAT(new_storage->data(), testing::Eq(original_storage.data()));
-}
-
-struct MyDeleterContext {
-  MyDeleterContext(void* bytes) : bytes(bytes) {}
-
-  ~MyDeleterContext() {
-    delete[] static_cast<std::byte*>(bytes);
-  }
-
-  void* bytes;
-};
-
-void my_deleter(void* ctx) {
-  delete static_cast<MyDeleterContext*>(ctx);
-}
-
-TEST(lazy_clone_storage_test, different_context) {
-  void* bytes = new std::byte[5];
-  StorageImpl storage(
-      {},
-      /*size_bytes=*/5,
-      at::DataPtr(
-          /*data=*/bytes,
-          /*ctx=*/new MyDeleterContext(bytes),
-          /*ctx_deleter=*/my_deleter,
-          /*device=*/Device(Device::Type::CPU)),
-      /*allocator=*/nullptr,
-      /*resizable=*/false);
-
-  // We can't handle an arbitrary context.
-  ASSERT_THAT(cow::lazy_clone_storage(storage), testing::IsNull());
-}
-
-TEST(lazy_clone_storage_test, already_copy_on_write) {
-  std::unique_ptr<void, DeleterFnPtr> data(
-      new std::byte[5],
-      +[](void* bytes) { delete[] static_cast<std::byte*>(bytes); });
-  void* data_ptr = data.get();
-  StorageImpl original_storage(
-      {},
-      /*size_bytes=*/5,
-      at::DataPtr(
-          /*data=*/data_ptr,
-          /*ctx=*/new cow::COWDeleterContext(std::move(data)),
-          cow::cow_deleter,
-          Device(Device::Type::CPU)),
-      /*allocator=*/nullptr,
-      /*resizable=*/false);
-
-  ASSERT_THAT(original_storage, is_copy_on_write());
-
-  intrusive_ptr<StorageImpl> new_storage =
-      cow::lazy_clone_storage(original_storage);
-  ASSERT_THAT(new_storage.get(), testing::NotNull());
-
-  // The result is a different storage.
-  ASSERT_THAT(&*new_storage, testing::Ne(&original_storage));
-  // But it is also copy-on-write.
-  ASSERT_THAT(*new_storage, is_copy_on_write());
-  // But they share the same data!
-  ASSERT_THAT(new_storage->data(), testing::Eq(original_storage.data()));
-}
-
 } // namespace
 } // namespace c10::impl
 // NOLINTEND(clang-analyzer-cplusplus*)