Skip to content

Commit 99ec92b

Browse files
gpandersgpanders
authored
Merge branch 'main' into push-nunwqontxqtp
2 parents 6f02c57 + ab75fd8 commit 99ec92b

File tree

6 files changed

+115
-473
lines changed

6 files changed

+115
-473
lines changed

‎.changeset/graduate-asset-server-experiment.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@cloudflare/workers-shared": patch
3+
---
4+
5+
perf: graduate asset-server binary search experiment to 100%
6+
7+
The improved iterative binary search implementation has been graduated from a 50% experiment to the default implementation. This provides better performance for asset manifest lookups by replacing the recursive binary search with an iterative approach.

‎packages/workers-shared/asset-worker/src/assets-manifest.2.ts

Lines changed: 0 additions & 122 deletions
This file was deleted.

‎packages/workers-shared/asset-worker/src/assets-manifest.ts

Lines changed: 94 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,23 @@
11
import {
2+
CONTENT_HASH_OFFSET,
23
CONTENT_HASH_SIZE,
34
ENTRY_SIZE,
45
HEADER_SIZE,
6+
PATH_HASH_OFFSET,
57
PATH_HASH_SIZE,
68
} from "../../utils/constants";
79

810
export class AssetsManifest {
9-
private data: ArrayBuffer;
11+
private data: Uint8Array;
1012

1113
constructor(data: ArrayBuffer) {
12-
this.data = data;
14+
this.data = new Uint8Array(data);
1315
}
1416

1517
async get(pathname: string) {
1618
const pathHash = await hashPath(pathname);
17-
const entry = binarySearch(
18-
new Uint8Array(this.data, HEADER_SIZE),
19-
pathHash
20-
);
21-
return entry ? contentHashToKey(entry) : null;
19+
const entry = binarySearch(this.data, pathHash);
20+
return entry ? Uint8ToHexString(entry) : null;
2221
}
2322
}
2423

@@ -32,41 +31,103 @@ export const hashPath = async (path: string) => {
3231
return new Uint8Array(hashBuffer, 0, PATH_HASH_SIZE);
3332
};
3433

