FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Refactor source report to use sparse line metrics by yomete · Pull Request #6153 · parca-dev/parca · GitHub

Refactor source report to use sparse line metrics - #6153

Merged
yomete merged 5 commits into
mainfrom
source-report
Jan 15, 2026
Merged

Refactor source report to use sparse line metrics#6153
yomete merged 5 commits into
mainfrom
source-report

Conversation

yomete commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

The source report will no longer return an error if the source file is not present and we also added explicit line numbers to the Arrow schema that is returned. Now, only lines with actual profiling data are included in the response, rather than returning an array with one entry per line in the source file.

Changes:
Backend returns empty source field instead of error when source file is missing
Arrow schema now includes line_number column with explicit line numbers
Response only contains rows for lines with non-zero cumulative or flat values

Replaces fixed-size arrays with a map for per-line metrics in sourceReportBuilder, allowing for sparse line data and adding explicit line_number to the Arrow schema.
yomete requested review from a team as code owners January 15, 2026 11:36

alwaysmeticulous Bot commented Jan 15, 2026
edited
Loading

Copy link
Copy Markdown

✅ Meticulous spotted 0 visual differences across 328 screens tested: view results.

Meticulous evaluated ~4 hours of user flows against your PR.

Expected differences? Click here. Last updated for commit 910e0cf. This comment will update as new commits are pushed.

Comment thread pkg/query/sources.go
return array.NewRecordBatch(
arrow.NewSchema(
[]arrow.Field{
{Name: "line_number", Type: arrow.PrimitiveTypes.Int64},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add the line number as a column in the Arrow schema

Comment thread pkg/query/columnquery.go
if err != nil {
if errors.Is(err, ErrSourceNotFound) || errors.Is(err, ErrNoSourceForBuildID) {
return nil, status.Error(codes.NotFound, err.Error())
source = ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Don't return an error if the source code is not present

const flat = table.getChild('flat');
return [cumulative, flat];

const metrics = new Map<number, {cumulative: bigint; flat: bigint}>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We return the record in sorted order, we can just perform a binary search for each row, no? That way, we can also render only the viewport.

I would prefer it if we got into the habit of not copying data returned by the backend (although probably less problematic in this case).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

that makes sense. I can change this

yomete merged commit bf9cc51 into main Jan 15, 2026
38 checks passed
yomete deleted the source-report branch February 13, 2026 10:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL