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

Incorrect Session.program when running a module #241

Closed
matthiasdiener opened this issue May 3, 2023 · 3 comments · Fixed by #258
Closed

Incorrect Session.program when running a module #241

matthiasdiener opened this issue May 3, 2023 · 3 comments · Fixed by #258

Comments

@matthiasdiener
Copy link
Contributor

matthiasdiener commented May 3, 2023

Due to sys.argv modifications (in __main__.py?), the program name is stored incorrectly in Session.program:

actual output:

$ pyinstrument -m mpi4py t.py -a 
Program: mpi4py -a

expected output:

$ pyinstrument -m mpi4py t.py -a 
Program: mpi4py t.py -a
@joerick
Copy link
Owner

joerick commented May 4, 2023 via email

@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented May 4, 2023

Here is what I get with Python 3.10 for the following program:

import sys

print(f"{sys.argv=}")
print(f"{' | '.join(sys.argv)=}")

Output:

$ python t.py --arg
sys.argv=['t.py', '--arg']
' | '.join(sys.argv)='t.py | --arg'

$ python -m mpi4py t.py --arg
sys.argv=['t.py', '--arg']
' | '.join(sys.argv)='t.py | --arg'

$ pyinstrument t.py --arg
sys.argv=['t.py', '--arg']
' | '.join(sys.argv)='t.py | --arg'

  _     ._   __/__   _ _  _  _ _/_   Recorded: 09:25:19  Samples:  0
 /_//_/// /_\ / //_// / //_'/ //     Duration: 0.001     CPU time: 0.001
/   _/                      v4.4.0

Program: t.py --arg

$ pyinstrument -m mpi4py t.py --arg
sys.argv=['t.py', '--arg']
' | '.join(sys.argv)='t.py | --arg'

  _     ._   __/__   _ _  _  _ _/_   Recorded: 09:25:26  Samples:  1
 /_//_/// /_\ / //_// / //_'/ //     Duration: 0.001     CPU time: 0.001
/   _/                      v4.4.0

Program: mpi4py --arg

ie, neither seems to affect sys.argv, but pyinstrument -m does pass down incorrect args to the Session.

One potential solution would be to pass down a copy of the original sys.argv down to the Session constructor, another way could be to use psutil.Process.cmdline.

@joerick
Copy link
Owner

joerick commented Jul 23, 2023

I think the issue here is that changing sys.argv is pretty common in these styles of wrapper scripts. pyinstrument, mpi4py, cProfile all do it. Each time it happens, each script doesn't reset the value once the script has finished running, because each assumes it's the top-level script.

My thought is that we should consider changing sys.argv a 'patch' - i.e. it should be reset to it's previous value once the code has finished. I'll push a PR of a fix that I think works, and should make more sense.

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 a pull request may close this issue.

2 participants