Improve the lltrace feature with the Py_Debug mode

[matrixise]

BPO	25571
Nosy	@vstinner, @gwk, @matrixise, @Vgr255
Files	issue25571.patch issue25571-2.diff issue25571-3.diff issue25571-4.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/vstinner'
closed_at = None
created_at = <Date 2015-11-06.21:13:45.991>
labels = ['interpreter-core', 'type-feature']
title = 'Improve the lltrace feature with the Py_Debug mode'
updated_at = <Date 2017-07-23.13:02:34.149>
user = 'https://github.com/matrixise'

bugs.python.org fields:

activity = <Date 2017-07-23.13:02:34.149>
actor = 'vstinner'
assignee = 'vstinner'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2015-11-06.21:13:45.991>
creator = 'matrixise'
dependencies = []
files = ['40964', '43747', '43992', '43997']
hgrepos = []
issue_num = 25571
keywords = ['patch']
message_count = 20.0
messages = ['254218', '254219', '255347', '270552', '270553', '270859', '270860', '270863', '270864', '271914', '271915', '271931', '271933', '278723', '294378', '294463', '298435', '298438', '298849', '298904']
nosy_count = 6.0
nosy_names = ['vstinner', 'akaptur', 'gwk', 'matrixise', 'xcombelle', 'abarry']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue25571'
versions = ['Python 3.6']

[matrixise]

When we use the Py_Debug flag for the compilation, we can see the Bytecodes and the opcode arguments.

Here is a small patch, it will show the label of the opcode in the ceval.c file.

[matrixise]

Here is my patch with the improvement.

[vstinner]

What is lltrace? I never used it :-( Is it documented? Can you give examples of output before/after?

We may make your change optional to give the choice of the output.

[matrixise]

Here is a small example of lltrace when you enable it.

stephane@sg1 ~/s/h/cpython> ./python 
Python 3.6.0a3+ (default:0d8f139a6e19+, Jul 16 2016, 11:59:46) 
[GCC 6.1.1 20160621 (Red Hat 6.1.1-3)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> print
<built-in function print>
>>> print("hello")
hello
>>> __ltrace__ = None
>>> print("hello")
0: LOAD_NAME, 0
push <built-in function print>
2: LOAD_CONST, 0
push 'hello'
4: CALL_FUNCTION, 1
ext_pop 'hello'
hello
ext_pop <built-in function print>
push None
6: PRINT_EXPR
pop None
8: LOAD_CONST, 1
push None
10: RETURN_VALUE
pop None
>>> del __ltrace__
0: DELETE_NAME, 0
2: LOAD_CONST, 0
push None
4: RETURN_VALUE
pop None
>>> print("hello")
hello
>>> print("hello")

[matrixise]

here is the patch, if you want to test it, just use the REPL and add __ltrace__ = None in the REPL.

[xcombelle]

I made some comment on code in review.

A thing that worry me that there is zero automated test.

In my opinion the minimal should be to test the expected output.

A nice to have would be a test for the write_contents function of makeopcodetranslations.py

[matrixise]

Totally agree with you, I am going to check that, because currently,
there is no test with this "hidden" feature in Python.

I am going to discuss with Victor about this point.

Thanks,

Stephane

On 07/20, Xavier Combelle wrote:

Xavier Combelle added the comment:

I made some comment on code in review.

A thing that worry me that there is zero automated test.

In my opinion the minimal should be to test the expected output.

A nice to have would be a test for the write_contents function of makeopcodetranslations.py

----------
nosy: +xcombelle

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue25571\>

---

---

Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/stephane%40wirtel.be

[vstinner]

It would be nice to have unit tests and docs :-)

For unit test, you can use a script like:
---

def func():
    return 1

__ltrace__ = True
func()

---

Run the script with test.support.assert_python_ok(), and check stdout.

To skip the test if Python is compiled in released mode, you can use:
---

# Were we compiled --with-pydebug or with #define Py_DEBUG?
Py_DEBUG = hasattr(sys, 'gettotalrefcount')

...
@unittest.skipUnless(Py_DEBUG, 'need Py_DEBUG')
---

[vstinner]

Sorry, I don't know what is the best place to add new unit tests.

I don't know neither what is the best place to document the __ltrace__ feature. Maybe somewhere near:

• https://docs.python.org/dev/library/debug.html

Or maybe:

• https://docs.python.org/dev/library/trace.html

[matrixise]

Here is a small patch for the __ltrace__ feature.

I would like to propose a new test, __ltrace__ should be defined in the globals() dictionary and there is no check on the value associated to this key. Maybe we could defined it as a boolean, True or False.

[Vgr255]

Left some comments on Rietveld; mostly documentation-related (and pointed out some typos).

I'm also +1 on having to specify a True value rather than just defining the variable at all (i.e. setting __ltrace__ to False is the same as not defining it). If you change this, I'd probably force the value to be an actual bool, i.e. True or False (much like __debug__; except that it's read-only and a syntax error to assign to it).

[matrixise]

Here is a new version of my patch "bpo-25571-4.diff" with the comments of Emanuel Barry.

[Vgr255]

Maybe someone who knows this feature better can weigh in, but otherwise LGTM.