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

Dump prof files from AdvancedProfiler #19698

Open
clumsy opened this issue Mar 25, 2024 · 1 comment · May be fixed by #19703
Open

Dump prof files from AdvancedProfiler #19698

clumsy opened this issue Mar 25, 2024 · 1 comment · May be fixed by #19703
Labels
feature Is an improvement or enhancement help wanted Open to be worked on profiler

Comments

@clumsy
Copy link
Contributor

clumsy commented Mar 25, 2024

Description & Motivation

Currently AdvancedProfiler only outputs textual summary where the rows are sorted by cumulative time. Unfortunately this does not give a full picture about the structure and duration of call stacks involved.

Pitch

Raw cProfiler dumps provide more information to help identify bottlenecks than just module names and line numbers. E.g. using https://jiffyclub.github.io/snakeviz one can see the hierarchy of the call stacks and sort/drill-down using an interactive interface.

Luckily there's not much needed to make this available. AdvancedProfiler already collects profiling data we can easily dump using dump_stats():

    @override
    def summary(self) -> str:
        recorded_stats = {}
        for action_name, pr in self.profiled_actions.items():
		 	####### dumping prof file ########
            if self.filename and self.dirpath:
                import os
                import tempfile
                from fsspec.core import url_to_fs
                from fsspec.implementations.local import AbstractFileSystem

                filepath = os.path.join(self.dirpath, self._prepare_filename())
                def get_filesystem(path, **kwargs) -> AbstractFileSystem:
                    fs, _ = url_to_fs(str(path), **kwargs)
                    return fs
                with tempfile.TemporaryDirectory(prefix="test", suffix="test", dir=os.getcwd()) as tmp_dir:
                    # alas dump_stats() expects a file path it can open()
                    tmp_file = os.path.join(tmp_dir, "tmp.prof")
                    pr.dump_stats(tmp_file)
                    fs = get_filesystem(filepath)
                    fs.mkdirs(self.dirpath, exist_ok=True)
                    fs.copy(tmp_file, filepath + ".prof")
			###############################
            s = io.StringIO()
            ps = pstats.Stats(pr, stream=s).strip_dirs().sort_stats("cumulative")
            ps.print_stats(self.line_count_restriction)
            recorded_stats[action_name] = s.getvalue()
        return self._stats_to_str(recorded_stats)

I'm willing to contribute this but would like to clarify the best configuration and injection options. Thanks!

Alternatives

Write a custom callback and do the profiling from there - thus not using the one provided by PyTorch Lightning.

Additional context

Related to #7424

cc @Borda @carmocca

@clumsy clumsy added feature Is an improvement or enhancement needs triage Waiting to be triaged by maintainers labels Mar 25, 2024
@awaelchli awaelchli added help wanted Open to be worked on profiler and removed needs triage Waiting to be triaged by maintainers labels Mar 25, 2024
clumsy pushed a commit to clumsy/pytorch-lightning that referenced this issue Mar 26, 2024
clumsy pushed a commit to clumsy/pytorch-lightning that referenced this issue Mar 26, 2024
@clumsy clumsy linked a pull request Mar 26, 2024 that will close this issue
12 tasks
clumsy pushed a commit to clumsy/pytorch-lightning that referenced this issue Mar 26, 2024
@clumsy
Copy link
Contributor Author

clumsy commented Apr 15, 2024

I'm happy to address any feedback, @Borda @carmocca @awaelchli
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement help wanted Open to be worked on profiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants