classification
Title: Disable [X refs, Y blocks] ouput in debug builds
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: brian.curtin, chris.jerdonek, ezio.melotti, gregory.p.smith, loewis, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2013-03-01 08:16 by ezio.melotti, last changed 2013-03-26 16:16 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
issue17323.diff ezio.melotti, 2013-03-06 19:29 Proof of concept against default. review
issue17323-2.diff ezio.melotti, 2013-03-20 10:10 review
issue17323-3.diff ezio.melotti, 2013-03-24 12:40 review
Messages (11)
msg183244 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-01 08:16
I suggest to disable the [X refs, Y blocks] ouput in debug builds by default, and provide an option to enable it if/when necessary.
Most of the time these values are not necessary, and they end up getting in the way while copy/pasting code from the interpreter and/or running tests (we even have a function in test.support to get rid of them).
They are sometimes useful while investigating refleaks, so there should still be an option to enable it.  I'm not sure what would be the best way to do it (a new python flag?).
msg183253 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-03-01 10:08
The "refs" output also complicates testing in some cases, e.g.

http://hg.python.org/cpython/file/bc4458493024/Lib/test/test_subprocess.py#l61
http://hg.python.org/cpython/file/bc4458493024/Lib/test/test_subprocess.py#l786
msg183548 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2013-03-05 19:12
I don't know exactly what the option would be called, but +1 on the idea. Perhaps something under the interpreter's -X option since it's implementation-specific?

This output gets in the way a fair bit when debugging interpreter sessions, and Chris brings up a good point about testability.
msg183608 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-06 19:29
Here's a proof of concept that defines a new _print_total_refs() function and calls it through the PRINT_TOTAL_REFS macro, disables printing the refs by default, and adds a "-X showrefcount" option to reenable it.  This can also be achieved at runtime by adding/removing 'showrefcount' from the sys._xoptions dict.

Things that should be done/decided before the final version:
 1) the function and/or prototype should probably be moved to a better place;
 2) the 'showrefcount' name sounds ok to me, but I'm open to suggestions if you can come up with something better;
 3) the function could do the equivalent of "if sys._xoptions.get('showrefcount', False):" instead of "if 'showrefcount' in sys._xoptions:";
 4) now that this can be enabled/disabled at runtime, we might make it available to non-debug builds too (unless there are other negative side-effects);
msg183650 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-03-07 10:38
_Py_GetRefTotal() wouldn't be available in non-debug builds IIRC.
msg183687 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-07 17:10
Yes -- I was proposing to make it available on non-debug builds too, unless it has a negative impact on the performance or other similar issues.
msg183882 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2013-03-10 16:36
If this is done, it should probably be on by default on all --with-pydebug buildbots.  Otherwise I suspect nobody will _ever_ look at its output and we should just remove the feature all together.

Being off in the main process on the build bots would still make test_subprocess.py happier though as that test's own child processes wouldn't pass the command line flag to enable it so they wouldn't see it in child process stderr.
msg184005 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-12 03:50
> If this is done, it should probably be on by default on all
> --with-pydebug buildbots.

With "on by default" you mean that the output should be disabled or enabled?

> Otherwise I suspect nobody will _ever_ look at its output and we
> should just remove the feature all together.

IME the refcount is only useful while trying things in the interactive interpreter manually, e.g. try to repeat the same operation and see if the refcount increases or not.  In this case the feature would still be useful, so it should be kept.
msg184750 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-20 10:10
Attached updated patch with docs and tests.
I changed the function to do the equivalent of "if sys._xoptions.get('showrefcount', False):" as proposed in msg183608.
Adding -X showrefcount to non-debug builds could be covered in a separate issue.
Cleaning up the test suite to avoid checking for the refcounts in stderr can also be done in a separate commit if necessary.
msg185134 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-24 12:40
Attached a new patch that addresses a couple of minor things pointed out in the reviews.
msg185252 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-26 00:00
New changeset 367167f93c30 by Ezio Melotti in branch 'default':
#17323: The "[X refs, Y blocks]" printed by debug builds has been disabled by default.  It can be re-enabled with the `-X showrefcount` option.
http://hg.python.org/cpython/rev/367167f93c30
History
Date User Action Args
2013-03-26 16:16:50ezio.melottisetstatus: open -> closed
assignee: ezio.melotti
resolution: fixed
stage: commit review -> resolved
2013-03-26 00:00:09python-devsetnosy: + python-dev
messages: + msg185252
2013-03-24 22:05:31ezio.melottisetnosy: + loewis
2013-03-24 12:40:38ezio.melottisetfiles: + issue17323-3.diff

messages: + msg185134
stage: patch review -> commit review
2013-03-20 10:10:59ezio.melottisetfiles: + issue17323-2.diff

messages: + msg184750
stage: needs patch -> patch review
2013-03-12 03:50:59ezio.melottisetmessages: + msg184005
2013-03-10 16:36:19gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg183882
2013-03-07 17:10:42ezio.melottisetmessages: + msg183687
2013-03-07 10:38:21pitrousetnosy: + pitrou
messages: + msg183650
2013-03-06 19:29:43ezio.melottisetfiles: + issue17323.diff
keywords: + patch
messages: + msg183608
2013-03-05 19:12:50brian.curtinsetnosy: + brian.curtin
messages: + msg183548
2013-03-01 10:08:41chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg183253
2013-03-01 08:16:09ezio.melotticreate