Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim flame graph nodes relative to node's parent #2815

Merged
merged 6 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gen/proto/go/parca/query/v1alpha1/query.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
73 changes: 41 additions & 32 deletions pkg/query/flamegraph_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
40 changes: 23 additions & 17 deletions pkg/query/flamegraph_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
}},
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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},
}},
}},
Expand Down
2 changes: 1 addition & 1 deletion proto/parca/query/v1alpha1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
*/
Expand Down
21 changes: 8 additions & 13 deletions ui/packages/shared/profile/src/ProfileIcicleGraph/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const ProfileIcicleGraph = ({
trimmedFormatted,
trimmedPercentage,
isFiltered,
filteredFormatted,
filteredPercentage,
] = useMemo(() => {
if (graph === undefined) {
Expand All @@ -90,8 +89,7 @@ const ProfileIcicleGraph = ({
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]);

Expand All @@ -118,6 +116,10 @@ const ProfileIcicleGraph = ({

if (total === BigInt(0) && !loading) return <>Profile has no samples</>;

if (isTrimmed) {
console.info(`Trimmed ${trimmedFormatted} (${trimmedPercentage}%) too small values.`);
}

return (
<div className="relative">
{compareMode && <DiffLegend />}
Expand All @@ -132,22 +134,15 @@ const ProfileIcicleGraph = ({
/>
</div>
<p className="my-2 text-xs">
Showing {totalFormatted} {isFiltered || isTrimmed ? <span>of {rawFormatted} </span> : <></>}
samples.{' '}
Showing {totalFormatted}{' '}
{isFiltered ? (
<span>
Filtered {filteredFormatted} ({filteredPercentage}%) samples.&nbsp;
</span>
) : (
<></>
)}
{isTrimmed ? (
<span>
Trimmed {trimmedFormatted} ({trimmedPercentage}%) too small samples.
({filteredPercentage}%) filtered of {rawFormatted}{' '}
</span>
) : (
<></>
)}
values.{' '}
</p>
</div>
);
Expand Down