perf: stream sampled profiles instead of loading every line #1094

Merged
JooaoPedro15 merged 1 commit from fix/stream-profile-input into main 2026-11-09 20:18:58 +00:00
JooaoPedro15 commented 2026-11-09 11:05:42 +00:00 (Migrated from github.com)

Replacement for #682 following the maintainer’s request to rebase on current main. GitHub rejected reopening the closed PR after the branch history changed.

scripts/prof.py reads every line into memory before it aggregates samples, even when skip is zero. Large captures can therefore use much more memory than the report needs.

Stream the input in one pass when skip is zero. For nonzero skip, count physical lines and rewind before streaming the selected range. This preserves the existing slice behavior, symbol lookup, filters and report output.

The branch is rebased on upstream main at ac3bd44, which includes the answer-domain fixture repair (022ae80). The current CI runner discovers the new tests/test_profile_stream.py suite automatically.

  • python3 -I -B tests/run_all.py --strict profile_stream passed on macOS. The suite covers output, duplicate frames, skip and filter behavior, empty input, and symbol resolution.
  • git diff --check upstream/main...HEAD passed.
  • The earlier macOS measurement on 100,000 synthetic samples produced identical output. Peak Python allocations measured by tracemalloc fell from 11.16 MiB to 0.02 MiB.

With nonzero skip, the lower memory use requires a second pass over the input. AI assistance from OpenAI Codex is recorded in the commit trailers.

Replacement for #682 following the maintainer’s request to rebase on current `main`. GitHub rejected reopening the closed PR after the branch history changed. `scripts/prof.py` reads every line into memory before it aggregates samples, even when `skip` is zero. Large captures can therefore use much more memory than the report needs. Stream the input in one pass when `skip` is zero. For nonzero `skip`, count physical lines and rewind before streaming the selected range. This preserves the existing slice behavior, symbol lookup, filters and report output. The branch is rebased on upstream `main` at `ac3bd44`, which includes the answer-domain fixture repair (`022ae80`). The current CI runner discovers the new `tests/test_profile_stream.py` suite automatically. - `python3 -I -B tests/run_all.py --strict profile_stream` passed on macOS. The suite covers output, duplicate frames, skip and filter behavior, empty input, and symbol resolution. - `git diff --check upstream/main...HEAD` passed. - The earlier macOS measurement on 100,000 synthetic samples produced identical output. Peak Python allocations measured by `tracemalloc` fell from 11.16 MiB to 0.02 MiB. With nonzero `skip`, the lower memory use requires a second pass over the input. AI assistance from OpenAI Codex is recorded in the commit trailers.
itaskaev2 commented 2026-11-09 20:01:04 +00:00 (Migrated from github.com)

The red "Python source checks" come from main at this branch's base, not from the change. On a merge of this branch with current main (ac3bd44), tests/test_profile_stream.py passes on Windows 11 with Python 3.12, alone and through tests/run_all.py profile. I have not tried it on a real capture yet.

The red "Python source checks" come from main at this branch's base, not from the change. On a merge of this branch with current main (ac3bd44), `tests/test_profile_stream.py` passes on Windows 11 with Python 3.12, alone and through `tests/run_all.py profile`. I have not tried it on a real capture yet.
hckrman101 (Migrated from github.com) approved these changes 2026-11-09 20:04:09 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good to me. I walked through the new profile_lines against the old readlines() slice and the behaviour matches:

  • skip == 0 streams straight through. Nonzero skip counts physical lines first, so blank lines still count before the filter, exactly as before.
  • Negative skip, skip >= 1, huge values and an empty file all clamp the same way as the old lines[int(len(lines) * skip):], and the tests cover those cases.
  • It's a generator but it's only consumed once, in the single loop in main(), so nothing else needs the list. The report output code is unchanged.
  • Both files are opened in with blocks, the encoding and errors="replace" are unchanged, and text mode keeps the same newline handling for \r\n and a last line with no newline.
  • scripts/prof.py on main is identical to this PR's base, so there are no conflicts. The new test is picked up by the runner automatically, and the checks I saw are green.

One optional nit: with a nonzero skip the file now has to be seekable, so it won't work on a pipe like /dev/stdin any more. The old code did. That's fine for a tool that reads stacks.txt from a run directory, but you could mention it in the docstring if you like.

Looks good to me. I walked through the new `profile_lines` against the old `readlines()` slice and the behaviour matches: - `skip == 0` streams straight through. Nonzero `skip` counts physical lines first, so blank lines still count before the filter, exactly as before. - Negative skip, skip >= 1, huge values and an empty file all clamp the same way as the old `lines[int(len(lines) * skip):]`, and the tests cover those cases. - It's a generator but it's only consumed once, in the single loop in `main()`, so nothing else needs the list. The report output code is unchanged. - Both files are opened in `with` blocks, the encoding and `errors="replace"` are unchanged, and text mode keeps the same newline handling for `\r\n` and a last line with no newline. - `scripts/prof.py` on main is identical to this PR's base, so there are no conflicts. The new test is picked up by the runner automatically, and the checks I saw are green. One optional nit: with a nonzero `skip` the file now has to be seekable, so it won't work on a pipe like `/dev/stdin` any more. The old code did. That's fine for a tool that reads `stacks.txt` from a run directory, but you could mention it in the docstring if you like.
Commenting is not possible because the repository is archived.
No description provided.