34+
/**
35+
* Search for an entry with the given hash path.
36+
*
37+
* @param manifest the manifest bytes
38+
* @param pathHash the path hash to find in the manifest
39+
* @returns The content hash when the entry is found and `false` otherwise
40+
*/
3541
export const binarySearch = (
36-
arr: Uint8Array,
37-
searchValue: Uint8Array
42+
manifest: Uint8Array,
43+
pathHash: Uint8Array
3844
): Uint8Array | false => {
39-
if (arr.byteLength === 0) {
40-
return false;
41-
}
42-
const offset =
43-
arr.byteOffset + ((arr.byteLength / ENTRY_SIZE) >> 1) * ENTRY_SIZE;
44-
const current = new Uint8Array(arr.buffer, offset, PATH_HASH_SIZE);
45-
if (current.byteLength !== searchValue.byteLength) {
45+
if (pathHash.byteLength !== PATH_HASH_SIZE) {
4646
throw new TypeError(
47-
"Search value and current value are of different lengths"
47+
`Search value should have a length of ${PATH_HASH_SIZE}`
4848
);
4949
}
50-
const cmp = compare(searchValue, current);
51-
if (cmp < 0) {
52-
const nextOffset = arr.byteOffset;
53-
const nextLength = offset - arr.byteOffset;
54-
return binarySearch(
55-
new Uint8Array(arr.buffer, nextOffset, nextLength),
56-
searchValue
57-
);
58-
} else if (cmp > 0) {
59-
const nextOffset = offset + ENTRY_SIZE;
60-
const nextLength = arr.buffer.byteLength - offset - ENTRY_SIZE;
61-
return binarySearch(
62-
new Uint8Array(arr.buffer, nextOffset, nextLength),
63-
searchValue
50+
51+
const numberOfEntries = (manifest.byteLength - HEADER_SIZE) / ENTRY_SIZE;
52+
53+
if (numberOfEntries === 0) {
54+
return false;
55+
}
56+
57+
let lowIndex = 0;
58+
let highIndex = numberOfEntries - 1;
59+
60+
while (lowIndex <= highIndex) {
61+
const middleIndex = (lowIndex + highIndex) >> 1;
62+
63+
const cmp = comparePathHashWithEntry(pathHash, manifest, middleIndex);
64+
65+
if (cmp < 0) {
66+
highIndex = middleIndex - 1;
67+
continue;
68+
}
69+
70+
if (cmp > 0) {
71+
lowIndex = middleIndex + 1;
72+
continue;
73+
}
74+
75+
return new Uint8Array(
76+
manifest.buffer,
77+
HEADER_SIZE + middleIndex * ENTRY_SIZE + CONTENT_HASH_OFFSET,
78+
CONTENT_HASH_SIZE
6479
);
65-
} else {
66-
return new Uint8Array(arr.buffer, offset, ENTRY_SIZE);
6780
}
81+
82+
return false;
6883
};
6984

85+
/**
86+
* Compares a search value with a path hash in the manifest
87+
*
88+
* @param searchValue a `Uint8Array` of size `PATH_HASH_SIZE`
89+
* @param manifest the manifest bytes
90+
* @param entryIndex the index in the manifest of the entry to compare
91+
*/
92+
function comparePathHashWithEntry(
93+
searchValue: Uint8Array,
94+
manifest: Uint8Array,
95+
entryIndex: number
96+
) {
97+
let pathHashOffset = HEADER_SIZE + entryIndex * ENTRY_SIZE + PATH_HASH_OFFSET;
98+
for (let offset = 0; offset < PATH_HASH_SIZE; offset++, pathHashOffset++) {
99+
// We know that both values could not be undefined
100+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
101+
const s = searchValue[offset]!;
102+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
103+
const e = manifest[pathHashOffset]!;
104+
if (s < e) {
105+
return -1;
106+
}
107+
if (s > e) {
108+
return 1;
109+
}
110+
}
111+
112+
return 0;
113+
}
114+
115+
/**
116+
* Converts an Uint8Array to an hex string
117+
*
118+
* @param array The content hash
119+
* @returns padded hex string
120+
*/
121+
const Uint8ToHexString = (array: Uint8Array) => {
122+
return [...array].map((b) => b.toString(16).padStart(2, "0")).join("");
123+
};
124+
125+
/**
126+
* Compare two Uint8Array values
127+
* @param a First array
128+
* @param b Second array
129+
* @returns -1 if a < b, 1 if a > b, 0 if equal
130+
*/
70131
export const compare = (a: Uint8Array, b: Uint8Array) => {
71132
if (a.byteLength < b.byteLength) {
72133
return -1;
@@ -87,11 +148,3 @@ export const compare = (a: Uint8Array, b: Uint8Array) => {
87148

88149
return 0;
89150
};
90-
91-
const contentHashToKey = (buffer: Uint8Array) => {
92-
const contentHash = buffer.slice(
93-
PATH_HASH_SIZE,
94-
PATH_HASH_SIZE + CONTENT_HASH_SIZE
95-
);
96-
return [...contentHash].map((b) => b.toString(16).padStart(2, "0")).join("");
97-
};

‎packages/workers-shared/asset-worker/src/worker.ts

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { setupSentry } from "../../utils/sentry";
44
import { mockJaegerBinding } from "../../utils/tracing";
55
import { Analytics } from "./analytics";
66
import { AssetsManifest } from "./assets-manifest";
7-
import { AssetsManifest as AssetsManifest2 } from "./assets-manifest.2";
87
import { normalizeConfiguration } from "./configuration";
98
import { ExperimentAnalytics } from "./experiment-analytics";
109
import { canFetch, handleRequest } from "./handler";
@@ -232,12 +231,6 @@ export default class<TEnv extends Env = Env> extends WorkerEntrypoint<TEnv> {
232231
pathname: string,
233232
_request?: Request
234233
): Promise<string | null> {
235-
const BINARY_SEARCH_EXPERIMENT_SAMPLE_RATE = 0.5;
236-
const binarySearchVersion =
237-
Math.random() < BINARY_SEARCH_EXPERIMENT_SAMPLE_RATE
238-
? "current"
239-
: "perfTest";
240-
241234
const analytics = new ExperimentAnalytics(this.env.EXPERIMENT_ANALYTICS);
242235
const performance = new PerformanceTimer(this.env.UNSAFE_PERFORMANCE);
243236
const jaeger = this.env.JAEGER ?? mockJaegerBinding();
@@ -250,31 +243,13 @@ export default class<TEnv extends Env = Env> extends WorkerEntrypoint<TEnv> {
250243
analytics.setData({
251244
accountId: this.env.CONFIG.account_id,
252245
experimentName: "manifest-read-timing",
253-
binarySearchVersion,
254246
});
255247
}
256248

257249
const startTimeMs = performance.now();
258250
try {
259-
let eTag: string | null;
260-
261-
if (binarySearchVersion === "perfTest") {
262-
try {
263-
const assetsManifest = new AssetsManifest2(
264-
this.env.ASSETS_MANIFEST
265-
);
266-
eTag = await assetsManifest.get(pathname);
267-
} catch {
268-
// Fallback to the "current" impl if the new one throws.
269-
// We use "current-fallback" to surface errors in the analytics data
270-
analytics.setData({ binarySearchVersion: "current-fallback" });
271-
const assetsManifest = new AssetsManifest(this.env.ASSETS_MANIFEST);
272-
eTag = await assetsManifest.get(pathname);
273-
}
274-
} else {
275-
const assetsManifest = new AssetsManifest(this.env.ASSETS_MANIFEST);
276-
eTag = await assetsManifest.get(pathname);
277-
}
251+
const assetsManifest = new AssetsManifest(this.env.ASSETS_MANIFEST);
252+
const eTag = await assetsManifest.get(pathname);
278253

279254
span.setTags({
280255
path: pathname,

0 commit comments

Comments
 (0)