From a9de0b9e06e09ccbafc982f56c48efaf4b42eaa0 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 3 Jan 2025 11:11:31 -0800 Subject: [PATCH 1/3] src: use LocalVector in more places --- src/crypto/crypto_util.h | 5 +++-- src/env.cc | 7 ++----- src/env.h | 10 ++++++++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 5c717c6fdb0fc4..a72c0a2a908294 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -547,7 +547,8 @@ void ThrowCryptoError(Environment* env, class CipherPushContext { public: - inline explicit CipherPushContext(Environment* env) : env_(env) {} + inline explicit CipherPushContext(Environment* env) + : list_(env->isolate()), env_(env) {} inline void push_back(const char* str) { list_.emplace_back(OneByteString(env_->isolate(), str)); @@ -558,7 +559,7 @@ class CipherPushContext { } private: - std::vector> list_; + v8::LocalVector list_; Environment* env_; }; diff --git a/src/env.cc b/src/env.cc index d4426432d67ba6..f0f97244fdef63 100644 --- a/src/env.cc +++ b/src/env.cc @@ -176,11 +176,7 @@ bool AsyncHooks::pop_async_context(double async_id) { } #endif native_execution_async_resources_.resize(offset); - if (native_execution_async_resources_.size() < - native_execution_async_resources_.capacity() / 2 && - native_execution_async_resources_.size() > 16) { - native_execution_async_resources_.shrink_to_fit(); - } + native_execution_async_resources_.shrink_to_fit(); } if (js_execution_async_resources()->Length() > offset) [[unlikely]] { @@ -1694,6 +1690,7 @@ AsyncHooks::AsyncHooks(Isolate* isolate, const SerializeInfo* info) fields_(isolate, kFieldsCount, MAYBE_FIELD_PTR(info, fields)), async_id_fields_( isolate, kUidFieldsCount, MAYBE_FIELD_PTR(info, async_id_fields)), + native_execution_async_resources_(isolate), info_(info) { HandleScope handle_scope(isolate); if (info == nullptr) { diff --git a/src/env.h b/src/env.h index d21f4dcb815116..9ebe3c06ad8b2f 100644 --- a/src/env.h +++ b/src/env.h @@ -26,7 +26,7 @@ #include "aliased_buffer.h" #if HAVE_INSPECTOR -#include "inspector_agent.h" +#include "inspector_agent.h"f #include "inspector_profiler.h" #endif #include "callback_queue.h" @@ -401,7 +401,13 @@ class AsyncHooks : public MemoryRetainer { void grow_async_ids_stack(); v8::Global js_execution_async_resources_; - std::vector> native_execution_async_resources_; + + // TODO(@jasnell): Note that this is technically illegal use of + // v8::Locals, which aren't supposed to live beyond the handle + // scope in which they were created. This should likely be a + // std::vector> but that's possibly a + // larger change that should be made separately. + v8::LocalVector native_execution_async_resources_; // Non-empty during deserialization const SerializeInfo* info_ = nullptr; From 1dd5a52f4d2807aeab35e6fb8b44930531fdf5ba Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 3 Jan 2025 15:37:20 -0800 Subject: [PATCH 2/3] Update src/env.h Co-authored-by: Anna Henningsen --- src/env.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/env.h b/src/env.h index 9ebe3c06ad8b2f..795db023006715 100644 --- a/src/env.h +++ b/src/env.h @@ -26,7 +26,7 @@ #include "aliased_buffer.h" #if HAVE_INSPECTOR -#include "inspector_agent.h"f +#include "inspector_agent.h" #include "inspector_profiler.h" #endif #include "callback_queue.h" From 26b3c04c5658efb9230b45d78b6c24d87d848b8d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 3 Jan 2025 15:40:11 -0800 Subject: [PATCH 3/3] Update src/env.h --- src/env.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/env.h b/src/env.h index 795db023006715..ec5b608cede6a1 100644 --- a/src/env.h +++ b/src/env.h @@ -403,10 +403,13 @@ class AsyncHooks : public MemoryRetainer { v8::Global js_execution_async_resources_; // TODO(@jasnell): Note that this is technically illegal use of - // v8::Locals, which aren't supposed to live beyond the handle - // scope in which they were created. This should likely be a - // std::vector> but that's possibly a - // larger change that should be made separately. + // v8::Locals which should be kept on the stack. Here, the entries + // in this object grows and shrinks with the C stack, and entries + // will be in the right handle scopes, but v8::Locals are supposed + // to remain on the stack and not the heap. For general purposes + // this *should* be ok but may need to be looked at further should + // v8 become stricter in the future about v8::Locals being held in + // the stack. v8::LocalVector native_execution_async_resources_; // Non-empty during deserialization