classification
Title: Improve the lltrace feature with the Py_Debug mode
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: haypo Nosy List: akaptur, ebarry, gwk, haypo, matrixise, xcombelle
Priority: normal Keywords: patch

Created on 2015-11-06 21:13 by matrixise, last changed 2017-07-23 13:02 by haypo.

Files
File name Uploaded Description Edit
issue25571.patch matrixise, 2015-11-06 21:14 review
issue25571-2.diff matrixise, 2016-07-16 10:21 review
issue25571-3.diff matrixise, 2016-08-03 16:59 review
issue25571-4.diff matrixise, 2016-08-03 23:06 review
Messages (20)
msg254218 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-11-06 21:13
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.
msg254219 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-11-06 21:14
Here is my patch with the improvement.
msg255347 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-11-25 13:30
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.
msg270552 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-07-16 10:17
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")
```
msg270553 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-07-16 10:21
here is the patch, if you want to test it, just use the REPL and add __ltrace__ = None in the REPL.
msg270859 - (view) Author: Xavier Combelle (xcombelle) * Date: 2016-07-20 08:35
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
msg270860 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-07-20 08:51
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
>
msg270863 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-07-20 11:05
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')
---
msg270864 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-07-20 11:10
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
msg271914 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-08-03 16:59
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.
msg271915 - (view) Author: Emanuel Barry (ebarry) * Date: 2016-08-03 17:17
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).
msg271931 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-08-03 23:06
Here is a new version of my patch "issue25571-4.diff" with the comments of Emanuel Barry.
msg271933 - (view) Author: Emanuel Barry (ebarry) * Date: 2016-08-04 00:00
Maybe someone who knows this feature better can weigh in, but otherwise LGTM.
msg278723 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-10-15 12:53
I have to update my patch to python-3.7, I will provide a patch asap.
msg294378 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2017-05-24 19:07
@haypo, you told me there is an alternative to my patch, provided by an other dev. what's the bpo issue for the alternative.
msg294463 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-05-25 10:50
https://bugs.python.org/issue29400
msg298435 - (view) Author: George King (gwk) * Date: 2017-07-16 12:33
@matrixise, I'm the author of the alternative in issue29400, and I'm finally finding the time to get back into it. I'm going to make a push this week to clean it up; your feedback would be much appreciated!
msg298438 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-07-16 14:14
I still strongly prefer bpo-29400 over "lltrace", since it would be usable in release mode.
msg298849 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2017-07-22 13:58
@gwk no problem for a review of your patch

@haypo as discussed before, maybe we could remove the lltrace feature because this one is never used by the developers.

or we could ask on @python-dev ML.

If there is a better solution and this one could replace the current `lltrace`, I am ok with that.
msg298904 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-07-23 13:02
I suggest to wait until sys.settrace() supports bytecode level tracing,
then rewrite lltrace on top of it, open a new issue to remove C lltrace and
close this issue. You need to write to python-dev if you want to remove the
current C lltrace.
History
Date User Action Args
2017-07-23 13:02:34hayposetmessages: + msg298904
2017-07-22 13:58:30matrixisesetmessages: + msg298849
2017-07-16 14:14:07hayposetmessages: + msg298438
2017-07-16 12:33:51gwksetnosy: + gwk
messages: + msg298435
2017-05-25 10:50:13hayposetmessages: + msg294463
2017-05-24 19:07:26matrixisesetmessages: + msg294378
2016-10-15 12:53:18matrixisesetmessages: + msg278723
2016-08-15 10:33:54matrixisesetassignee: haypo
2016-08-04 00:00:34ebarrysetmessages: + msg271933
2016-08-03 23:06:11matrixisesetfiles: + issue25571-4.diff

messages: + msg271931
2016-08-03 22:54:04akaptursetnosy: + akaptur
2016-08-03 17:17:34ebarrysetnosy: + ebarry
messages: + msg271915

type: enhancement
stage: patch review
2016-08-03 16:59:20matrixisesetfiles: + issue25571-3.diff

messages: + msg271914
2016-07-20 11:10:04hayposetmessages: + msg270864
2016-07-20 11:05:56hayposetmessages: + msg270863
2016-07-20 08:51:11matrixisesetmessages: + msg270860
2016-07-20 08:35:50xcombellesetnosy: + xcombelle
messages: + msg270859
2016-07-19 15:20:05matrixisesetcomponents: + Interpreter Core
versions: + Python 3.6
2016-07-16 10:21:51matrixisesetfiles: + issue25571-2.diff

messages: + msg270553
2016-07-16 10:17:33matrixisesetmessages: + msg270552
2015-11-25 13:30:58hayposetnosy: + haypo
messages: + msg255347
2015-11-06 21:14:24matrixisesetfiles: + issue25571.patch
keywords: + patch
messages: + msg254219
2015-11-06 21:13:45matrixisecreate