Repository: denoland/deno
Pull request: perf(ext/http): recover memory for serve and optimize AbortController

Max rps without a signal is unchanged, however we can drastically reduce memory usage by not creating the signal until needed, and we can optimize the rps in the case where the signal is created.

With a quick memory benchmark, it looks like this helps pretty drastically with # of GCs when benchmarking w/wrk:

 - 1.42.4: 1763
 - canary: 1093
 - this patch: 874

This branch:
```

Running 10s test @ http://localhost:8080/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    87.33us  439.95us  20.68ms   99.67%
    Req/Sec    66.70k     6.39k   74.11k    83.66%
  1340255 requests in 10.10s, 191.73MB read
Requests/sec: 132696.90
Transfer/sec:     18.98MB

cpu: Apple M2 Pro
runtime: deno 1.43.0 (aarch64-apple-darwin)

file:///Users/matt/Documents/scripts/bench_request.js
benchmark                                      time (avg)        iter/s             (min … max)       p75       p99      p995
----------------------------------------------------------------------------------------------- -----------------------------
newRequest                                     986.5 ns/iter   1,013,682.6    (878.2 ns … 1.18 µs) 1.01 µs 1.18 µs 1.18 µs
newAbortController                                18 ns/iter  55,541,104.1    (15.6 ns … 42.62 ns) 17.71 ns 25.05 ns 26.27 ns
newAbortControllerSignal                       18.66 ns/iter  53,578,966.7   (16.49 ns … 32.16 ns) 18.71 ns 25.67 ns 26.39 ns
newAbortControllerSignalOnAbort               106.49 ns/iter   9,390,164.9  (97.87 ns … 120.61 ns) 108.6 ns 114.24 ns 115.89 ns
newAbortControllerSignalAddEventListener       86.92 ns/iter  11,504,880.2  (81.88 ns … 103.15 ns) 90 ns 98.28 ns 99.55 ns
newAbortControllerSignalOnAbortNoListener       3.01 µs/iter     331,964.4      (2.97 µs … 3.1 µs) 3.06 µs 3.1 µs 3.1 µs
newAbortControllerSignalOnAbortAbort            3.26 µs/iter     306,662.6     (3.22 µs … 3.36 µs) 3.27 µs 3.36 µs 3.36 µs


```

Latest canary:
```
Running 10s test @ http://localhost:8080/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    72.86us   71.23us   4.47ms   99.05%
    Req/Sec    64.66k     5.54k   72.48k    82.18%
  1299015 requests in 10.10s, 185.83MB read
Requests/sec: 128616.02
Transfer/sec:     18.40MB


cpu: Apple M2 Pro
runtime: deno 1.43.0+bc4aa5f (aarch64-apple-darwin)

file:///Users/matt/Documents/scripts/bench_request.js
benchmark                                      time (avg)        iter/s             (min … max)       p75       p99      p995
----------------------------------------------------------------------------------------------- -----------------------------
newRequest                                      1.25 µs/iter     800,005.2     (1.01 µs … 4.18 µs) 1.16 µs 4.18 µs 4.18 µs
newAbortController                             18.56 ns/iter  53,868,204.3   (16.04 ns … 38.73 ns) 18.38 ns 26.1 ns 26.63 ns
newAbortControllerSignal                       18.72 ns/iter  53,430,746.1   (16.13 ns … 36.71 ns) 18.71 ns 26.19 ns 26.98 ns
newAbortControllerSignalOnAbort               193.91 ns/iter   5,156,992.4 (184.25 ns … 211.41 ns) 194.96 ns 207.87 ns 209.4 ns
newAbortControllerSignalAddEventListener      171.45 ns/iter   5,832,569.2    (153 ns … 182.03 ns) 176.17 ns 180.75 ns 181.05 ns
newAbortControllerSignalOnAbortNoListener       3.07 µs/iter     326,263.3     (2.98 µs … 3.17 µs) 3.08 µs 3.17 µs 3.17 µs
newAbortControllerSignalOnAbortAbort            3.32 µs/iter     301,344.6      (3.29 µs … 3.4 µs) 3.33 µs 3.4 µs 3.4 µs
```



