Skip to content

Conversation

@Marxlp
Copy link

@Marxlp Marxlp commented Jul 31, 2023

Currently, only one file was parsed, but we may need to aggregate multiple files to output result. Scenarios like multiple processing and time-consuming running will need it.

Currently, only one file was parsed, but we many need to aggregate multiple runs to output result. Scenario like multiple processing or time-consuming run will need it.
@Marxlp Marxlp changed the title feat: support multiple profile file in line_profiler.py feat: support multiple profile files in line_profiler.py Jul 31, 2023
@Erotemic
Copy link
Member

Erotemic commented Aug 3, 2023

This is a new feature, or does it fix something that was broken?

In any case this will need a test, changelog entry, and some more discussion.

@Erotemic
Copy link
Member

Should this stay open, or can it be closed?

@TTsangSC
Copy link
Collaborator

I am kinda a fan of the idea (as is evident from the shoutouts in previous PRs). There are some changes that I'd make if it was up to me though:

  • Like how we subclass the Cython class line_profiler._line_profiler.LineProfiler and add functionalities on the user-facing Python class line_profiler.line_profiler.LineProfiler, we can do the same for LineStats.
  • load_stats() would then:
    • Become a class method LineStats.from_file() (but we can keep load_stats() in the namespace as an alias); and
    • Have the signature (filename: str | os.PathLike, *filenames: str | os.PathLike) instead of (filenames: List[str | os.PathLike]) so that the old invocation (supplying a single filename as the argument) still works.
  • aggregate_stats() can become another (class or instance) method LineStats.aggregate(), or maybe we can even make it .__add__().

See a sample implementation at pytest_autoprofile.profiler._LineStatsCollator.

@Marxlp, are you still interested in implementing the feature? If not, would you mind if we took it over and gave it a makeover?

Apropos, this would also be the basis for allowing profiling to extend into subprocesses, the separate results for which can then be aggregated in the main kernprof process. Again, that's a feature I've already implemented downstream1 (see: pytest_autoprofile.startup_hook, and ._multiprocessing) and think would be useful for kernprof too. @Erotemic are you interested?

Footnotes

  1. With reference to how coverage.py did it.

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.

3 participants