Skip to content

Commit 3b61c41

Browse files
dario-piotrowiczdario-piotrowicz
authored
performance improvement: restart a mixed mode session only if the worker's remote bindings have changed (#9536)
1 parent fb88a83 commit 3b61c41

File tree

8 files changed

+131
-64
lines changed

8 files changed

+131
-64
lines changed

‎.changeset/cool-peas-march.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
expose `Unstable_Binding` type

‎.changeset/tall-insects-read.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@cloudflare/vite-plugin": patch
3+
"@cloudflare/vitest-pool-workers": patch
4+
"wrangler": patch
5+
---
6+
7+
performance improvement: restart a mixed mode session only if the worker's remote bindings have changed

‎packages/vite-plugin-cloudflare/src/miniflare-options.ts‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import type { FetchFunctionOptions } from "vite/module-runner";
3737
import type {
3838
Experimental_MixedModeSession,
3939
SourcelessWorkerOptions,
40+
Unstable_Binding,
4041
Unstable_Config,
4142
} from "wrangler";
4243

@@ -218,10 +219,13 @@ function logUnknownTails(
218219
}
219220
}
220221

221-
/** Map that maps worker configPaths to their existing mixed mode session (if any) */
222-
const mixedModeSessionsMap = new Map<
222+
/** Map that maps worker configPaths to their existing mixed mode session data (if any) */
223+
const mixedModeSessionsDataMap = new Map<
223224
string,
224-
Experimental_MixedModeSession | null
225+
{
226+
session: Experimental_MixedModeSession;
227+
remoteBindings: Record<string, Unstable_Binding>;
228+
} | null
225229
>();
226230

