Trim relative to node's parent

So far we have trimmed nodes based on the absolute cumulative value. This is not what we want to do, given we might lose data when diving further into a flame graph.
However, in relation to the parent's cumulative value, there may occasion where the direct child still has to few values and would never be rendered. Those we want to trim.

gen/proto/swagger/parca/query/v1alpha1/query.swagger.json

@@ -988,7 +988,7 @@
         "trimmed": {
           "type": "string",
           "format": "int64",
-          "description": "trimmed is the amount of samples trimmed from the flame graph."
+          "description": "trimmed is the amount of cumulative value trimmed from the flame graph."
         }
       },
       "title": "Flamegraph is the flame graph report type"

gen/proto/swagger/parca/share/v1alpha1/share.swagger.json

@@ -430,7 +430,7 @@
         "trimmed": {
           "type": "string",
           "format": "int64",
-          "description": "trimmed is the amount of samples trimmed from the flame graph."
+          "description": "trimmed is the amount of cumulative value trimmed from the flame graph."
         }
       },
       "title": "Flamegraph is the flame graph report type"

pkg/query/flamegraph_table.go

@@ -501,34 +501,33 @@ func (n FlamegraphChildren) Diff() int64 {
 	return diff
 }
 
-func TrimFlamegraph(ctx context.Context, tracer trace.Tracer, graph *querypb.Flamegraph, thresholdRate float32) *querypb.Flamegraph {
+func TrimFlamegraph(ctx context.Context, tracer trace.Tracer, graph *querypb.Flamegraph, threshold float32) *querypb.Flamegraph {
 	ctx, span := tracer.Start(ctx, "trimFlamegraph")
 	defer span.End()
 	if graph == nil {
 		return nil
 	}
-	//nolint:staticcheck // SA1019: TODO: The total should be passed differently in the future.
-	threshold := int64(float64(thresholdRate) * float64(graph.Total))
-	var children FlamegraphChildren = trimFlamegraphNodes(ctx, tracer, graph.Root.Children, threshold)
-	newTotal := int64(0)
-	newDiff := int64(0)
-	if len(graph.Root.Children) > 0 {
-		newTotal = children.Cumulative()
-		newDiff = children.Diff()
-	}
+
+	children, trimmedCumulative := trimFlamegraphNodes(
+		ctx,
+		tracer,
+		graph.Root.Children,
+		graph.Root.Cumulative,
+		threshold,
+	)
 
 	trimmedGraph := &querypb.Flamegraph{
 		Root: &querypb.FlamegraphRootNode{
 			Children:   children,
-			Cumulative: newTotal,
-			Diff:       newDiff,
+			Cumulative: graph.Root.Cumulative,
+			Diff:       graph.Root.Diff,
 		},
 		//nolint:staticcheck // SA1019: Fow now we want to support these APIs
-		Total: newTotal,
+		Total: graph.Total,
 		//nolint:staticcheck // SA1019: Fow now we want to support these APIs
 		UntrimmedTotal: graph.Total,
 		//nolint:staticcheck // SA1019: Fow now we want to support these APIs
-		Trimmed:     graph.Total - newTotal,
+		Trimmed:     trimmedCumulative,
 		Unit:        graph.Unit,
 		Height:      graph.Height,
 		StringTable: graph.StringTable,
@@ -540,27 +539,37 @@ func TrimFlamegraph(ctx context.Context, tracer trace.Tracer, graph *querypb.Fla
 	return trimmedGraph
 }
 
-func trimFlamegraphNodes(ctx context.Context, tracer trace.Tracer, nodes []*querypb.FlamegraphNode, threshold int64) []*querypb.FlamegraphNode {
-	var trimmedNodes []*querypb.FlamegraphNode
+func trimFlamegraphNodes(ctx context.Context, tracer trace.Tracer, nodes []*querypb.FlamegraphNode, parentCumulative int64, threshold float32) ([]*querypb.FlamegraphNode, int64) {
+	var (
+		trimmedCumulative int64
+		remainingNodes    []*querypb.FlamegraphNode
+	)
 	for _, node := range nodes {
-		if node.Cumulative < threshold {
+		c := float32(node.Cumulative)
+		ct := float32(parentCumulative) * (threshold)
+		// If the node's cumulative value is less than the (threshold * (cumulative value)) of this level, skip it.
+		if c < ct {
+			trimmedCumulative += node.Cumulative
 			continue
 		}
-		var oldChildren FlamegraphChildren = node.Children
-		flat := node.Cumulative - oldChildren.Cumulative()
-		var children FlamegraphChildren = trimFlamegraphNodes(ctx, tracer, node.Children, threshold)
-		newCum := flat
-		newDiff := node.Diff
-		if len(children) > 0 {
-			newCum += children.Cumulative()
-			newDiff += children.Diff()
+		// We have reached a leaf node.
+		if node.Children == nil {
+			remainingNodes = append(remainingNodes, node)
+			continue
 		}
-		trimmedNodes = append(trimmedNodes, &querypb.FlamegraphNode{
-			Meta:       node.Meta,
-			Cumulative: newCum,
-			Diff:       newDiff,
-			Children:   children,
-		})
+
+		children, childrenTrimmedCumulative := trimFlamegraphNodes(
+			ctx,
+			tracer,
+			node.Children,
+			node.Cumulative,
+			threshold,
+		)
+
+		trimmedCumulative += childrenTrimmedCumulative
+		node.Children = children
+		remainingNodes = append(remainingNodes, node)
 	}
-	return trimmedNodes
+
+	return remainingNodes, trimmedCumulative
 }

pkg/query/flamegraph_table_test.go

@@ -729,7 +729,10 @@ func TestFlamegraphTrimming(t *testing.T) {
 						LineIndex:     0,
 					},
 					Children: []*pb.FlamegraphNode{
-						{Cumulative: 1},
+						{
+							// This node is trimmed because it is below the threshold.
+							Cumulative: 1,
+						},
 						{
 							Cumulative: 30,
 							Meta: &pb.FlamegraphNodeMeta{
@@ -761,7 +764,8 @@ func TestFlamegraphTrimming(t *testing.T) {
 					},
 				},
 				{
-					Cumulative: 1,
+					// This node is trimmed because it is below the threshold.
+					Cumulative: 3,
 					Meta: &pb.FlamegraphNodeMeta{
 						LocationIndex: 3,
 						LineIndex:     0,
@@ -770,16 +774,17 @@ func TestFlamegraphTrimming(t *testing.T) {
 			},
 		},
 	}
-	trimmedGraph := TrimFlamegraph(context.Background(), trace.NewNoopTracerProvider().Tracer(""), fullGraph, 0.02)
+	// trim all children that have less than 10% cumulative value of the parent.
+	trimmedGraph := TrimFlamegraph(context.Background(), trace.NewNoopTracerProvider().Tracer(""), fullGraph, 0.1)
 	require.Equal(t, &pb.Flamegraph{
-		Total:          100,
-		Trimmed:        2,
+		Total:          102,
+		Trimmed:        4,
 		UntrimmedTotal: 102,
 		Root: &pb.FlamegraphRootNode{
-			Cumulative: 100,
+			Cumulative: 102,
 			Children: []*pb.FlamegraphNode{
 				{
-					Cumulative: 100,
+					Cumulative: 101,
 					Meta: &pb.FlamegraphNodeMeta{
 						LocationIndex: 1,
 						LineIndex:     0,
@@ -861,13 +866,13 @@ func TestFlamegraphTrimmingNodeWithFlatValues(t *testing.T) {
 	}
 	trimmedGraph := TrimFlamegraph(context.Background(), trace.NewNoopTracerProvider().Tracer(""), fullGraph, float32(0.02))
 	require.Equal(t, &pb.Flamegraph{
-		Total:          150,
+		Total:          151,
 		UntrimmedTotal: 151,
 		Trimmed:        1,
 		Root: &pb.FlamegraphRootNode{
-			Cumulative: 150,
+			Cumulative: 151,
 			Children: []*pb.FlamegraphNode{{
-				Cumulative: 150,
+				Cumulative: 151,
 				Children: []*pb.FlamegraphNode{{
 					Cumulative: 100,
 				}},
@@ -1009,11 +1014,12 @@ func TestFlamegraphTrimmingAndFiltering(t *testing.T) {
 
 	require.Equal(t, int32(6), fg.Height)
 
-	// The raw flamegraph had 12+3+3 = 18 samples.
+	// The unfiltered flamegraph had 15+3 = 18 samples.
+	// There were nodes that got trimmed with a cumulative value of 3.
 	require.Equal(t, int64(3), filtered)
 	require.Equal(t, int64(3), fg.Trimmed)
 	//nolint:staticcheck // SA1019: Fow now we want to support these APIs
-	require.Equal(t, int64(12), fg.Total)
+	require.Equal(t, int64(15), fg.Total)
 
 	// Check if tables and thus deduplication was correct and deterministic
 
@@ -1042,18 +1048,18 @@ func TestFlamegraphTrimmingAndFiltering(t *testing.T) {
 	// Check the recursive flamegraph that references the tables above.
 
 	expected := &pb.FlamegraphRootNode{
-		Cumulative: 12,
+		Cumulative: 15,
 		Children: []*pb.FlamegraphNode{{
-			Cumulative: 12,
+			Cumulative: 15,
 			Meta:       &pb.FlamegraphNodeMeta{LocationIndex: 1},
 			Children: []*pb.FlamegraphNode{{
-				Cumulative: 12,
+				Cumulative: 15,
 				Meta:       &pb.FlamegraphNodeMeta{LocationIndex: 2},
 				Children: []*pb.FlamegraphNode{{
-					Cumulative: 12,
+					Cumulative: 15,
 					Meta:       &pb.FlamegraphNodeMeta{LocationIndex: 3},
 					Children: []*pb.FlamegraphNode{{
-						Cumulative: 12,
+						Cumulative: 15,
 						Meta:       &pb.FlamegraphNodeMeta{LocationIndex: 4},
 					}},
 				}},

proto/parca/query/v1alpha1/query.proto

@@ -316,7 +316,7 @@ message Flamegraph {
   // Use trimmed instead.
   int64 untrimmed_total = 9 [deprecated = true];
 
-  // trimmed is the amount of samples trimmed from the flame graph.
+  // trimmed is the amount of cumulative value trimmed from the flame graph.
   int64 trimmed = 10;
 }
 

ui/packages/shared/client/src/parca/query/v1alpha1/query.ts

@@ -595,7 +595,7 @@ export interface Flamegraph {
      */
     untrimmedTotal: string;
     /**
-     * trimmed is the amount of samples trimmed from the flame graph.
+     * trimmed is the amount of cumulative value trimmed from the flame graph.
      *
      * @generated from protobuf field: int64 trimmed = 10;
      */

ui/packages/shared/profile/src/ProfileIcicleGraph/index.tsx

@@ -69,7 +69,6 @@ const ProfileIcicleGraph = ({
     trimmedFormatted,
     trimmedPercentage,
     isFiltered,
-    filteredFormatted,
     filteredPercentage,
   ] = useMemo(() => {
     if (graph === undefined) {
@@ -83,15 +82,16 @@ const ProfileIcicleGraph = ({
     // safeguard against division by zero
     const rawTotalDivisor = rawTotal > 0 ? rawTotal : BigInt(1);
 
+    console.log(rawTotal, total, BigInt(100) * total / rawTotalDivisor);
+
     return [
       numberFormatter.format(total),
       numberFormatter.format(rawTotal),
       trimmed > 0,
       numberFormatter.format(trimmed),
       numberFormatter.format((trimmed * BigInt(100)) / rawTotalDivisor),
       filtered > 0,
-      numberFormatter.format(filtered),
-      numberFormatter.format((filtered * BigInt(100)) / rawTotalDivisor),
+      numberFormatter.format((total * BigInt(100)) / rawTotalDivisor)
     ];
   }, [filtered, graph, total]);
 
@@ -132,18 +132,11 @@ const ProfileIcicleGraph = ({
         />
       </div>
       <p className="my-2 text-xs">
-        Showing {totalFormatted} {isFiltered || isTrimmed ? <span>of {rawFormatted} </span> : <></>}
-        samples.{' '}
-        {isFiltered ? (
-          <span>
-            Filtered {filteredFormatted} ({filteredPercentage}%) samples.&nbsp;
-          </span>
-        ) : (
-          <></>
-        )}
+        Showing {totalFormatted} {isFiltered ? <span>({filteredPercentage}%) filtered of {rawFormatted} </span> : <></>}
+        values.{' '}
         {isTrimmed ? (
           <span>
-            Trimmed {trimmedFormatted} ({trimmedPercentage}%) too small samples.
+            Trimmed {trimmedFormatted} ({trimmedPercentage}%) too small values.
           </span>
         ) : (
           <></>

ui/packages/shared/profile/src/useQuery.tsx

@@ -45,6 +45,7 @@ export const useQuery = (
       req.reportType = reportType;
       req.nodeTrimThreshold = options?.nodeTrimThreshold;
 
+      console.info('query', req);
       const {response} = await client.query(req, {meta: metadata});
       return response;
     },