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

python -c: Line causing exception not shown for exceptions other than SyntaxErrors #67224

Closed
Arfrever mannequin opened this issue Dec 12, 2014 · 23 comments
Closed

python -c: Line causing exception not shown for exceptions other than SyntaxErrors #67224

Arfrever mannequin opened this issue Dec 12, 2014 · 23 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@Arfrever
Copy link
Mannequin

Arfrever mannequin commented Dec 12, 2014

BPO 23035
Nosy @terryjreedy, @pitrou, @vstinner, @benjaminp, @serhiy-storchaka

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 = None
closed_at = None
created_at = <Date 2014-12-12.01:44:07.380>
labels = ['interpreter-core', 'type-feature', '3.11']
title = 'python -c: Line causing exception not shown for exceptions other than SyntaxErrors'
updated_at = <Date 2021-06-15.16:58:00.697>
user = 'https://bugs.python.org/Arfrever'

bugs.python.org fields:

activity = <Date 2021-06-15.16:58:00.697>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2014-12-12.01:44:07.380>
creator = 'Arfrever'
dependencies = []
files = []
hgrepos = []
issue_num = 23035
keywords = []
message_count = 7.0
messages = ['232506', '232537', '232578', '232582', '232583', '232584', '232585']
nosy_count = 6.0
nosy_names = ['terry.reedy', 'pitrou', 'vstinner', 'benjamin.peterson', 'Arfrever', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue23035'
versions = ['Python 3.11']

Linked PRs

@Arfrever
Copy link
Mannequin Author

Arfrever mannequin commented Dec 12, 2014

When 'python -c ${command}' is used and exception other than SyntaxError occurs, then line causing exception is not shown.

Problem seen in output of last 2 commands below:

$ cat /tmp/test1
1 /
$ cat /tmp/test2
1 / 0
$ cat /tmp/test3
a
$ python3.5 /tmp/test1
  File "/tmp/test1", line 1
    1 /
      ^
SyntaxError: invalid syntax
$ python3.5 /tmp/test2
Traceback (most recent call last):
  File "/tmp/test2", line 1, in <module>
    1 / 0
ZeroDivisionError: division by zero
$ python3.5 /tmp/test3
Traceback (most recent call last):
  File "/tmp/test3", line 1, in <module>
    a
NameError: name 'a' is not defined
$ python3.5 -c '1 /'
  File "<string>", line 1
    1 /
      ^
SyntaxError: invalid syntax
$ python3.5 -c '1 / 0'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ZeroDivisionError: division by zero
$ python3.5 -c 'a'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'a' is not defined

@Arfrever Arfrever mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Dec 12, 2014
@vstinner vstinner changed the title -c: Line causing exception not shown for exceptions other than SyntaxErrors python -c: Line causing exception not shown for exceptions other than SyntaxErrors Dec 12, 2014
@vstinner
Copy link
Member

SyntaxError exceptions have a text attribute which contains the line where the error occurred. It's really a special case. For other exceptions, Python only knows that the error occurred in the file called "<string>".

Being able to display the line for any exception requires a complex development. I'm not interested to implement it, I don't think that it's very useful (compared to the time needed to develop it).

@Arfrever
Copy link
Mannequin Author

Arfrever mannequin commented Dec 12, 2014

It should not be more complex to read a line from a command line argument than to read a line from a regular file.

@terryjreedy
Copy link
Member

Code entered with -c seems to be treated the same as code entered at the >>> prompt of the interactive interpreter.

>>> 1/0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ZeroDivisionError: division by zero

In both cases, the offending code is right there to be seen, so I can understand reluctance to echo it. For SyntaxErrors (and only them) echoing the code is needed to have something to point to.

Idle's Shell does what you want.
>>> 1/0
Traceback (most recent call last):
  File "<pyshell#12>", line 1, in <module>
    1/0
ZeroDivisionError: division by zero

Shell can do this because it has easy, platform-independent access to the tkinter Text widget storing and displaying previously entered code. I presume accessing a system-dependent console history buffer is much harder.

Where the difference really matters is when the error is in previously defined objects.

>>> def f():
...   return a

>>> f()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 2, in f
NameError: name 'a' is not defined

versus (Shell)

>>> def f():
	return a

>>> f()
Traceback (most recent call last):
  File "<pyshell#16>", line 1, in <module>
    f()
  File "<pyshell#15>", line 2, in f
    return a
NameError: name 'a' is not defined

@Arfrever
Copy link
Mannequin Author

Arfrever mannequin commented Dec 13, 2014

Argument of -c option can have multiple lines, while only 1 line can be directly entered in interactive interpreter.

python -c $'line1\nline2\nline3\nline4\n...'

@terryjreedy
Copy link
Member

One can paste multiple lines, comprising multiple statements, into the console interprer. (Shell only recognizes a single pasted statement.)
I agree, however, that it seems that Python could keep the split version of the input line for the purpose of tracebacks. I just tried

C:\Users\Terry>python -c "import sys; print(sys.argv)"
['-c']

I expected to see to see a list of 3 strings, not 1.

@Arfrever
Copy link
Mannequin Author

Arfrever mannequin commented Dec 13, 2014

Arguments after argument of -c option are included in sys.argv:

$ python -c "import sys; print(sys.argv)" a b
['-c', 'a', 'b']

@iritkatriel iritkatriel added 3.11 only security fixes type-feature A feature request or enhancement labels Jun 15, 2021
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@iritkatriel
Copy link
Member

CC @pablogsal

pablogsal added a commit to pablogsal/cpython that referenced this issue Oct 23, 2023
pablogsal added a commit to pablogsal/cpython that referenced this issue Oct 23, 2023
pablogsal added a commit to pablogsal/cpython that referenced this issue Oct 23, 2023
…ion when running Python

Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
pablogsal added a commit to pablogsal/cpython that referenced this issue Oct 24, 2023
pablogsal added a commit to pablogsal/cpython that referenced this issue Oct 24, 2023
pablogsal added a commit to pablogsal/cpython that referenced this issue Oct 25, 2023
…when using the -c option when running Python
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@brandtbucher
Copy link
Member

Reopening because it appears this change slowed the python_startup_no_site benchmark down by ~24%. I believe this is because linecache._register_code is now imported and called on startup for -c commands.

Just to clarify, I think this is a good feature, and the benchmark (timing python -S -c pass) doesn't represent real-world code. I'm just wondering if there's a way to speed this up. Some options that could be worth exploring:

  • Freeze linecache. Seems easy enough to test, but may not be much faster.
  • Only import linecache if an exception is raised. Keeps the import out of the happy path, but seems trickier to fix up the traceback after-the-fact.
  • Sort of like the above, except keep a bunch of "lines to be cached" in the interpreter and populate linecache.cache with them only once it's actually imported (which should happen if an exception is raised). Requires a bit of surgery but seems a bit simpler.

@brandtbucher brandtbucher reopened this Apr 2, 2024
@brandtbucher
Copy link
Member

CC @pablogsal, thoughts?

@pitrou
Copy link
Member

pitrou commented Apr 3, 2024

Another option: try to make linecache faster to import? What is there that takes so long?

@pablogsal
Copy link
Member

Only import linecache if an exception is raised. Keeps the import out of the happy path, but seems trickier to fix up the traceback after-the-fact.

Hummmm I fail to see how that would work. We use linecache to store lines written as soon as they happen and attach that to code object names. We cannot do this after the fact because the line itself will be gone and/or the code object won't be updated with the mappeable name

@pablogsal
Copy link
Member

Another option: try to make linecache faster to import? What is there that takes so long?

Yeah I would like to investigate first what's the main problem with the import and why freezing wouldn't work here

@pablogsal
Copy link
Member

Sort of like the above, except keep a bunch of "lines to be cached" in the interpreter and populate linecache.cache with them only once it's actually imported (which should happen if an exception is raised). Requires a bit of surgery but seems a bit simpler.

This is probably the cleanest but I think it's going to be slightly confusing to maintain because it's a bit of a spooky action at a distance.

From all the solutions proposed this may be the better one, although I would like to understand why freezing doesn't work or why importing linecache is slow.

@pablogsal
Copy link
Member

According to ./python -Ximporttime -c pass importing linecache takes 363 milliseconds:

import time: self [us] | cumulative | imported package
import time:       533 |        533 |   _io
import time:       100 |        100 |   marshal
import time:       819 |        819 |   posix
import time:      1671 |       3121 | _frozen_importlib_external
import time:       157 |        157 |   time
import time:       395 |        552 | zipimport
import time:       107 |        107 |     _codecs
import time:      1019 |       1126 |   codecs
import time:       726 |        726 |   encodings.aliases
import time:       871 |       2722 | encodings
import time:       359 |        359 | encodings.utf_8
import time:       163 |        163 | _signal
import time:        56 |         56 |     _abc
import time:       290 |        345 |   abc
import time:       310 |        655 | io
import time:        91 |         91 |       _stat
import time:       282 |        372 |     stat
import time:      1731 |       1731 |     _collections_abc
import time:       134 |        134 |       genericpath
import time:       268 |        401 |     posixpath
import time:       946 |       3449 |   os
import time:       190 |        190 |   _sitebuiltins
import time:       264 |        264 |   sitecustomize
import time:        97 |         97 |   usercustomize
import time:       596 |       4594 | site
import time:       363 |        363 | linecache

@pablogsal
Copy link
Member

After freezing linecache I get:

import time: self [us] | cumulative | imported package
import time:       454 |        454 |   _io
import time:        82 |         82 |   marshal
import time:       736 |        736 |   posix
import time:      1455 |       2726 | _frozen_importlib_external
import time:       211 |        211 |   time
import time:       528 |        738 | zipimport
import time:       130 |        130 |     _codecs
import time:      1317 |       1446 |   codecs
import time:       952 |        952 |   encodings.aliases
import time:      1076 |       3474 | encodings
import time:       336 |        336 | encodings.utf_8
import time:       150 |        150 | _signal
import time:        51 |         51 |     _abc
import time:       266 |        317 |   abc
import time:       284 |        600 | io
import time:        84 |         84 |       _stat
import time:       269 |        353 |     stat
import time:      1519 |       1519 |     _collections_abc
import time:       125 |        125 |       genericpath
import time:       243 |        368 |     posixpath
import time:       885 |       3124 |   os
import time:       175 |        175 |   _sitebuiltins
import time:       243 |        243 |   sitecustomize
import time:        65 |         65 |   usercustomize
import time:       535 |       4139 | site
import time:       118 |        118 | linecache

@pablogsal
Copy link
Member

pablogsal commented Apr 3, 2024

That reduces the import time by 3 times, which hopefully it's enough. What do you think @brandtbucher ?

@pablogsal
Copy link
Member

pablogsal commented Apr 3, 2024

Flamegraph of startup time (download the svg and play locally):

perf

@pablogsal
Copy link
Member

If you explore the flamegraph seems that the higher cost is getting the os module:
blech

@pablogsal
Copy link
Member

After making moving import sys, import os to the functions that use it I can get a big chunk out of the time:

## Main

Benchmark 1: ./python -S -c pass
  Time (mean ± σ):      22.5 ms ±   5.5 ms    [User: 17.6 ms, System: 4.0 ms]
  Range (min … max):     7.6 ms …  30.1 ms    102 runs

## After
hyperfine "./python -S -c pass"
Benchmark 1: ./python -S -c pass
  Time (mean ± σ):      18.7 ms ±   4.5 ms    [User: 15.2 ms, System: 3.6 ms]
  Range (min … max):     4.9 ms …  27.3 ms    120 runs

With both effects, I get:

blech

@pablogsal
Copy link
Member

This analysis has being delivered to you thanks to the Perf integration 😉

@pitrou
Copy link
Member

pitrou commented Apr 3, 2024

363 µs (not ms!) looks small in any case.

pablogsal added a commit to pablogsal/cpython that referenced this issue Apr 3, 2024
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
pablogsal added a commit to pablogsal/cpython that referenced this issue Apr 3, 2024
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@brandtbucher
Copy link
Member

Thanks for tackling this, @pablogsal. Your fix made the benchmark 27% faster, more than recovering the lost performance (and keeping the feature)! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants