Skip to content

Commit 5ee5420

Browse files
committed
refactor(@angular/build): bypass worker dispatch for untransformed files in JS transformer
Make transformData symmetrical to accept both string and Uint8Array inputs, allowing transformFile to directly delegate to transformData after reading from disk or cache. When no transformations are required, untransformed files bypass worker pool dispatch, thread synchronization, and string decoding overhead. In addition, introduce a fast byte-level check on raw ASCII bytes using a pre-allocated comment buffer to immediately return untouched buffers when no sourcemap comment exists.
1 parent 715852b commit 5ee5420

3 files changed

Lines changed: 115 additions & 34 deletions

File tree

packages/angular/build/src/tools/esbuild/javascript-transformer-worker.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ export default async function transformJavaScript(
8989
const transformedData = await transformJavaScriptImpl(filename, textData, options);
9090

9191
// Transfer the data via `move` instead of cloning
92+
if (transformedData === textData && typeof data !== 'string') {
93+
return Piscina.move(data);
94+
}
95+
9296
return Piscina.move(textEncoder.encode(transformedData));
9397
}
9498

packages/angular/build/src/tools/esbuild/javascript-transformer.ts

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { removeSourceMappingURL } from '../../utils/source-map';
1313
import { WorkerPool, WorkerPoolOptions } from '../../utils/worker-pool';
1414
import { Cache } from './cache';
1515