Changed files:
filename: ext/fetch/23_request.js
patch: @@ -14,6 +14,7 @@ const {
   ArrayPrototypeMap,
   ArrayPrototypeSlice,
   ArrayPrototypeSplice,
+  ObjectFreeze,
   ObjectKeys,
   ObjectPrototypeIsPrototypeOf,
   RegExpPrototypeExec,
@@ -24,7 +25,6 @@ const {
 } = primordials;
 
 import * as webidl from "ext:deno_webidl/00_webidl.js";
-import { assert } from "ext:deno_web/00_infra.js";
 import { createFilteredInspectProxy } from "ext:deno_console/01_console.js";
 import {
   byteUpperCase,
@@ -43,15 +43,20 @@ import {
   headersFromHeaderList,
 } from "ext:deno_fetch/20_headers.js";
 import { HttpClientPrototype } from "ext:deno_fetch/22_http_client.js";
-import * as abortSignal from "ext:deno_web/03_abort_signal.js";
-
+import {
+  createDependentAbortSignal,
+  newSignal,
+  signalAbort,
+} from "ext:deno_web/03_abort_signal.js";
+import { DOMException } from "ext:deno_web/01_dom_exception.js";
 const { internalRidSymbol } = core;
 
 const _request = Symbol("request");
 const _headers = Symbol("headers");
 const _getHeaders = Symbol("get headers");
 const _headersCache = Symbol("headers cache");
 const _signal = Symbol("signal");
+const _signalCache = Symbol("signalCache");
 const _mimeType = Symbol("mime type");
 const _body = Symbol("body");
 const _url = Symbol("url");
@@ -262,7 +267,13 @@ class Request {
   }
 
   /** @type {AbortSignal} */
-  [_signal];
+  get [_signal]() {
+    const signal = this[_signalCache];
+    if (signal !== undefined) {
+      return signal;
+    }
+    return (this[_signalCache] = newSignal());
+  }
   get [_mimeType]() {
     const values = getDecodeSplitHeader(
       headerListFromHeaders(this[_headers]),
@@ -363,11 +374,10 @@ class Request {
     // 28.
     this[_request] = request;
 
-    // 29.
-    const signals = signal !== null ? [signal] : [];
-
-    // 30.
-    this[_signal] = abortSignal.createDependentAbortSignal(signals, prefix);
+    // 29 & 30.
+    if (signal !== null) {
+      this[_signalCache] = createDependentAbortSignal([signal], prefix);
+    }
 
     // 31.
     this[_headers] = headersFromHeaderList(request.headerList, "request");
@@ -473,17 +483,21 @@ class Request {
     }
     const clonedReq = cloneInnerRequest(this[_request]);
 
-    assert(this[_signal] !== null);
-    const clonedSignal = abortSignal.createDependentAbortSignal(
-      [this[_signal]],
+    const materializedSignal = this[_signal];
+    const clonedSignal = createDependentAbortSignal(
+      [materializedSignal],
       prefix,
     );
 
-    return fromInnerRequest(
-      clonedReq,
-      clonedSignal,
-      guardFromHeaders(this[_headers]),
-    );
+    const request = new Request(_brand);
+    request[_request] = clonedReq;
+    request[_signalCache] = clonedSignal;
+    request[_getHeaders] = () =>
+      headersFromHeaderList(
+        clonedReq.headerList,
+        guardFromHeaders(this[_headers]),
+      );
+    return request;
   }
 
   [SymbolFor("Deno.privateCustomInspect")](inspect, inspectOptions) {
@@ -562,19 +576,30 @@ function toInnerRequest(request) {
 
 /**
  * @param {InnerRequest} inner
- * @param {AbortSignal} signal
  * @param {"request" | "immutable" | "request-no-cors" | "response" | "none"} guard
  * @returns {Request}
  */
-function fromInnerRequest(inner, signal, guard) {
+function fromInnerRequest(inner, guard) {
   const request = new Request(_brand);
   request[_request] = inner;
-  request[_signal] = signal;
   request[_getHeaders] = () => headersFromHeaderList(inner.headerList, guard);
   return request;
 }
 
+const signalAbortError = new DOMException(
+  "The request has been cancelled.",
+  "AbortError",
+);
+ObjectFreeze(signalAbortError);
+
+function abortRequest(request) {
+  if (request[_signal]) {
+    request[_signal][signalAbort](signalAbortError);
+  }
+}
+
 export {
+  abortRequest,
   fromInnerRequest,
   newInnerRequest,
   processUrlList,

filename: ext/fetch/internal.d.ts
patch: @@ -70,7 +70,6 @@ declare module "ext:deno_fetch/26_fetch.js" {
   function toInnerRequest(request: Request): InnerRequest;
   function fromInnerRequest(
     inner: InnerRequest,
-    signal: AbortSignal | null,
     guard:
       | "request"
       | "immutable"

filename: ext/http/00_serve.ts
patch: @@ -49,7 +49,11 @@ import {
   ResponsePrototype,
   toInnerResponse,
 } from "ext:deno_fetch/23_response.js";
-import { fromInnerRequest, toInnerRequest } from "ext:deno_fetch/23_request.js";
+import {
+  abortRequest,
+  fromInnerRequest,
+  toInnerRequest,
+} from "ext:deno_fetch/23_request.js";
 import { AbortController } from "ext:deno_web/03_abort_signal.js";
 import {
   _eventLoop,
@@ -126,8 +130,6 @@ function addTrailers(resp, headerList) {
   op_http_set_response_trailers(inner.external, headerList);
 }
 
-let signalAbortError;
-
 class InnerRequest {
   #external;
   #context;
@@ -137,14 +139,13 @@ class InnerRequest {
   #upgraded;
   #urlValue;
   #completed;
-  #abortController;
+  request;
 
-  constructor(external, context, abortController) {
+  constructor(external, context) {
     this.#external = external;
     this.#context = context;
     this.#upgraded = false;
     this.#completed = undefined;
-    this.#abortController = abortController;
   }
 
   close(success = true) {
@@ -158,15 +159,7 @@ class InnerRequest {
         );
       }
     }
-    if (!signalAbortError) {
-      signalAbortError = new DOMException(
-        "The request has been cancelled.",
-        "AbortError",
-      );
-    }
-    // Unconditionally abort the request signal. Note that we don't use
-    // an error here.
-    this.#abortController.abort(signalAbortError);
+    abortRequest(this.request);
     this.#external = null;
   }
 
@@ -492,17 +485,16 @@ function fastSyncResponseOrStream(
  */
 function mapToCallback(context, callback, onError) {
   return async function (req) {
-    const abortController = new AbortController();
-    const signal = abortController.signal;
-
     // Get the response from the user-provided callback. If that fails, use onError. If that fails, return a fallback
     // 500 error.
     let innerRequest;
     let response;
     try {
-      innerRequest = new InnerRequest(req, context, abortController);
+      innerRequest = new InnerRequest(req, context);
+      const request = fromInnerRequest(innerRequest, "immutable");
+      innerRequest.request = request;
       response = await callback(
-        fromInnerRequest(innerRequest, signal, "immutable"),
+        request,
         new ServeHandlerInfo(innerRequest),
       );
 

filename: ext/http/01_http.js
patch: @@ -38,10 +38,10 @@ import {
   toInnerResponse,
 } from "ext:deno_fetch/23_response.js";
 import {
+  abortRequest,
   fromInnerRequest,
   newInnerRequest,
 } from "ext:deno_fetch/23_request.js";
-import { AbortController } from "ext:deno_web/03_abort_signal.js";
 import {
   _eventLoop,
   _idleTimeoutDuration,
@@ -147,19 +147,17 @@ class HttpConn {
       body !== null ? new InnerBody(body) : null,
       false,
     );
-    const abortController = new AbortController();
     const request = fromInnerRequest(
       innerRequest,
-      abortController.signal,
       "immutable",
       false,
     );
 
     const respondWith = createRespondWith(
       this,
+      request,
       readStreamRid,
       writeStreamRid,
-      abortController,
     );
 
     return { request, respondWith };
@@ -200,9 +198,9 @@ class HttpConn {
 
 function createRespondWith(
   httpConn,
+  request,
   readStreamRid,
   writeStreamRid,
-  abortController,
 ) {
   return async function respondWith(resp) {
     try {
@@ -384,7 +382,7 @@ function createRespondWith(
         ws[_serverHandleIdleTimeout]();
       }
     } catch (error) {
-      abortController.abort(error);
+      abortRequest(request);
       throw error;
     } finally {
       if (deleteManagedResource(httpConn, readStreamRid)) {

filename: ext/web/03_abort_signal.js
patch: @@ -7,8 +7,8 @@ import { primordials } from "ext:core/mod.js";
 const {
   ArrayPrototypeEvery,
   ArrayPrototypePush,
+  FunctionPrototypeApply,
   ObjectPrototypeIsPrototypeOf,
-  SafeArrayIterator,
   SafeSet,
   SafeSetIterator,
   SafeWeakRef,
@@ -82,6 +82,14 @@ const timerId = Symbol("[[timerId]]");
 const illegalConstructorKey = Symbol("illegalConstructorKey");
 
 class AbortSignal extends EventTarget {
+  [abortReason] = undefined;
+  [abortAlgos] = null;
+  [dependent] = false;
+  [sourceSignals] = null;
+  [dependentSignals] = null;
+  [timerId] = null;
+  [webidl.brand] = webidl.brand;
+
   static any(signals) {
     const prefix = "Failed to execute 'AbortSignal.any'";
     webidl.requiredArguments(arguments.length, 1, prefix);
@@ -141,9 +149,11 @@ class AbortSignal extends EventTarget {
     const algos = this[abortAlgos];
     this[abortAlgos] = null;
 
-    const event = new Event("abort");
-    setIsTrusted(event, true);
-    super.dispatchEvent(event);
+    if (listenerCount(this, "abort") > 0) {
+      const event = new Event("abort");
+      setIsTrusted(event, true);
+      super.dispatchEvent(event);
+    }
     if (algos !== null) {
       for (const algorithm of new SafeSetIterator(algos)) {
         algorithm();
@@ -168,13 +178,6 @@ class AbortSignal extends EventTarget {
       throw new TypeError("Illegal constructor.");
     }
     super();
-    this[abortReason] = undefined;
-    this[abortAlgos] = null;
-    this[dependent] = false;
-    this[sourceSignals] = null;
-    this[dependentSignals] = null;
-    this[timerId] = null;
-    this[webidl.brand] = webidl.brand;
   }
 
   get aborted() {
@@ -199,8 +202,8 @@ class AbortSignal extends EventTarget {
   // `[add]` and `[remove]` don't ref and unref the timer because they can
   // only be used by Deno internals, which use it to essentially cancel async
   // ops which would block the event loop.
-  addEventListener(...args) {
-    super.addEventListener(...new SafeArrayIterator(args));
+  addEventListener() {
+    FunctionPrototypeApply(super.addEventListener, this, arguments);
     if (listenerCount(this, "abort") > 0) {
       if (this[timerId] !== null) {
         refTimer(this[timerId]);
@@ -216,8 +219,8 @@ class AbortSignal extends EventTarget {
     }
   }
 
-  removeEventListener(...args) {
-    super.removeEventListener(...new SafeArrayIterator(args));
+  removeEventListener() {
+    FunctionPrototypeApply(super.removeEventListener, this, arguments);
     if (listenerCount(this, "abort") === 0) {
       if (this[timerId] !== null) {
         unrefTimer(this[timerId]);