From 3c474b9c09ac44e009bab008c8da94353594a8f9 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Fri, 20 Oct 2023 09:44:46 +0200
Subject: [PATCH] perf(core): avoid unnecessary callbacks in after render hooks

A few performance improvements and code cleanups in the after render hooks:
1. We were wrapping each `destroy` callback in another callback, because it was typed as `|undefined`. This is unnecessary, because the callback is guaranteed to exist. These changes pass the `destroy` function around directly and avoid the additional callback.
2. In server platforms we were recreating a noop `AfterRenderRef` on each invocation. We can save some memory by returning the same one.
3. Reworks the `AfterRenderCallback` so that it injects `NgZone` and `ErrorHandler` itself, instead of expecting them to be passed in. This reduces the amount of repetition in the code.
---
 .../core/src/render3/after_render_hooks.ts    | 47 ++++++++++---------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/packages/core/src/render3/after_render_hooks.ts b/packages/core/src/render3/after_render_hooks.ts
index 4662c4b711ab..9d9a84d1eb3e 100644
--- a/packages/core/src/render3/after_render_hooks.ts
+++ b/packages/core/src/render3/after_render_hooks.ts
@@ -130,6 +130,11 @@ export interface InternalAfterNextRenderOptions {
   injector?: Injector;
 }
 
+/** `AfterRenderRef` that does nothing. */
+const NOOP_AFTER_RENDER_REF: AfterRenderRef = {
+  destroy() {}
+};
+
 /**
  * Register a callback to run once before any userspace `afterRender` or
  * `afterNextRender` callbacks.
@@ -211,24 +216,21 @@ export function afterRender(callback: VoidFunction, options?: AfterRenderOptions
   const injector = options?.injector ?? inject(Injector);
 
   if (!isPlatformBrowser(injector)) {
-    return {destroy() {}};
+    return NOOP_AFTER_RENDER_REF;
   }
 
-  let destroy: VoidFunction|undefined;
-  const unregisterFn = injector.get(DestroyRef).onDestroy(() => destroy?.());
   const afterRenderEventManager = injector.get(AfterRenderEventManager);
   // Lazily initialize the handler implementation, if necessary. This is so that it can be
   // tree-shaken if `afterRender` and `afterNextRender` aren't used.
   const callbackHandler = afterRenderEventManager.handler ??= new AfterRenderCallbackHandlerImpl();
-  const ngZone = injector.get(NgZone);
-  const errorHandler = injector.get(ErrorHandler, null, {optional: true});
   const phase = options?.phase ?? AfterRenderPhase.MixedReadWrite;
-  const instance = new AfterRenderCallback(ngZone, errorHandler, phase, callback);
-
-  destroy = () => {
+  const destroy = () => {
     callbackHandler.unregister(instance);
     unregisterFn();
   };
+  const unregisterFn = injector.get(DestroyRef).onDestroy(destroy);
+  const instance = new AfterRenderCallback(injector, phase, callback);
+
   callbackHandler.register(instance);
   return {destroy};
 }
@@ -288,27 +290,24 @@ export function afterNextRender(
   const injector = options?.injector ?? inject(Injector);
 
   if (!isPlatformBrowser(injector)) {
-    return {destroy() {}};
+    return NOOP_AFTER_RENDER_REF;
   }
 
-  let destroy: VoidFunction|undefined;
-  const unregisterFn = injector.get(DestroyRef).onDestroy(() => destroy?.());
   const afterRenderEventManager = injector.get(AfterRenderEventManager);
   // Lazily initialize the handler implementation, if necessary. This is so that it can be
   // tree-shaken if `afterRender` and `afterNextRender` aren't used.
   const callbackHandler = afterRenderEventManager.handler ??= new AfterRenderCallbackHandlerImpl();
-  const ngZone = injector.get(NgZone);
-  const errorHandler = injector.get(ErrorHandler, null, {optional: true});
   const phase = options?.phase ?? AfterRenderPhase.MixedReadWrite;
-  const instance = new AfterRenderCallback(ngZone, errorHandler, phase, () => {
-    destroy?.();
-    callback();
-  });
-
-  destroy = () => {
+  const destroy = () => {
     callbackHandler.unregister(instance);
     unregisterFn();
   };
+  const unregisterFn = injector.get(DestroyRef).onDestroy(destroy);
+  const instance = new AfterRenderCallback(injector, phase, () => {
+    destroy();
+    callback();
+  });
+
   callbackHandler.register(instance);
   return {destroy};
 }
@@ -317,9 +316,15 @@ export function afterNextRender(
  * A wrapper around a function to be used as an after render callback.
  */
 class AfterRenderCallback {
+  private zone: NgZone;
+  private errorHandler: ErrorHandler|null;
+
   constructor(
-      private zone: NgZone, private errorHandler: ErrorHandler|null,
-      public readonly phase: AfterRenderPhase, private callbackFn: VoidFunction) {}
+      injector: Injector, public readonly phase: AfterRenderPhase,
+      private callbackFn: VoidFunction) {
+    this.zone = injector.get(NgZone);
+    this.errorHandler = injector.get(ErrorHandler, null, {optional: true});
+  }
 
   invoke() {
     try {