16+
const SOURCEMAP_COMMENT_BYTES = Buffer.from('sourceMappingURL=');
17+
1618
/**
1719
* Transformation options that should apply to all transformed files and data.
1820
*/
@@ -132,50 +134,39 @@ export class JavaScriptTransformer {
132134
return this.#runWithThrottle(async () => {
133135
const data = await readFile(filename);
134136

135-
let result;
136-
let cacheKey;
137+
let cacheKey: string | undefined;
137138
if (this.cache) {
138139
// Create a cache key from the file data and options that effect the output.
139140
// NOTE: If additional options are added, this may need to be updated.
140-
// TODO: Consider xxhash or similar instead of SHA256
141141
const hash = createHash('sha256');
142142
hash.update(`${!!skipLinker}--${!!sideEffects}`);
143143
hash.update(data);
144144
hash.update(this.#fileCacheKeyBase);
145145
cacheKey = hash.digest('hex');
146146

147147
try {
148-
result = await this.cache?.get(cacheKey);
148+
const cached = await this.cache.get(cacheKey);
149+
if (cached !== undefined) {
150+
return cached;
151+
}
149152
} catch {
150153
// Failure to get the value should not fail the transform
151154
}
152155
}
153156

154-
if (result === undefined) {
155-
// If there is no cache or no cached entry, process the file
156-
result = (await this.#ensureWorkerPool().run(
157-
{
158-
filename,
159-
data,
160-
skipLinker,
161-
sideEffects,
162-
instrumentForCoverage,
163-
...this.#commonOptions,
164-
},
165-
{
166-
// The below is disable as with Yarn PNP this causes build failures with the below message
167-
// `Unable to deserialize cloned data`.
168-
transferList: process.versions.pnp ? undefined : [data.buffer],
169-
},
170-
)) as Uint8Array;
171-
172-
// If there is a cache then store the result
173-
if (this.cache && cacheKey) {
174-
try {
175-
await this.cache.put(cacheKey, result);
176-
} catch {
177-
// Failure to store the value in the cache should not fail the transform
178-
}
157+
const result = await this.transformData(
158+
filename,
159+
data,
160+
!!skipLinker,
161+
sideEffects,
162+
instrumentForCoverage,
163+
);
164+
165+
if (this.cache && cacheKey) {
166+
try {
167+
await this.cache.put(cacheKey, result);
168+
} catch {
169+
// Failure to store the value in the cache should not fail the transform
179170
}
180171
}
181172

@@ -194,7 +185,7 @@ export class JavaScriptTransformer {
194185
*/
195186
async transformData(
196187
filename: string,
197-
data: string,
188+
data: string | Uint8Array,
198189
skipLinker: boolean,
199190
sideEffects?: boolean,
200191
instrumentForCoverage?: boolean,
@@ -206,18 +197,52 @@ export class JavaScriptTransformer {
206197
this.#commonOptions.sourcemap &&
207198
(!!this.#commonOptions.thirdPartySourcemaps || !/[\\/]node_modules[\\/]/.test(filename));
208199

209-
return Buffer.from(keepSourcemap ? data : removeSourceMappingURL(data), 'utf-8');
200+
if (typeof data === 'string') {
201+
return Buffer.from(keepSourcemap ? data : removeSourceMappingURL(data), 'utf-8');
202+
}
203+
204+
if (keepSourcemap) {
205+
return data;
206+
}
207+
208+
const dataBuffer = Buffer.isBuffer(data)
209+
? data
210+
: Buffer.from(data.buffer, data.byteOffset, data.byteLength);
211+
212+
// Fast check on raw ASCII bytes to avoid UTF-8 string decoding if no comment exists.
213+
if (dataBuffer.indexOf(SOURCEMAP_COMMENT_BYTES) === -1) {
214+
return data;
215+
}
216+
217+
const text = dataBuffer.toString('utf-8');
218+
const stripped = removeSourceMappingURL(text);
219+
220+
return stripped === text ? data : Buffer.from(stripped, 'utf-8');
210221
}
211222

212-
return this.#runWithThrottle(() =>
213-
this.#ensureWorkerPool().run({
223+
// Only standalone (non-pooled) ArrayBuffers can be transferred across worker threads.
224+
// Node.js shares an internal 8KB ArrayBuffer pool for small buffers, and transferring
225+
// a pooled buffer will throw a DataCloneError because detaching it invalidates other slices.
226+
// In addition, SharedArrayBuffers cannot be transferred, and Yarn PnP has deserialization issues.
227+
const isTransferable =
228+
typeof data !== 'string' &&
229+
data.buffer instanceof ArrayBuffer &&
230+
data.byteOffset === 0 &&
231+
data.byteLength === data.buffer.byteLength &&
232+
!process.versions.pnp;
233+
234+
return this.#ensureWorkerPool().run(
235+
{
214236
filename,
215237
data,
216238
skipLinker,
217239
sideEffects,
218240
instrumentForCoverage,
219241
...this.#commonOptions,
220-
}),
242+
},
243+
{
244+
transferList: isTransferable ? [data.buffer] : undefined,
245+
},
221246
);
222247
}
223248

packages/angular/build/src/tools/esbuild/javascript-transformer_spec.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,4 +239,56 @@ describe('JavaScriptTransformer sourcemaps', () => {
239239
expect(typeof map?.['mappings']).toBe('string');
240240
expect((map?.['mappings'] as string).length).toBeGreaterThan(0);
241241
});
242+
243+
it('should accept a Uint8Array input in transformData', async () => {
244+
transformer = new JavaScriptTransformer(
245+
{
246+
sourcemap: true,
247+
advancedOptimizations: true,
248+
},
249+
1,
250+
);
251+
252+
const inputBuffer = Buffer.from('var x = new SomeClass();', 'utf-8');
253+
const result = await transformer.transformData('src/app.js', inputBuffer, true);
254+
const text = Buffer.from(result).toString('utf-8');
255+
const map = extractSourcemap(text);
256+
257+
expect(map).toBeDefined();
258+
expect(map?.['version']).toBe(3);
259+
expect(map?.['sources']).toContain('src/app.js');
260+
expect(typeof map?.['mappings']).toBe('string');
261+
});
262+
263+
it('should strip trailing sourcemap comments from Uint8Array input on fast-path', async () => {
264+
transformer = new JavaScriptTransformer(
265+
{
266+
sourcemap: false,
267+
},
268+
1,
269+
);
270+
271+
const inputBuffer = Buffer.from(
272+
'console.log("hello");\n//# sourceMappingURL=app.js.map',
273+
'utf-8',
274+
);
275+
const result = await transformer.transformData('node_modules/my-lib/lib.js', inputBuffer, true);
276+
const text = Buffer.from(result).toString('utf-8');
277+
278+
expect(text).toBe('console.log("hello");\n');
279+
});
280+
281+
it('should return Uint8Array input untouched on fast-path when no sourcemap comment is present', async () => {
282+
transformer = new JavaScriptTransformer(
283+
{
284+
sourcemap: false,
285+
},
286+
1,
287+
);
288+
289+
const inputBuffer = Buffer.from('console.log("hello");\nconst x = 1;', 'utf-8');
290+
const result = await transformer.transformData('node_modules/my-lib/lib.js', inputBuffer, true);
291+
292+
expect(result).toBe(inputBuffer);
293+
});
242294
});

0 commit comments

Comments
 (0)