Skip to content

Commit a89fc17

Browse files
stream: reuse unexposed managed read buffers
Signed-off-by: GetThatCookie <NimmenKeks@gmx.de> PR-URL: #64990 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 7072d76 commit a89fc17

4 files changed

Lines changed: 43 additions & 4 deletions

File tree

src/env.cc

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ using v8::Undefined;
7777
using v8::Value;
7878
using worker::Worker;
7979

80+
constexpr size_t kManagedBufferCacheSize = 64 * 1024;
81+
8082
int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64;
8183
void* const ContextEmbedderTag::kNodeContextTagPtr = const_cast<void*>(
8284
static_cast<const void*>(&ContextEmbedderTag::kNodeContextTag));
@@ -782,10 +784,16 @@ void Environment::add_refs(int64_t diff) {
782784
}
783785

784786
uv_buf_t Environment::allocate_managed_buffer(const size_t suggested_size) {
785-
std::unique_ptr<BackingStore> bs = ArrayBuffer::NewBackingStore(
786-
isolate(),
787-
suggested_size,
788-
BackingStoreInitializationMode::kUninitialized);
787+
std::unique_ptr<BackingStore> bs;
788+
if (suggested_size == kManagedBufferCacheSize &&
789+
managed_buffer_cache_ != nullptr) {
790+
bs = std::move(managed_buffer_cache_);
791+
} else {
792+
bs = ArrayBuffer::NewBackingStore(
793+
isolate(),
794+
suggested_size,
795+
BackingStoreInitializationMode::kUninitialized);
796+
}
789797
uv_buf_t buf = uv_buf_init(static_cast<char*>(bs->Data()), bs->ByteLength());
790798
released_allocated_buffers_.emplace(buf.base, std::move(bs));
791799
return buf;
@@ -803,6 +811,11 @@ std::unique_ptr<BackingStore> Environment::release_managed_buffer(
803811
return bs;
804812
}
805813

814+
void Environment::recycle_managed_buffer(std::unique_ptr<BackingStore> bs) {
815+
if (bs != nullptr && bs->ByteLength() == kManagedBufferCacheSize)
816+
managed_buffer_cache_ = std::move(bs);
817+
}
818+
806819
std::string Environment::GetExecPath(const std::vector<std::string>& argv) {
807820
char exec_path_buf[2 * PATH_MAX];
808821
size_t exec_path_len = sizeof(exec_path_buf);

src/env.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,8 @@ class Environment final : public MemoryRetainer {
10631063

10641064
uv_buf_t allocate_managed_buffer(const size_t suggested_size);
10651065
std::unique_ptr<v8::BackingStore> release_managed_buffer(const uv_buf_t& buf);
1066+
// Only buffers that were not exposed externally may be recycled.
1067+
void recycle_managed_buffer(std::unique_ptr<v8::BackingStore> bs);
10661068

10671069
void AddUnmanagedFd(int fd);
10681070
void RemoveUnmanagedFd(int fd);
@@ -1279,6 +1281,7 @@ class Environment final : public MemoryRetainer {
12791281
// track of the BackingStore for a given pointer.
12801282
std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>
12811283
released_allocated_buffers_;
1284+
std::unique_ptr<v8::BackingStore> managed_buffer_cache_;
12821285

12831286
v8::CpuProfiler* cpu_profiler_ = nullptr;
12841287
std::vector<v8::ProfilerId> pending_profiles_;

src/stream_base.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,7 @@ void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
697697
std::unique_ptr<BackingStore> bs = env->release_managed_buffer(buf_);
698698

699699
if (nread <= 0) {
700+
env->recycle_managed_buffer(std::move(bs));
700701
if (nread < 0)
701702
stream->CallJSOnreadMethod(nread, Local<ArrayBuffer>());
702703
return;
@@ -708,6 +709,7 @@ void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
708709
bs = ArrayBuffer::NewBackingStore(
709710
isolate, nread, BackingStoreInitializationMode::kUninitialized);
710711
memcpy(bs->Data(), old_bs->Data(), nread);
712+
env->recycle_managed_buffer(std::move(old_bs));
711713
}
712714

713715
stream->CallJSOnreadMethod(nread, ArrayBuffer::New(isolate, std::move(bs)));

test/cctest/test_environment.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,27 @@ class EnvironmentTest : public EnvironmentTestFixture {
3939
}
4040
};
4141

42+
TEST_F(EnvironmentTest, ManagedBufferCache) {
43+
constexpr size_t kCacheSize = 64 * 1024;
44+
constexpr size_t kOtherSize = 1024;
45+
const v8::HandleScope handle_scope(isolate_);
46+
Argv argv;
47+
Env env{handle_scope, argv};
48+
49+
(*env)->recycle_managed_buffer(nullptr);
50+
51+
uv_buf_t buffer = (*env)->allocate_managed_buffer(kCacheSize);
52+
char* cached_data = buffer.base;
53+
(*env)->recycle_managed_buffer((*env)->release_managed_buffer(buffer));
54+
55+
buffer = (*env)->allocate_managed_buffer(kOtherSize);
56+
(*env)->recycle_managed_buffer((*env)->release_managed_buffer(buffer));
57+
58+
buffer = (*env)->allocate_managed_buffer(kCacheSize);
59+
EXPECT_EQ(buffer.base, cached_data);
60+
(*env)->release_managed_buffer(buffer);
61+
}
62+
4263
TEST_F(EnvironmentTest, EnvironmentWithoutBrowserGlobals) {
4364
const v8::HandleScope handle_scope(isolate_);
4465
Argv argv;

0 commit comments

Comments
 (0)