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.

Sign up for GitHub

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

Jump to bottom

Should PyObject_Call() call the profiler on C functions, use C_TRACE() macro? #73688

Closed
vstinner opened this issue Feb 8, 2017 · 7 comments
Closed

Should PyObject_Call() call the profiler on C functions, use C_TRACE() macro? #73688

vstinner opened this issue Feb 8, 2017 · 7 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

vstinner commented Feb 8, 2017

BPO 29502
Nosy @vstinner, @methane, @serhiy-storchaka, @jdemeyer, @pppery
Files
  • profiler_c_code.py
    		•

    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 = <Date 2021-09-21.22:20:13.342>
created_at = <Date 2017-02-08.15:39:38.860>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'Should PyObject_Call() call the profiler on C functions, use C_TRACE() macro?'
updated_at = <Date 2021-09-21.22:20:13.342>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-09-21.22:20:13.342>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-09-21.22:20:13.342>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2017-02-08.15:39:38.860>
creator = 'vstinner'
dependencies = []
files = ['46604']
hgrepos = []
issue_num = 29502
keywords = []
message_count = 7.0
messages = ['287343', '287402', '322810', '322833', '322838', '322925', '402392']
nosy_count = 5.0
nosy_names = ['vstinner', 'methane', 'serhiy.storchaka', 'jdemeyer', 'ppperry']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29502'
versions = ['Python 3.7']
    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 8, 2017

    call_function() and do_call_core() functions of Python/ceval.c use C_TRACE() macro to call the profiler, but PyObject_Call() and similar functions like _PyObject_FastCallKeywords() don't.

    Is it a bug or a deliberate choice for performance?

    Since PyObject_Call() and similar functions have now fast paths, I don't think that it's still needed to have these fast paths in ceval.c too. But I kept fast paths in ceval.c because of C_TRACE().

    @vstinner vstinner added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Feb 8, 2017
    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 9, 2017

    Output of attached profiler_c_code.py example:
    ---
    haypo@selma$ ./python profiler_c_code.py
    (<frame object at 0xf9d788>, 'call', None)
    (<frame object at 0xf9d788>, 'return', 'h')
    (<frame object at 0xf9d788>, 'call', None)
    (<frame object at 0xf9d788>, 'return', 'e')
    (<frame object at 0xf9d788>, 'call', None)
    (<frame object at 0xf9d788>, 'return', 'l')
    (<frame object at 0xf9d788>, 'call', None)
    (<frame object at 0xf9d788>, 'return', 'l')
    (<frame object at 0xf9d788>, 'call', None)
    (<frame object at 0xf9d788>, 'return', 'o')
    (<frame object at 0xf9d498>, 'c_call', <built-in function setprofile>)
    ---

    Except of sys.setprofile(), the profiler doesn't see any C calls! list() and map() are simply "hidden".

    So tools like coverage miss a lot of C call? Again, is it a compromise between performance and correctness, or just a regular bug?

    @jdemeyer
    Copy link
    Contributor

    I always assumed that enabling profiling only from the Python bytecode interpreter was a deliberate choice.

    @pppery
    Copy link
    Mannequin

    pppery mannequin commented Aug 1, 2018

    bpo-30990 is related

    @methane
    Copy link
    Member

    methane commented Aug 1, 2018

    FYI, _lsprof uses PyObject_Call()

    cpython/Modules/_lsprof.c

    Lines 120 to 123 in ea68d83

    static long long CallExternalTimer(ProfilerObject *pObj)
    {
    long long result;
    PyObject *o = PyObject_Call(pObj->externalTimer, empty_tuple, NULL);

    If PyObject_Call() trigger profiler, lsprof should avoid recursion.

    @jdemeyer
    Copy link
    Contributor

    jdemeyer commented Aug 2, 2018

    I would prefer to wait with this issue until PEP-580 has been decided. If it's accepted, it will change function calling code a lot. As I wrote in PEP-580, I was planning to have a PEP on profiling anyway.

    @vstinner
    Copy link
    Member Author

    This issue has no activity for 3 years and was created 4 years ago. I close it.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @methane @jdemeyer