Skip to content

[libc] Use RAII based alloc in gpu rpc_server instead of manual new/delete #110341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 28, 2024

Conversation

Hardcode84
Copy link
Contributor

No description provided.

@Hardcode84 Hardcode84 requested a review from jhuber6 September 27, 2024 23:49
@Hardcode84 Hardcode84 marked this pull request as ready for review September 28, 2024 01:49
@llvmbot llvmbot added the libc label Sep 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 28, 2024

@llvm/pr-subscribers-libc

Author: Ivan Butygin (Hardcode84)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/110341.diff

1 Files Affected:

  • (modified) libc/utils/gpu/server/rpc_server.cpp (+29-22)
diff --git a/libc/utils/gpu/server/rpc_server.cpp b/libc/utils/gpu/server/rpc_server.cpp
index 602b266f9dacb0..29bbcd2df2303f 100644
--- a/libc/utils/gpu/server/rpc_server.cpp
+++ b/libc/utils/gpu/server/rpc_server.cpp
@@ -206,6 +206,18 @@ void handle_printf(rpc::Server::Port &port) {
     delete[] reinterpret_cast<char *>(ptr);
 }
 
+namespace {
+struct TempStorage {
+  char *alloc(size_t size) {
+    storage.emplace_back(std::make_unique<char[]>(size));
+    return storage.back().get();
+  }
+
+  // TODO: SmallVector
+  std::vector<std::unique_ptr<char[]>> storage;
+};
+} // namespace
+
 template <uint32_t lane_size>
 rpc_status_t handle_server_impl(
     rpc::Server &server,
@@ -216,6 +228,8 @@ rpc_status_t handle_server_impl(
   if (!port)
     return RPC_STATUS_SUCCESS;
 
+  TempStorage temp_storage;
+
   switch (port->get_opcode()) {
   case RPC_WRITE_TO_STREAM:
   case RPC_WRITE_TO_STDERR:
@@ -234,7 +248,8 @@ rpc_status_t handle_server_impl(
       std::fill(files, files + lane_size, stdout);
     }
 
-    port->recv_n(strs, sizes, [&](uint64_t size) { return new char[size]; });
+    port->recv_n(strs, sizes,
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->send([&](rpc::Buffer *buffer, uint32_t id) {
       flockfile(files[id]);
       buffer->data[0] = fwrite_unlocked(strs[id], 1, sizes[id], files[id]);
@@ -242,7 +257,6 @@ rpc_status_t handle_server_impl(
           buffer->data[0] == sizes[id])
         buffer->data[0] += fwrite_unlocked("\n", 1, 1, files[id]);
       funlockfile(files[id]);
-      delete[] reinterpret_cast<uint8_t *>(strs[id]);
     });
     break;
   }
@@ -250,13 +264,12 @@ rpc_status_t handle_server_impl(
     uint64_t sizes[lane_size] = {0};
     void *data[lane_size] = {nullptr};
     port->recv([&](rpc::Buffer *buffer, uint32_t id) {
-      data[id] = new char[buffer->data[0]];
+      data[id] = temp_storage.alloc(buffer->data[0]);
       sizes[id] =
           fread(data[id], 1, buffer->data[0], file::to_stream(buffer->data[1]));
     });
     port->send_n(data, sizes);
     port->send([&](rpc::Buffer *buffer, uint32_t id) {
-      delete[] reinterpret_cast<uint8_t *>(data[id]);
       std::memcpy(buffer->data, &sizes[id], sizeof(uint64_t));
     });
     break;
@@ -265,27 +278,24 @@ rpc_status_t handle_server_impl(
     uint64_t sizes[lane_size] = {0};
     void *data[lane_size] = {nullptr};
     port->recv([&](rpc::Buffer *buffer, uint32_t id) {
-      data[id] = new char[buffer->data[0]];
+      data[id] = temp_storage.alloc(buffer->data[0]);
       const char *str =
           fgets(reinterpret_cast<char *>(data[id]), buffer->data[0],
                 file::to_stream(buffer->data[1]));
       sizes[id] = !str ? 0 : std::strlen(str) + 1;
     });
     port->send_n(data, sizes);
-    for (uint32_t id = 0; id < lane_size; ++id)
-      if (data[id])
-        delete[] reinterpret_cast<uint8_t *>(data[id]);
     break;
   }
   case RPC_OPEN_FILE: {
     uint64_t sizes[lane_size] = {0};
     void *paths[lane_size] = {nullptr};
-    port->recv_n(paths, sizes, [&](uint64_t size) { return new char[size]; });
+    port->recv_n(paths, sizes,
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->recv_and_send([&](rpc::Buffer *buffer, uint32_t id) {
       FILE *file = fopen(reinterpret_cast<char *>(paths[id]),
                          reinterpret_cast<char *>(buffer->data));
       buffer->data[0] = reinterpret_cast<uintptr_t>(file);
-      delete[] reinterpret_cast<uint8_t *>(paths[id]);
     });
     break;
   }
@@ -316,13 +326,12 @@ rpc_status_t handle_server_impl(
   case RPC_HOST_CALL: {
     uint64_t sizes[lane_size] = {0};
     void *args[lane_size] = {nullptr};
-    port->recv_n(args, sizes, [&](uint64_t size) { return new char[size]; });
+    port->recv_n(args, sizes,
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->recv([&](rpc::Buffer *buffer, uint32_t id) {
       reinterpret_cast<void (*)(void *)>(buffer->data[0])(args[id]);
     });
-    port->send([&](rpc::Buffer *, uint32_t id) {
-      delete[] reinterpret_cast<uint8_t *>(args[id]);
-    });
+    port->send([&](rpc::Buffer *, uint32_t id) {});
     break;
   }
   case RPC_FEOF: {
@@ -385,11 +394,11 @@ rpc_status_t handle_server_impl(
   case RPC_REMOVE: {
     uint64_t sizes[lane_size] = {0};
     void *args[lane_size] = {nullptr};
-    port->recv_n(args, sizes, [&](uint64_t size) { return new char[size]; });
+    port->recv_n(args, sizes,
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->send([&](rpc::Buffer *buffer, uint32_t id) {
       buffer->data[0] = static_cast<uint64_t>(
           remove(reinterpret_cast<const char *>(args[id])));
-      delete[] reinterpret_cast<uint8_t *>(args[id]);
     });
     break;
   }
@@ -399,26 +408,24 @@ rpc_status_t handle_server_impl(
     void *oldpath[lane_size] = {nullptr};
     void *newpath[lane_size] = {nullptr};
     port->recv_n(oldpath, oldsizes,
-                 [&](uint64_t size) { return new char[size]; });
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->recv_n(newpath, newsizes,
-                 [&](uint64_t size) { return new char[size]; });
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->send([&](rpc::Buffer *buffer, uint32_t id) {
       buffer->data[0] = static_cast<uint64_t>(
           rename(reinterpret_cast<const char *>(oldpath[id]),
                  reinterpret_cast<const char *>(newpath[id])));
-      delete[] reinterpret_cast<uint8_t *>(oldpath[id]);
-      delete[] reinterpret_cast<uint8_t *>(newpath[id]);
     });
     break;
   }
   case RPC_SYSTEM: {
     uint64_t sizes[lane_size] = {0};
     void *args[lane_size] = {nullptr};
-    port->recv_n(args, sizes, [&](uint64_t size) { return new char[size]; });
+    port->recv_n(args, sizes,
+                 [&](uint64_t size) { return temp_storage.alloc(size); });
     port->send([&](rpc::Buffer *buffer, uint32_t id) {
       buffer->data[0] = static_cast<uint64_t>(
           system(reinterpret_cast<const char *>(args[id])));
-      delete[] reinterpret_cast<uint8_t *>(args[id]);
     });
     break;
   }

@Hardcode84 Hardcode84 merged commit ef390b3 into llvm:main Sep 28, 2024
7 checks passed
@Hardcode84 Hardcode84 deleted the rpc-serv-raii branch September 28, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants