diff --git a/.changeset/curly-beans-exercise.md b/.changeset/curly-beans-exercise.md new file mode 100644 index 000000000..e987a6723 --- /dev/null +++ b/.changeset/curly-beans-exercise.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Improve composable cache performance diff --git a/packages/open-next/src/adapters/composable-cache.ts b/packages/open-next/src/adapters/composable-cache.ts index 5ed51478e..d6f1370b8 100644 --- a/packages/open-next/src/adapters/composable-cache.ts +++ b/packages/open-next/src/adapters/composable-cache.ts @@ -1,39 +1,24 @@ import type { ComposableCacheEntry, ComposableCacheHandler } from "types/cache"; -import type { CacheValue } from "types/overrides"; import { writeTags } from "utils/cache"; import { fromReadableStream, toReadableStream } from "utils/stream"; import { debug } from "./logger"; -const pendingWritePromiseMap = new Map< - string, - Promise> ->(); +const pendingWritePromiseMap = new Map>(); export default { async get(cacheKey: string) { try { - // We first check if we have a pending write for this cache key - // If we do, we return the pending promise instead of fetching the cache - if (pendingWritePromiseMap.has(cacheKey)) { - const stored = pendingWritePromiseMap.get(cacheKey); - if (stored) { - return stored.then((entry) => ({ - ...entry, - value: toReadableStream(entry.value), - })); - } - } + const stored = pendingWritePromiseMap.get(cacheKey); + if (stored) return stored; + const result = await globalThis.incrementalCache.get( cacheKey, "composable", ); - if (!result?.value?.value) { - return undefined; - } + if (!result?.value?.value) return undefined; debug("composable cache result", result); - // We need to check if the tags associated with this entry has been revalidated if ( globalThis.tagCache.mode === "nextMode" && result.value.tags.length > 0 @@ -69,73 +54,77 @@ export default { }, async set(cacheKey: string, pendingEntry: Promise) { - const promiseEntry = pendingEntry.then(async (entry) => ({ - ...entry, - value: await fromReadableStream(entry.value), - })); - pendingWritePromiseMap.set(cacheKey, promiseEntry); + const teedPromise = pendingEntry.then((entry) => { + // Optimization: We avoid consuming and stringifying the stream here, + // because it creates double copies just to be discarded when this function + // ends. This avoids unnecessary memory usage, and reduces GC pressure. + const [stream1, stream2] = entry.value.tee(); + return [ + { ...entry, value: stream1 }, + { ...entry, value: stream2 }, + ] as const; + }); - const entry = await promiseEntry.finally(() => { + pendingWritePromiseMap.set( + cacheKey, + teedPromise.then(([entry]) => entry), + ); + + const [, entryForStorage] = await teedPromise.finally(() => { pendingWritePromiseMap.delete(cacheKey); }); + await globalThis.incrementalCache.set( cacheKey, { - ...entry, - value: entry.value, + ...entryForStorage, + value: await fromReadableStream(entryForStorage.value), }, "composable", ); + if (globalThis.tagCache.mode === "original") { const storedTags = await globalThis.tagCache.getByPath(cacheKey); - const tagsToWrite = entry.tags.filter((tag) => !storedTags.includes(tag)); + const tagsToWrite = []; + for (const tag of entryForStorage.tags) { + if (!storedTags.includes(tag)) { + tagsToWrite.push({ tag, path: cacheKey }); + } + } if (tagsToWrite.length > 0) { - await writeTags(tagsToWrite.map((tag) => ({ tag, path: cacheKey }))); + await writeTags(tagsToWrite); } } }, - async refreshTags() { - // We don't do anything for now, do we want to do something here ??? - return; - }, + async refreshTags() {}, + async getExpiration(...tags: string[]) { - if (globalThis.tagCache.mode === "nextMode") { - return globalThis.tagCache.getLastRevalidated(tags); - } - // We always return 0 here, original tag cache are handled directly in the get part - // TODO: We need to test this more, i'm not entirely sure that this is working as expected - return 0; + return globalThis.tagCache.mode === "nextMode" + ? globalThis.tagCache.getLastRevalidated(tags) + : 0; }, + async expireTags(...tags: string[]) { if (globalThis.tagCache.mode === "nextMode") { return writeTags(tags); } + const tagCache = globalThis.tagCache; const revalidatedAt = Date.now(); - // For the original mode, we have more work to do here. - // We need to find all paths linked to to these tags const pathsToUpdate = await Promise.all( tags.map(async (tag) => { const paths = await tagCache.getByTag(tag); - return paths.map((path) => ({ - path, - tag, - revalidatedAt, - })); + return paths.map((path) => ({ path, tag, revalidatedAt })); }), ); - // We need to deduplicate paths, we use a set for that - const setToWrite = new Set<{ path: string; tag: string }>(); + + const dedupeMap = new Map(); for (const entry of pathsToUpdate.flat()) { - setToWrite.add(entry); + dedupeMap.set(`${entry.path}|${entry.tag}`, entry); } - await writeTags(Array.from(setToWrite)); + await writeTags(Array.from(dedupeMap.values())); }, - // This one is necessary for older versions of next - async receiveExpiredTags(...tags: string[]) { - // This function does absolutely nothing - return; - }, + async receiveExpiredTags() {}, } satisfies ComposableCacheHandler; diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index b66eda577..56e743ecd 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -108,7 +108,8 @@ export async function openNextHandler( overwrittenResponseHeaders[key] = value; } headers[key] = value; - delete headers[rawKey]; + // @ts-expect-error Setting it to undefined is faster than deleting the attribute. + headers[rawKey] = undefined; } if ( @@ -238,7 +239,7 @@ async function processRequest( // @ts-ignore // Next.js doesn't parse body if the property exists // https://github.com/dougmoscrop/serverless-http/issues/227 - delete req.body; + req.body = undefined; // Here we try to apply as much request metadata as possible // We apply every metadata from `resolve-routes` https://github.com/vercel/next.js/blob/916f105b97211de50f8580f0b39c9e7c60de4886/packages/next/src/server/lib/router-utils/resolve-routes.ts diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts index c27ba0ec8..f2980fc29 100644 --- a/packages/open-next/src/core/routing/util.ts +++ b/packages/open-next/src/core/routing/util.ts @@ -150,15 +150,9 @@ export function convertToQueryString(query: Record) { * @__PURE__ */ export function convertToQuery(querystring: string) { + if (!querystring) return {}; const query = new URLSearchParams(querystring); - const queryObject: Record = {}; - - for (const key of query.keys()) { - const queries = query.getAll(key); - queryObject[key] = queries.length > 1 ? queries : queries[0]; - } - - return queryObject; + return getQueryFromIterator(query.entries()); } /** diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 951be07c5..4e83aefb8 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -93,7 +93,8 @@ export default async function routingHandler( key.startsWith(INTERNAL_HEADER_PREFIX) || key.startsWith(MIDDLEWARE_HEADER_PREFIX) ) { - delete event.headers[key]; + // @ts-expect-error Assigning undefined is faster than deleting the attribute. + event.headers[key] = undefined; } } diff --git a/packages/open-next/src/http/openNextResponse.ts b/packages/open-next/src/http/openNextResponse.ts index 4a521e760..51981828d 100644 --- a/packages/open-next/src/http/openNextResponse.ts +++ b/packages/open-next/src/http/openNextResponse.ts @@ -128,7 +128,7 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse { if (key === SET_COOKIE_HEADER) { this._cookies = []; } else { - delete this.headers[key]; + this.headers[key] = undefined; } return this; } @@ -188,10 +188,6 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse { const parsedHeaders = parseHeaders(this.headers); - // We need to remove the set-cookie header from the parsed headers because - // it does not handle multiple set-cookie headers properly - delete parsedHeaders[SET_COOKIE_HEADER]; - if (this.streamCreator) { this.responseStream = this.streamCreator?.writeHeaders({ statusCode: this.statusCode ?? 200, diff --git a/packages/open-next/src/http/util.ts b/packages/open-next/src/http/util.ts index de3b07de6..71ec2ffa4 100644 --- a/packages/open-next/src/http/util.ts +++ b/packages/open-next/src/http/util.ts @@ -14,6 +14,12 @@ export const parseHeaders = ( continue; } const keyLower = key.toLowerCase(); + if (keyLower === "set-cookie") { + // We need to remove the set-cookie header from the parsed headers because + // it does not handle multiple set-cookie headers properly + continue; + } + /** * Next can return an Array for the Location header when you return null from a get in the cacheHandler on a page that has a redirect() * We dont want to merge that into a comma-separated string @@ -22,7 +28,7 @@ export const parseHeaders = ( * See: https://github.com/opennextjs/opennextjs-cloudflare/issues/875#issuecomment-3258248276 * and https://github.com/opennextjs/opennextjs-aws/pull/977#issuecomment-3261763114 */ - if (keyLower === "location" && Array.isArray(value)) { + if (Array.isArray(value) && keyLower === "location") { if (value[0] === value[1]) { result[keyLower] = value[0]; } else { diff --git a/packages/open-next/src/overrides/converters/aws-apigw-v2.ts b/packages/open-next/src/overrides/converters/aws-apigw-v2.ts index 15cf8199d..af161e9ab 100644 --- a/packages/open-next/src/overrides/converters/aws-apigw-v2.ts +++ b/packages/open-next/src/overrides/converters/aws-apigw-v2.ts @@ -64,8 +64,10 @@ function normalizeAPIGatewayProxyEventV2Headers( headers.cookie = cookies.join("; "); } - for (const [key, value] of Object.entries(rawHeaders || {})) { - headers[key.toLowerCase()] = value!; + if (rawHeaders) { + for (const [key, value] of Object.entries(rawHeaders)) { + headers[key.toLowerCase()] = value!; + } } return headers; diff --git a/packages/open-next/src/overrides/converters/aws-cloudfront.ts b/packages/open-next/src/overrides/converters/aws-cloudfront.ts index 322f0071e..119c707fd 100644 --- a/packages/open-next/src/overrides/converters/aws-cloudfront.ts +++ b/packages/open-next/src/overrides/converters/aws-cloudfront.ts @@ -70,9 +70,10 @@ function normalizeCloudFrontRequestEventHeaders( const headers: Record = {}; for (const [key, values] of Object.entries(rawHeaders)) { + const lowerKey = key.toLowerCase(); for (const { value } of values) { if (value) { - headers[key.toLowerCase()] = value; + headers[lowerKey] = value; } } } diff --git a/packages/open-next/src/overrides/converters/edge.ts b/packages/open-next/src/overrides/converters/edge.ts index af4d6d11d..47b09c7fa 100644 --- a/packages/open-next/src/overrides/converters/edge.ts +++ b/packages/open-next/src/overrides/converters/edge.ts @@ -24,8 +24,6 @@ const converter: Converter = { const searchParams = url.searchParams; const query = getQueryFromSearchParams(searchParams); - // Transform body into Buffer - const body = await event.arrayBuffer(); const headers: Record = {}; event.headers.forEach((value, key) => { headers[key] = value; @@ -34,6 +32,11 @@ const converter: Converter = { const method = event.method; const shouldHaveBody = method !== "GET" && method !== "HEAD"; + // Only read body for methods that should have one + const body = shouldHaveBody + ? Buffer.from(await event.arrayBuffer()) + : undefined; + const cookieHeader = event.headers.get("cookie"); const cookies = cookieHeader ? (cookieParser.parse(cookieHeader) as Record) @@ -44,7 +47,7 @@ const converter: Converter = { method, rawPath, url: event.url, - body: shouldHaveBody ? Buffer.from(body) : undefined, + body, headers, remoteAddress: event.headers.get("x-forwarded-for") ?? "::1", query, diff --git a/packages/open-next/src/overrides/converters/node.ts b/packages/open-next/src/overrides/converters/node.ts index 82f784191..f959bf60a 100644 --- a/packages/open-next/src/overrides/converters/node.ts +++ b/packages/open-next/src/overrides/converters/node.ts @@ -8,12 +8,28 @@ import { extractHostFromHeaders, getQueryFromSearchParams } from "./utils.js"; const converter: Converter = { convertFrom: async (req: IncomingMessage & { protocol?: string }) => { const body = await new Promise((resolve) => { + const contentLength = req.headers["content-length"]; + const expectedLength = contentLength + ? Number.parseInt(contentLength, 10) + : undefined; const chunks: Uint8Array[] = []; + let receivedLength = 0; + req.on("data", (chunk) => { chunks.push(chunk); + receivedLength += chunk.length; }); req.on("end", () => { - resolve(Buffer.concat(chunks)); + // Use pre-allocated buffer if we have content-length and it matches + if ( + expectedLength && + receivedLength === expectedLength && + chunks.length === 1 + ) { + resolve(Buffer.from(chunks[0])); + } else { + resolve(Buffer.concat(chunks)); + } }); }); diff --git a/packages/open-next/src/overrides/originResolver/pattern-env.ts b/packages/open-next/src/overrides/originResolver/pattern-env.ts index 24308c59a..e33225a46 100644 --- a/packages/open-next/src/overrides/originResolver/pattern-env.ts +++ b/packages/open-next/src/overrides/originResolver/pattern-env.ts @@ -3,34 +3,71 @@ import type { OriginResolver } from "types/overrides"; import { debug, error } from "../../adapters/logger"; +// Cache parsed origin and compiled patterns at module level +let cachedOrigin: Record | null = null; +const cachedPatterns: Array<{ + key: string; + patterns: string[]; + regexes: RegExp[]; +}> = []; +let initialized = false; + +function initialize() { + if (initialized) return; + + // Parse origin JSON once + cachedOrigin = JSON.parse(process.env.OPEN_NEXT_ORIGIN ?? "{}") as Record< + string, + Origin + >; + + // Pre-compile all regex patterns + const functions = globalThis.openNextConfig.functions ?? {}; + for (const key in functions) { + if (key !== "default") { + const value = functions[key]; + const regexes: RegExp[] = []; + + for (const pattern of value.patterns) { + // Convert cloudfront pattern to regex + const regexPattern = `/${pattern + .replace(/\*\*/g, "(.*)") + .replace(/\*/g, "([^/]*)") + .replace(/\//g, "\\/") + .replace(/\?/g, ".")}`; + regexes.push(new RegExp(regexPattern)); + } + + cachedPatterns.push({ + key, + patterns: value.patterns, + regexes, + }); + } + } + + initialized = true; +} + const envLoader: OriginResolver = { name: "env", resolve: async (_path: string) => { try { - const origin = JSON.parse(process.env.OPEN_NEXT_ORIGIN ?? "{}") as Record< - string, - Origin - >; - for (const [key, value] of Object.entries( - globalThis.openNextConfig.functions ?? {}, - ).filter(([key]) => key !== "default")) { - if ( - value.patterns.some((pattern) => { - // Convert cloudfront pattern to regex - return new RegExp( - // transform glob pattern to regex - `/${pattern - .replace(/\*\*/g, "(.*)") - .replace(/\*/g, "([^/]*)") - .replace(/\//g, "\\/") - .replace(/\?/g, ".")}`, - ).test(_path); - }) - ) { - debug("Using origin", key, value.patterns); - return origin[key]; + initialize(); + + // Use cached origin + const origin = cachedOrigin!; + + // Test against pre-compiled patterns + for (const { key, patterns, regexes } of cachedPatterns) { + for (const regex of regexes) { + if (regex.test(_path)) { + debug("Using origin", key, patterns); + return origin[key]; + } } } + if (_path.startsWith("/_next/image") && origin.imageOptimizer) { debug("Using origin", "imageOptimizer", _path); return origin.imageOptimizer; diff --git a/packages/open-next/src/overrides/wrappers/cloudflare-node.ts b/packages/open-next/src/overrides/wrappers/cloudflare-node.ts index b3d8b374e..6cc46d6a7 100644 --- a/packages/open-next/src/overrides/wrappers/cloudflare-node.ts +++ b/packages/open-next/src/overrides/wrappers/cloudflare-node.ts @@ -41,30 +41,61 @@ const handler: WrapperHandler = }): Writable { const { statusCode, cookies, headers } = prelude; - const responseHeaders = new Headers(headers); - for (const cookie of cookies) { - responseHeaders.append("Set-Cookie", cookie); - } + headers["set-cookie"] = cookies.join(","); // TODO(vicb): this is a workaround to make PPR work with `wrangler dev` // See https://github.com/cloudflare/workers-sdk/issues/8004 if (url.hostname === "localhost") { - responseHeaders.set("Content-Encoding", "identity"); + headers["content-encoding"] = "identity"; } - const { readable, writable } = new TransformStream({ - transform(chunk, controller) { - controller.enqueue(Uint8Array.from(chunk.chunk ?? chunk)); + // Optimize: skip ReadableStream creation for null body statuses + if (NULL_BODY_STATUSES.has(statusCode)) { + const response = new Response(null, { + status: statusCode, + headers, + }); + resolveResponse(response); + + // Return a no-op Writable that discards all data + return new Writable({ + write(chunk, encoding, callback) { + callback(); + }, + }); + } + + let controller: ReadableStreamDefaultController; + const readable = new ReadableStream({ + start(c) { + controller = c; }, }); - const body = NULL_BODY_STATUSES.has(statusCode) ? null : readable; - const response = new Response(body, { + + const response = new Response(readable, { status: statusCode, - headers: responseHeaders, + headers, }); resolveResponse(response); - return Writable.fromWeb(writable); + return new Writable({ + write(chunk, encoding, callback) { + controller.enqueue(chunk); + callback(); + }, + final(callback) { + controller.close(); + callback(); + }, + destroy(error, callback) { + if (error) { + controller.error(error); + } else { + controller.close(); + } + callback(error); + }, + }); }, // This is for passing along the original abort signal from the initial Request you retrieve in your worker // Ensures that the response we pass to NextServer is aborted if the request is aborted diff --git a/packages/open-next/src/utils/stream.ts b/packages/open-next/src/utils/stream.ts index 52e834f63..a30f51f51 100644 --- a/packages/open-next/src/utils/stream.ts +++ b/packages/open-next/src/utils/stream.ts @@ -20,13 +20,9 @@ export async function fromReadableStream( return Buffer.from(chunks[0]).toString(base64 ? "base64" : "utf8"); } - // Pre-allocate buffer with exact size to avoid reallocation - const buffer = Buffer.alloc(totalLength); - let offset = 0; - for (const chunk of chunks) { - buffer.set(chunk, offset); - offset += chunk.length; - } + // Use Buffer.concat which is more efficient than manual allocation and copy + // It handles the allocation and copy in optimized native code + const buffer = Buffer.concat(chunks, totalLength); return buffer.toString(base64 ? "base64" : "utf8"); }