src: use per-env cached strings for coroutine type names

Author: jasnell

src/coro/README.md

@@ -128,9 +128,9 @@ adds three phases around `Start()`:
    cancellation during teardown. If async\_hooks listeners are active
    (`kInit > 0` or `kUsesExecutionAsyncResource > 0`), a resource object
    is created for `executionAsyncResource()` and the `init` hook is emitted.
-   The type name V8 string is cached per template instantiation via
-   `v8::Eternal<v8::String>`, so only the first coroutine of a given type
-   pays the `String::NewFromUtf8` cost.
+   The type name V8 string is cached per Isolate in
+   `IsolateData::static_str_map`, so only the first coroutine of a given
+   type per Isolate pays the `String::NewFromUtf8` cost.
 
 3. **`Start()`**: Marks the task as detached (fire-and-forget) and resumes
    the coroutine. Each resume-to-suspend segment is wrapped in an
@@ -194,6 +194,13 @@ frames up to 4096 bytes (which covers typical coroutine frames). Frames larger
 than 4096 bytes fall through to the global `operator new`. Since all coroutines
 run on the event loop thread, the free-list requires no locking.
 
+Each bucket has a high-water mark of 32 cached frames. When a frame is freed
+and its bucket is already at capacity, the frame is returned directly to the
+system allocator instead of being cached. This bounds the retained memory
+per bucket to at most 32 \* bucket\_size bytes (e.g., 32 \* 1024 = 32KB for the
+1024-byte size class), preventing unbounded growth after a burst of concurrent
+coroutines.
+
 After the first coroutine of a given size class completes, subsequent
 coroutines of the same size class are allocated from the free-list with zero
 `malloc` overhead.
@@ -239,14 +246,14 @@ coroutine does not necessarily pay N times the `uv_fs_t` cost.
   uses the provided name; the `destroy` trace event currently uses a generic
   `"coroutine"` category name rather than the per-instance name.
 
-* **Free-list growth**: The thread-local free-list does not have a cap on the
-  number of cached frames per size class. Under a workload that creates a
-  large burst of concurrent coroutines and then goes idle, the free-list will
-  retain all of those frames until the thread exits. A maximum per-bucket
-  count could be added if this becomes a concern.
-
-* **Static Eternal handles**: The cached type name `v8::Eternal<v8::String>`
-  is a static variable per template instantiation. It is never freed and is
-  shared across all Isolates on the same thread. This is safe for the
-  single-Isolate case (the common case for Node.js), but would need
-  per-Isolate caching if multiple Isolates use the same coroutine types.
+* **Free-list retention**: The thread-local free-list retains up to 32 frames
+  per size class bucket after a burst of concurrent coroutines. These frames
+  are held until reused or the thread exits. The bound is configurable via
+  `kMaxCachedPerBucket`.
+
+* **Cached type name strings**: The type name `v8::Eternal<v8::String>` is
+  cached in `IsolateData::static_str_map`, keyed by the `const char*` from
+  the `ConstString` template parameter. This is per-Isolate and safe with
+  Worker threads (each Worker has its own `IsolateData`). The Eternal handles
+  are never freed, but there is at most one per unique type name string per
+  Isolate.

src/coro/uv_tracked_task.h

@@ -60,6 +60,9 @@ inline constexpr size_t kSizeClassGranularity = 256;
 // Number of size class buckets.
 inline constexpr size_t kNumSizeClasses =
     kMaxCachedFrameSize / kSizeClassGranularity;
