perf(ext/http): recover memory for serve and optimize AbortController

ext/fetch/23_request.js

@@ -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,

ext/fetch/internal.d.ts

@@ -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"

ext/http/00_serve.ts

@@ -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),
       );
 

ext/http/01_http.js

@@ -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)) {

ext/web/03_abort_signal.js

@@ -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]);