Browse Source

feat(profiling): remove unnecessary copy (#43242)

removes the this.frames = [...this.flamegraph.frames] which for large
profiles does a massive shallow copy for no reason (we are already
protecting ourselves with ReadonlyArray<Frames> and the property is
already Readonly so we can just store a reference (same with roots)

I also did some smaller updates to some variable names and added a check
for color buffer so we dont end up throwing a more generic GL error.
Jonas 2 years ago
parent
commit
317bcee720

+ 13 - 2
static/app/utils/profiling/renderers/flamegraphRenderer.spec.tsx

@@ -47,7 +47,13 @@ describe('flamegraphRenderer', () => {
             // @ts-ignore overridee the colors implementation
             STACK_TO_COLOR: () => {
               const colorMap = new Map<string, number[]>([['f0', [1, 0, 0, 1]]]);
-              return {colorBuffer: [1, 0, 0, 1], colorMap};
+              return {
+                colorBuffer: [
+                  // 2 triangles, each with 3 vertices
+                  1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1,
+                ],
+                colorMap,
+              };
             },
           },
         },
@@ -55,7 +61,12 @@ describe('flamegraphRenderer', () => {
       );
 
       expect(JSON.stringify(renderer.colors)).toEqual(
-        JSON.stringify(new Float32Array([1, 0, 0, 1]))
+        JSON.stringify(
+          new Float32Array([
+            // 2 triangles, each with 3 vertices
+            1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1,
+          ])
+        )
       );
     });
   });

+ 22 - 18
static/app/utils/profiling/renderers/flamegraphRenderer.tsx

@@ -18,6 +18,8 @@ import {fragment, vertex} from './shaders';
 // These are both mutable and are used to avoid unnecessary allocations during rendering.
 const PHYSICAL_SPACE_PX = new Rect(0, 0, 1, 1);
 const CONFIG_TO_PHYSICAL_SPACE = mat3.create();
+const VERTICES_PER_FRAME = 6;
+const COLOR_COMPONENTS = 4;
 
 class FlamegraphRenderer {
   canvas: HTMLCanvasElement | null;
@@ -83,12 +85,8 @@ class FlamegraphRenderer {
   }
 
   init(): void {
-    const VERTICES = 6;
-    const COLOR_COMPONENTS = 4;
-
-    this.colors = new Float32Array(VERTICES * COLOR_COMPONENTS);
-    this.frames = [...this.flamegraph.frames];
-    this.roots = [...this.flamegraph.root.children];
+    this.frames = this.flamegraph.frames;
+    this.roots = this.flamegraph.root.children;
 
     // Generate colors for the flamegraph
     const {colorBuffer, colorMap} = this.theme.COLORS.STACK_TO_COLOR(
@@ -98,6 +96,14 @@ class FlamegraphRenderer {
     );
 
     this.colorMap = colorMap;
+
+    if (
+      VERTICES_PER_FRAME * COLOR_COMPONENTS * this.frames.length !==
+      colorBuffer.length
+    ) {
+      throw new Error('Color buffer length does not match the number of vertices');
+    }
+
     this.colors = new Float32Array(colorBuffer);
 
     this.initCanvasContext();
@@ -108,13 +114,12 @@ class FlamegraphRenderer {
   initVertices(): void {
     const POSITIONS = 2;
     const BOUNDS = 4;
-    const VERTICES = 6;
 
     const FRAME_COUNT = this.frames.length;
 
-    this.bounds = new Float32Array(VERTICES * BOUNDS * FRAME_COUNT);
-    this.positions = new Float32Array(VERTICES * POSITIONS * FRAME_COUNT);
-    this.searchResults = new Float32Array(FRAME_COUNT * VERTICES);
+    this.bounds = new Float32Array(VERTICES_PER_FRAME * BOUNDS * FRAME_COUNT);
+    this.positions = new Float32Array(VERTICES_PER_FRAME * POSITIONS * FRAME_COUNT);
+    this.searchResults = new Float32Array(FRAME_COUNT * VERTICES_PER_FRAME);
 
     for (let index = 0; index < FRAME_COUNT; index++) {
       const frame = this.frames[index];
@@ -143,9 +148,9 @@ class FlamegraphRenderer {
 
       // @TODO check if we can pack bounds across vertex calls,
       // we are allocating 6x the amount of memory here
-      const boundsOffset = index * VERTICES * BOUNDS;
+      const boundsOffset = index * VERTICES_PER_FRAME * BOUNDS;
 
-      for (let i = 0; i < VERTICES; i++) {
+      for (let i = 0; i < VERTICES_PER_FRAME; i++) {
         const offset = boundsOffset + i * BOUNDS;
 
         this.bounds[offset] = x1;
@@ -427,6 +432,10 @@ class FlamegraphRenderer {
   }
 
   setSearchResults(searchResults: FlamegraphSearch['results']['frames']) {
+    if (!this.program || !this.gl) {
+      return;
+    }
+
     const matchedFrame = new Float32Array(6).fill(1);
     const unMatchedFrame = new Float32Array(6).fill(0);
 
@@ -439,10 +448,6 @@ class FlamegraphRenderer {
       );
     }
 
-    if (!this.program || !this.gl) {
-      return;
-    }
-
     const aIsSearchResult = this.gl.getAttribLocation(this.program, 'a_is_search_result');
     // attributes get data from buffers
     this.attributes.a_is_search_result = aIsSearchResult;
@@ -509,8 +514,7 @@ class FlamegraphRenderer {
       configSpacePixel.height
     );
 
-    const VERTICES = 6;
-    this.gl.drawArrays(this.gl.TRIANGLES, 0, this.frames.length * VERTICES);
+    this.gl.drawArrays(this.gl.TRIANGLES, 0, this.frames.length * VERTICES_PER_FRAME);
   }
 }