Skip to content

Commit 833c1f2

Browse files
JeanMechethePunderWoman
authored andcommitted
perf(core): avoid unnecessary aborts and reuse abort reason during resource teardown
When tearing down component trees containing resources, `abortInProgressLoad` previously invoked `pendingController.abort()` without arguments for every resource. This caused two performance issues: 1. Omitting the reason parameter in `abort()` instantiates a new `DOMException` ('AbortError') each time. In engines like V8, this triggers a full call stack capture (`CaptureStackTrace()`), whose cost scales linearly with call stack depth. On deep component tree teardown, this can take up to 20-40ms. 2. `ResourceImpl` retained its `pendingController` even after a non-streaming load had completed, unnecessarily executing `abort()` for operations that were no longer in flight. This change: - Reuses a shared `AbortError` `DOMException` reason when aborting loads, preventing repeated V8 stack captures while maintaining full compatibility with consumers expecting a standard `DOMException`. - Clears `pendingController` upon completion for non-streaming resources, avoiding unnecessary abort work when tearing down already settled resources. Streaming resources (such as `rxResource`) continue to receive abort signals on destroy to clean up active subscriptions. Fixes angular#71422
1 parent ca6e15c commit 833c1f2

2 files changed

Lines changed: 99 additions & 7 deletions

File tree

‎packages/core/src/resource/resource.ts‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {effect, EffectRef} from '../render3/reactivity/effect';
1212
import {signal, signalAsReadonlyFn, WritableSignal} from '../render3/reactivity/signal';
1313
import {untracked} from '../render3/reactivity/untracked';
1414
import {
15+
PromiseResourceOptions,
1516
Resource,
1617
ResourceDependencyError,
1718
ResourceOptions,
@@ -74,14 +75,17 @@ export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T |
7475
options as ResourceOptions<T, R> & {request: ResourceOptions<T, R>['params']}
7576
).request;
7677
const params = options.params ?? oldNameForParams ?? (() => null!);
78+
const isStreaming = isStreamingResourceOptions(options);
7779
return new ResourceImpl<T | undefined, R>(
7880
params,
79-
getLoader(options),
81+
isStreaming ? options.stream : createLoader(options),
8082
options.defaultValue,
8183
options.equal ? wrapEqualityFn(options.equal) : undefined,
8284
options.debugName,
8385
options.injector ?? inject(Injector),
8486
options.id as StateKey<T>,
87+
undefined,
88+
isStreaming,
8589
);
8690
}
8791

@@ -210,6 +214,7 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
210214
injector: Injector,
211215
private transferCacheKey: StateKey<T> | undefined,
212216
getInitialStream?: (request: R) => Signal<ResourceStreamItem<T>> | undefined,
217+
private readonly isStreaming = false,
213218
) {
214219
if (isInParamsFunction()) {
215220
throw invalidResourceCreationInParams();
@@ -526,11 +531,17 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
526531
// Resolve the pending task now that the resource has a value.
527532
resolvePendingTask?.();
528533
resolvePendingTask = undefined;
534+
535+
// For non-streaming resources, once the load is complete (settled),
536+
// there is no longer an in-progress operation to abort.
537+
if (!this.isStreaming && this.pendingController?.signal === abortSignal) {
538+
this.pendingController = undefined;
539+
}
529540
}
530541
}
531542

532543
private abortInProgressLoad(): void {
533-
untracked(() => this.pendingController?.abort());
544+
untracked(() => this.pendingController?.abort(getAbortReason()));
534545
this.pendingController = undefined;
535546

536547
// Once the load is aborted, we no longer want to block stability on its resolution.
@@ -539,6 +550,16 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
539550
}
540551
}
541552

