Refactor source report to use sparse line metrics#6153
Conversation
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.
|
✅ 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. |
| return array.NewRecordBatch( | ||
| arrow.NewSchema( | ||
| []arrow.Field{ | ||
| {Name: "line_number", Type: arrow.PrimitiveTypes.Int64}, |
There was a problem hiding this comment.
Add the line number as a column in the Arrow schema
| if err != nil { | ||
| if errors.Is(err, ErrSourceNotFound) || errors.Is(err, ErrNoSourceForBuildID) { | ||
| return nil, status.Error(codes.NotFound, err.Error()) | ||
| source = "" |
There was a problem hiding this comment.
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}>(); |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
that makes sense. I can change this
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