-
- Notifications
You must be signed in to change notification settings - Fork 33.3k
bpo-44895: Introduce PYTHONDUMPREFSFILE variable for refcount dumping #27767
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
Conversation
e1190fd to 04d3241 Compare 04d3241 to 500bb24 Compare | @vstinner @pablogsal |
500bb24 to b0edcd4 Compare | Hummm, I would prefer not to have more environment variables. In this case is very easy to just dump the contents to a file via shell redirection ( 2> my_file). Maybe @vstinner thinks otherwise :) |
Hmm, I feel inconvenient while investigating this issue lol (It can be relative to each person. It can be only me feeling ;)) Out of curiosity, how do you extract the dump when you have to execute the python multiple times with Sorry I am not shell expert :( |
| And if we feel the adding variable is a burden how about separating But if we feel a burden about codebases, it's another discussion :) |
Python/pylifecycle.c Outdated
| if (dump_file) { | ||
| char dump_file_name[255]; | ||
| time_t now = time(NULL); | ||
| sprintf(dump_file_name, "cpython-tracerefs-%ld.dump", now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to use the pid rather than the timestamp. An alternative is that the environment variable contains the filename, so the user handles filename conflict. The problem is that sometimes, the correct directory is removed once the process completes. It is the case when a regrtest worker process completes.
PYTHONDUMPFILE=/path/to/dump.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pid looks better, I would like to use PYTHONDUMPDIR instead of PYTHONDUMPFILE.
So the file will be created as PYTHONDUMPDIR/cpython-tracerefs-<:pid>.txt
Python/pylifecycle.c Outdated
| char dump_file_name[255]; | ||
| time_t now = time(NULL); | ||
| sprintf(dump_file_name, "cpython-tracerefs-%ld.dump", now); | ||
| fp = fopen(dump_file_name,"w"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how dump_refs is set to non-zero.
I would prefer to call _Py_PrintReferences(fp); fclose(fp); here. So both options are not exclusive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we have to call _Py_PrintReferenceAddresses(fp) also.
And python_dump_dir is optional.
I use a shell for loop: |
How do you specify the name of the file then? I am not fully opposed to this, but I see the dumping to file option much more constrained, specially given that this is a very niche option that is not used a lot. An example is precisely what you pointed out. What is an easy way to dump to different files in a repeat or loop run? You need to modify the environment variable every time, which feels a bit odd to me. On the other hand, if others find this useful, then is fine to me. |
My first implementation was creating a single file based on timestamp, which now is based on PID and I intended to create a file on where the interpreter run. This was what I intended when I suggested separating IMHO, I don't prefer to designate a specific file name because the user feels inconvenient to set the name. |
I don't see what's wrong with setting an env var to invoke a command. You can reuse shell variable, like the loop counter. You cannot do that if the filename is generated. If the filename is generated, how do you use it in your loop? Example: |
Hmm, I try to reduce bash level manipulation.
Yeah, in that case, we can not grep the file name. but we may be able to print filename through stderr instead in that case we can grep it :) |
| My intention is like this |
bash#!/bin/bash export PYTHONDUMPREFS=1 # if we do not use PYTHONDUMPDIR variable. export PYTHONDUMPDIR="./log" for i in 1 2 3 4 5 6 7 8 9 10 do ./python.exe -m test -R 3:3 test_exceptions -m test_no_hang_on_context_chain_cycle2 done output |
| I don't know any program which gives the choice where a generated filename is created. Either the program requires a filename, or it generates a filename. Example: |
| Example: Hum, I don't recall which programs generate filenames for the output. It is not common on Unix. Maybe there is a reason? ;-) |
Hmm okay, let's change it to use designated file name :) |
02161b9 to 5324b4f Compare 3bc3ba8 to 1895e69 Compare 56af6c4 to 82a34d5 Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I just left a minor suggestion for the doc.
Co-authored-by: Victor Stinner <vstinner@python.org>
https://bugs.python.org/issue44895