553+
let abortReason: DOMException | undefined;
554+
555+
/**
556+
* Returns a shared `AbortError` reason, since `abort()` without a reason creates a new one (with a
557+
* stack trace) on every call, causing performance degradation on deep component trees.
558+
*/
559+
function getAbortReason(): DOMException {
560+
return (abortReason ??= new DOMException('The operation was aborted.', 'AbortError'));
561+
}
562+
542563
function saveToTransferState<R, T>(
543564
result: ResourceStreamItem<T> | undefined,
544565
transferCacheKey: StateKey<T> | undefined,
@@ -556,11 +577,9 @@ function wrapEqualityFn<T>(equal: ValueEqualityFn<T>): ValueEqualityFn<T | undef
556577
return (a, b) => (a === undefined || b === undefined ? a === b : equal(a, b));
557578
}
558579

559-
function getLoader<T, R>(options: ResourceOptions<T, R>): ResourceStreamingLoader<T, R> {
560-
if (isStreamingResourceOptions(options)) {
561-
return options.stream;
562-
}
563-
580+
function createLoader<T, R>(
581+
options: PromiseResourceOptions<T, R> & {debugName?: string},
582+
): ResourceStreamingLoader<T, R> {
564583
return async (params) => {
565584
try {
566585
return signal(

‎packages/core/test/resource/resource_spec.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,79 @@ describe('resource', () => {
527527
expect(aborted).toEqual([{counter: 0}]);
528528
});
529529

530+
it('should reuse the same abort reason when aborting in-progress resource loads', async () => {
531+
const signals: AbortSignal[] = [];
532+
const injector = createEnvironmentInjector([], TestBed.inject(EnvironmentInjector));
533+
const resources = [0, 1].map((id) =>
534+
resource({
535+
params: () => id,
536+
loader: ({abortSignal}) => {
537+
signals.push(abortSignal);
538+
return new Promise(() => {}); // never resolves
539+
},
540+
injector,
541+
}),
542+
);
543+
544+
TestBed.tick();
545+
await Promise.resolve();
546+
547+
resources.forEach((res) => res.destroy());
548+
549+
expect(signals.length).toBe(2);
550+
expect(signals[0].aborted).toBeTrue();
551+
expect(signals[0].reason).toEqual(jasmine.any(DOMException));
552+
expect(signals[0].reason.name).toBe('AbortError');
553+
expect(signals[1].reason).toBe(signals[0].reason);
554+
});
555+
556+
it('should not abort completed non-streaming resources on destroy', async () => {
557+
let capturedSignal: AbortSignal | undefined;
558+
const res = resource({
559+
loader: async ({abortSignal}) => {
560+
capturedSignal = abortSignal;
561+
return 'loaded';
562+
},
563+
injector: TestBed.inject(Injector),
564+
});
565+
566+
TestBed.tick();
567+
await flushMicrotasks();
568+
569+
expect(res.status()).toBe('resolved');
570+
expect(res.value()).toBe('loaded');
571+
expect(capturedSignal!.aborted).toBeFalse();
572+
573+
res.destroy();
574+
575+
// The load was already completed, so destroying the resource should not trigger abort
576+
expect(capturedSignal!.aborted).toBeFalse();
577+
});
578+
579+
it('should abort streaming resources on destroy using the shared abort reason', async () => {
580+
let capturedSignal: AbortSignal | undefined;
581+
const streamSignal = signal<any>({value: 42});
582+
const res = resource({
583+
stream: ({abortSignal}) => {
584+
capturedSignal = abortSignal;
585+
return streamSignal;
586+
},
587+
injector: TestBed.inject(Injector),
588+
});
589+
590+
TestBed.tick();
591+
await flushMicrotasks();
592+
593+
expect(res.status()).toBe('resolved');
594+
expect(capturedSignal!.aborted).toBeFalse();
595+
596+
res.destroy();
597+
598+
expect(capturedSignal!.aborted).toBeTrue();
599+
expect(capturedSignal!.reason).toEqual(jasmine.any(DOMException));
600+
expect(capturedSignal!.reason.name).toBe('AbortError');
601+
});
602+
530603
it('should not respond to reactive state changes in a loader', async () => {
531604
const unrelated = signal('a');
532605
const backend = new MockResponseCountingBackend();

0 commit comments

Comments
 (0)