227231
export async function getDevMiniflareOptions(
@@ -323,10 +327,10 @@ export async function getDevMiniflareOptions(
323327
);
324328

325329
const preExistingMixedModeSession = workerConfig.configPath
326-
? mixedModeSessionsMap.get(workerConfig.configPath)
330+
? mixedModeSessionsDataMap.get(workerConfig.configPath)
327331
: undefined;
328332

329-
const mixedModeSession = resolvedPluginConfig.experimental
333+
const mixedModeSessionData = resolvedPluginConfig.experimental
330334
.mixedMode
331335
? await experimental_maybeStartOrUpdateMixedModeSession(
332336
{
@@ -337,10 +341,10 @@ export async function getDevMiniflareOptions(
337341
)
338342
: undefined;
339343

340-
if (workerConfig.configPath && mixedModeSession) {
341-
mixedModeSessionsMap.set(
344+
if (workerConfig.configPath && mixedModeSessionData) {
345+
mixedModeSessionsDataMap.set(
342346
workerConfig.configPath,
343-
mixedModeSession
347+
mixedModeSessionData
344348
);
345349
}
346350

@@ -352,7 +356,7 @@ export async function getDevMiniflareOptions(
352356
resolvedPluginConfig.cloudflareEnv,
353357
{
354358
mixedModeConnectionString:
355-
mixedModeSession?.mixedModeConnectionString,
359+
mixedModeSessionData?.session?.mixedModeConnectionString,
356360
mixedModeEnabled: resolvedPluginConfig.experimental.mixedMode,
357361
}
358362
);
@@ -628,30 +632,33 @@ export async function getPreviewMiniflareOptions(
628632
const bindings =
629633
unstable_convertConfigBindingsToStartWorkerBindings(workerConfig);
630634

631-
const preExistingMixedModeSession = workerConfig.configPath
632-
? mixedModeSessionsMap.get(workerConfig.configPath)
635+
const preExistingMixedModeSessionData = workerConfig.configPath
636+
? mixedModeSessionsDataMap.get(workerConfig.configPath)
633637
: undefined;
634638

635-
const mixedModeSession = mixedModeEnabled
639+
const mixedModeSessionData = mixedModeEnabled
636640
? await experimental_maybeStartOrUpdateMixedModeSession(
637641
{
638642
name: workerConfig.name,
639643
bindings: bindings ?? {},
640644
},
641-
preExistingMixedModeSession ?? null
645+
preExistingMixedModeSessionData ?? null
642646
)
643647
: undefined;
644648

645-
if (workerConfig.configPath && mixedModeSession) {
646-
mixedModeSessionsMap.set(workerConfig.configPath, mixedModeSession);
649+
if (workerConfig.configPath && mixedModeSessionData) {
650+
mixedModeSessionsDataMap.set(
651+
workerConfig.configPath,
652+
mixedModeSessionData
653+
);
647654
}
648655

649656
const miniflareWorkerOptions = unstable_getMiniflareWorkerOptions(
650657
workerConfig,
651658
undefined,
652659
{
653660
mixedModeConnectionString:
654-
mixedModeSession?.mixedModeConnectionString,
661+
mixedModeSessionData?.session?.mixedModeConnectionString,
655662
mixedModeEnabled,
656663
}
657664
);

‎packages/vitest-pool-workers/src/pool/config.ts‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { getProjectPath, getRelativeProjectPath } from "./helpers";
1313
import type { ModuleRule, WorkerOptions } from "miniflare";
1414
import type { ProvidedContext } from "vitest";
1515
import type { WorkspaceProject } from "vitest/node";
16-
import type { Experimental_MixedModeSession } from "wrangler";
16+
import type { Experimental_MixedModeSession, Unstable_Binding } from "wrangler";
1717
import type { ParseParams, ZodError } from "zod";
1818

1919
export interface WorkersConfigPluginAPI {
@@ -177,10 +177,13 @@ function filterTails(
177177
});
178178
}
179179

180-
/** Map that maps worker configPaths to their existing mixed mode session (if any) */
181-
const mixedModeSessionsMap = new Map<
180+
/** Map that maps worker configPaths to their existing mixed mode session data (if any) */
181+
const mixedModeSessionsDataMap = new Map<
182182
string,
183-
Experimental_MixedModeSession | null
183+
{
184+
session: Experimental_MixedModeSession;
185+
remoteBindings: Record<string, Unstable_Binding>;
186+
} | null
184187
>();
185188

186189
async function parseCustomPoolOptions(
@@ -247,19 +250,22 @@ async function parseCustomPoolOptions(
247250
// Lazily import `wrangler` if and when we need it
248251
const wrangler = await import("wrangler");
249252

250-
const preExistingMixedModeSession = options.wrangler?.configPath
251-
? mixedModeSessionsMap.get(options.wrangler.configPath)
253+
const preExistingMixedModeSessionData = options.wrangler?.configPath
254+
? mixedModeSessionsDataMap.get(options.wrangler.configPath)
252255
: undefined;
253256

254-
const mixedModeSession = options.experimental_mixedMode
257+
const mixedModeSessionData = options.experimental_mixedMode
255258
? await wrangler.experimental_maybeStartOrUpdateMixedModeSession(
256259
configPath,
257-
preExistingMixedModeSession ?? null
260+
preExistingMixedModeSessionData ?? null
258261
)
259-
: undefined;
262+
: null;
260263

261-
if (options.wrangler?.configPath && mixedModeSession) {
262-
mixedModeSessionsMap.set(options.wrangler.configPath, mixedModeSession);
264+
if (options.wrangler?.configPath && mixedModeSessionData) {
265+
mixedModeSessionsDataMap.set(
266+
options.wrangler.configPath,
267+
mixedModeSessionData
268+
);
263269
}
264270

265271
const { workerOptions, externalWorkers, define, main } =
@@ -270,7 +276,7 @@ async function parseCustomPoolOptions(
270276
imagesLocalMode: true,
271277
overrides: { assets: options.miniflare.assets },
272278
mixedModeConnectionString:
273-
mixedModeSession?.mixedModeConnectionString,
279+
mixedModeSessionData?.session?.mixedModeConnectionString,
274280
}
275281
);
276282

‎packages/wrangler/src/api/mixedMode/index.ts‎

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,25 @@ export function pickRemoteBindings(
106106
*
107107
* @param configPathOrWorkerConfig either a file path to a wrangler configuration file or an object containing the name of
108108
* the target worker alongside its bindings.
109-
* @param preExistingMixedModeSession an pre-existing mixed mode session to use or null if there is no such session
109+
* @param preExistingMixedModeSessionData the data of a pre-existing mixed mode session if there was one null otherwise
110110
* @returns null if no existing mixed mode session was provided and one should not be created (because the worker is not
111-
* defining any remote bindings), the created/updated mixed mode session otherwise.
111+
* defining any remote bindings), the data associated to the created/updated mixed mode session otherwise.
112112
*/
113113
export async function maybeStartOrUpdateMixedModeSession(
114114
configPathOrWorkerConfig:
115115
| string
116-
| { name?: string; bindings: NonNullable<StartDevWorkerInput["bindings"]> },
117-
preExistingMixedModeSession: MixedModeSession | null
118-
): Promise<MixedModeSession | null> {
116+
| {
117+
name?: string;
118+
bindings: NonNullable<StartDevWorkerInput["bindings"]>;
119+
},
120+
preExistingMixedModeSessionData: {
121+
session: MixedModeSession;
122+
remoteBindings: Record<string, Binding>;
123+
} | null
124+
): Promise<{
125+
session: MixedModeSession;
126+
remoteBindings: Record<string, Binding>;
127+
} | null> {
119128
if (typeof configPathOrWorkerConfig === "string") {
120129
const configPath = configPathOrWorkerConfig;
121130
const config = readConfig({ config: configPath });
@@ -129,25 +138,44 @@ export async function maybeStartOrUpdateMixedModeSession(
129138
}
130139
const workerConfigs = configPathOrWorkerConfig;
131140

132-
const workerRemoteBindings = pickRemoteBindings(workerConfigs.bindings);
141+
const remoteBindings = pickRemoteBindings(workerConfigs.bindings);
133142

134-
let mixedModeSession = preExistingMixedModeSession;
143+
let mixedModeSession = preExistingMixedModeSessionData?.session;
135144

136-
// TODO(DEVX-1893): here we can save the converted remote bindings
137-
// and on new iterations we can diff the old and new
138-
// converted remote bindings, if they are all the
139-
// same we can just leave the mixedModeSession untouched
140-
if (!mixedModeSession) {
141-
if (Object.keys(workerRemoteBindings).length > 0) {
142-
mixedModeSession = await startMixedModeSession(workerRemoteBindings);
145+
const remoteBindingsAreSameAsBefore = deepStrictEqual(
146+
remoteBindings,
147+
preExistingMixedModeSessionData?.remoteBindings
148+
);
149+
150+
// We only want to perform updates on the mixed mode session if the session's remote bindings have changed
151+
if (!remoteBindingsAreSameAsBefore) {
152+
if (!mixedModeSession) {
153+
if (Object.keys(remoteBindings).length > 0) {
154+
mixedModeSession = await startMixedModeSession(remoteBindings);
155+
}
156+
} else {
157+
// Note: we always call updateBindings even when there are zero remote bindings, in these
158+
// cases we could terminate the remote session if we wanted, that's probably
159+
// something to consider down the line
160+
await mixedModeSession.updateBindings(remoteBindings);
143161
}
144-
} else {
145-
// Note: we always call updateBindings even when there are zero remote bindings, in these
146-
// cases we could terminate the remote session if we wanted, that's probably
147-
// something to consider down the line
148-
await mixedModeSession.updateBindings(workerRemoteBindings);
149162
}
150163

151164
await mixedModeSession?.ready;
152-
return mixedModeSession;
165+
if (!mixedModeSession) {
166+
return null;
167+
}
168+
return {
169+
session: mixedModeSession,
170+
remoteBindings,
171+
};
172+
}
173+
174+
function deepStrictEqual(source: unknown, target: unknown): boolean {
175+
try {
176+
assert.deepStrictEqual(source, target);
177+
return true;
178+
} catch {
179+
return false;
180+
}
153181
}

‎packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import type {
1919
ReloadCompleteEvent,
2020
ReloadStartEvent,
2121
} from "./events";
22-
import type { File, StartDevWorkerOptions } from "./types";
22+
import type { Binding, File, StartDevWorkerOptions } from "./types";
2323

2424
async function getBinaryFileContents(file: File<string | Uint8Array>) {
2525
if ("contents" in file) {
@@ -158,7 +158,10 @@ export class LocalRuntimeController extends RuntimeController {
158158
#mutex = new Mutex();
159159
#mf?: Miniflare;
160160

161-
#mixedModeSession: MixedModeSession | null = null;
161+
#mixedModeSessionData: {
162+
session: MixedModeSession;
163+
remoteBindings: Record<string, Binding>;
164+
} | null = null;
162165

163166
onBundleStart(_: BundleStartEvent) {
164167
// Ignored in local runtime
@@ -178,14 +181,14 @@ export class LocalRuntimeController extends RuntimeController {
178181
"../mixedMode"
179182
);
180183

181-
this.#mixedModeSession = await maybeStartOrUpdateMixedModeSession(
184+
this.#mixedModeSessionData = await maybeStartOrUpdateMixedModeSession(
182185
{
183186
name: configBundle.name,
184187
bindings:
185188
convertCfWorkerInitBindingsToBindings(configBundle.bindings) ??
186189
{},
187190
},
188-
this.#mixedModeSession ?? null
191+
this.#mixedModeSessionData ?? null
189192
);
190193
}
191194

@@ -194,7 +197,7 @@ export class LocalRuntimeController extends RuntimeController {
194197
this.#log,
195198
configBundle,
196199
this.#proxyToUserWorkerAuthenticationSecret,
197-
this.#mixedModeSession?.mixedModeConnectionString,
200+
this.#mixedModeSessionData?.session?.mixedModeConnectionString,
198201
!!experimentalMixedMode
199202
);
200203
options.liveReload = false; // TODO: set in buildMiniflareOptions once old code path is removed
@@ -306,12 +309,12 @@ export class LocalRuntimeController extends RuntimeController {
306309
await this.#mf?.dispose();
307310
this.#mf = undefined;
308311

309-
if (this.#mixedModeSession) {
312+
if (this.#mixedModeSessionData) {
310313
logger.log(chalk.dim("⎔ Shutting down remote connection..."));
311314
}
312315

313-
await this.#mixedModeSession?.dispose();
314-
this.#mixedModeSession = null;
316+
await this.#mixedModeSessionData?.session?.dispose();
317+
this.#mixedModeSessionData = null;
315318

316319
logger.debug("LocalRuntimeController teardown complete");
317320
};

0 commit comments

Comments
 (0)