+// Maximum number of frames to cache per size class bucket.
+// Bounds memory retained after a burst of concurrent coroutines.
+inline constexpr size_t kMaxCachedPerBucket = 32;
 
 inline size_t SizeClassIndex(size_t n) {
   return (n + kSizeClassGranularity - 1) / kSizeClassGranularity - 1;
@@ -69,20 +72,26 @@ inline size_t RoundUpToSizeClass(size_t n) {
   return (SizeClassIndex(n) + 1) * kSizeClassGranularity;
 }
 
+struct FreeBucket {
+  FreeBlock* head = nullptr;
+  size_t count = 0;
+};
+
 // Thread-local free lists, one per size class.
 // Safe without locks because coroutines run on the event loop thread.
-inline FreeBlock*& GetFreeList(size_t index) {
-  static thread_local FreeBlock* free_lists[kNumSizeClasses] = {};
-  return free_lists[index];
+inline FreeBucket& GetBucket(size_t index) {
+  static thread_local FreeBucket buckets[kNumSizeClasses] = {};
+  return buckets[index];
 }
 
 inline void* CoroFrameAlloc(size_t n) {
   if (n <= kMaxCachedFrameSize) {
     size_t idx = SizeClassIndex(n);
-    FreeBlock*& head = GetFreeList(idx);
-    if (head != nullptr) {
-      FreeBlock* block = head;
-      head = block->next;
+    FreeBucket& bucket = GetBucket(idx);
+    if (bucket.head != nullptr) {
+      FreeBlock* block = bucket.head;
+      bucket.head = block->next;
+      bucket.count--;
       return block;
     }
     // Nothing on free list; allocate at rounded-up size.
@@ -94,11 +103,16 @@ inline void* CoroFrameAlloc(size_t n) {
 inline void CoroFrameFree(void* p, size_t n) {
   if (n <= kMaxCachedFrameSize) {
     size_t idx = SizeClassIndex(n);
-    FreeBlock*& head = GetFreeList(idx);
+    FreeBucket& bucket = GetBucket(idx);
+    if (bucket.count >= kMaxCachedPerBucket) {
+      // Bucket is full; return directly to the system allocator.
+      ::operator delete(p, RoundUpToSizeClass(n));
+      return;
+    }
     auto* block = static_cast<FreeBlock*>(p);
-    block->next = head;
-    block->size = n;
-    head = block;
+    block->next = bucket.head;
+    bucket.head = block;
+    bucket.count++;
     return;
   }
   ::operator delete(p, n);
@@ -183,9 +197,7 @@ struct TrackedPromiseBase : PromiseBase {
   static void* operator new(size_t n) { return CoroFrameAlloc(n); }
   static void operator delete(void* p, size_t n) { CoroFrameFree(p, n); }
 
-  void init_tracking(Environment* env,
-                     const char* type_name,
-                     v8::Eternal<v8::String>& cached_type_name) {
+  void init_tracking(Environment* env, const char* type_name) {
     env_ = env;
     v8::Isolate* isolate = env->isolate();
     v8::HandleScope handle_scope(isolate);
@@ -217,14 +229,19 @@ struct TrackedPromiseBase : PromiseBase {
     }
 
     if (hooks->fields()[AsyncHooks::kInit] > 0) {
-      // Cache the type name V8 string per template instantiation.
-      // Eternal handles are immortal and free to dereference.
-      if (cached_type_name.IsEmpty()) {
-        cached_type_name.Set(
+      // Cache the type name string in the per-isolate static_str_map.
+      // The key is the const char* from ConstString, which is a unique
+      // pointer per template instantiation.  This gives us per-isolate
+      // caching that is safe with Worker threads (each Worker has its
+      // own Isolate and IsolateData).
+      auto& str_map = env->isolate_data()->static_str_map;
+      v8::Eternal<v8::String>& eternal = str_map[type_name];
+      if (eternal.IsEmpty()) {
+        eternal.Set(
             isolate,
             v8::String::NewFromUtf8(isolate, type_name).ToLocalChecked());
       }
-      v8::Local<v8::String> type = cached_type_name.Get(isolate);
+      v8::Local<v8::String> type = eternal.Get(isolate);
       AsyncWrap::EmitAsyncInit(
           env, resource_.Get(isolate), type, async_id_, trigger_async_id_);
     }
@@ -372,11 +389,7 @@ class UvTrackedTask {
   }
 
   void InitTracking(Environment* env) {
-    // Per-template-instantiation cache for the V8 type name string.
-    // Eternal handles are immortal, zero-cost to dereference, and
-    // safe across GC cycles.
-    static v8::Eternal<v8::String> cached_type_name;
-    handle_.promise().init_tracking(env, Name.c_str(), cached_type_name);
+    handle_.promise().init_tracking(env, Name.c_str());
   }
 
   void Start() {
@@ -449,8 +462,7 @@ class UvTrackedTask<void, Name> {
   void await_resume() { handle_.promise().rethrow_if_exception(); }
 
   void InitTracking(Environment* env) {
-    static v8::Eternal<v8::String> cached_type_name;
-    handle_.promise().init_tracking(env, Name.c_str(), cached_type_name);
+    handle_.promise().init_tracking(env, Name.c_str());
   }
 
   void Start() {