Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

GH-98894: Fixfunction__return andfunction__entry dTrace probe missing afterGH-103083#125019

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Zheaoli wants to merge6 commits intopython:main
base:main
Choose a base branch
Loading
fromZheaoli:manjusaka/fix-dtrace

Conversation

Zheaoli
Copy link
Contributor

@ZheaoliZheaoli commentedOct 6, 2024
edited by bedevere-appbot
Loading

thesamesam, Decodetalkers, x-zvf, jschwinger233, and cakemanny reacted with thumbs up emoji
@Zheaoli
Copy link
ContributorAuthor

Zheaoli commentedOct 6, 2024
edited
Loading

After#103083, thefunction__return andfunction__entry dTrace probe is missing. In the production environment, the developer may combine the dtrace probe with other tools like eBPF to trace the internal state of the CPython like

sudo bpftrace -e'usdt:/home/manjusaka/Documents/projects/cpython/python:python:function__return {printf("filename: %s, funcname:%s, lineno:%d\n",str(arg0),str(arg1),arg2);}' -p 291832

And thefunction__return andfunction__entry part is still in the official document of the CPython for the Dtrace module.

So I think we should keep the codebase same with the document or we need to update the document if we confirm that we don't need the dtrace feature any more.

@picnixz
Copy link
Member

The build errors:

/tmp/tmpwuomaa14/_RETURN_VALUE.c:124:13: error: call to undeclared function 'PyDTrace_FUNCTION_RETURN_ENABLED'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
124 | DTRACE_FUNCTION_EXIT();

Something is probably missing somewhere.

@Zheaoli
Copy link
ContributorAuthor

The build errors:

/tmp/tmpwuomaa14/_RETURN_VALUE.c:124:13: error: call to undeclared function 'PyDTrace_FUNCTION_RETURN_ENABLED'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
124 | DTRACE_FUNCTION_EXIT();

Something is probably missing somewhere.

I have noticed this, I'm working on the CI. Thanks for the tips. Draft this PR first

picnixz and Wulian233 reacted with thumbs up emoji

@ZheaoliZheaoli marked this pull request as draftOctober 6, 2024 11:20
@markshannon
Copy link
Member

DTRACE_FUNCTION_ENTRY() andDTRACE_FUNCTION_EXIT() arestatic instrumentation points for dtrace, are they not?

IIUC, this means that they will not work with the JIT. Even if the instrumentation points are compiled into the templates, dtrace will not be able to find them.

How does dtrace handle jit-in-time compiled code?

@Zheaoli
Copy link
ContributorAuthor

Zheaoli commentedOct 9, 2024
edited
Loading

DTRACE_FUNCTION_ENTRY() andDTRACE_FUNCTION_EXIT() arestatic instrumentation points for dtrace, are they not?

Yes, it's static

IIUC, this means that they will not work with the JIT. Even if the instrumentation points are compiled into the templates, dtrace will not be able to find them.

How does dtrace handle jit-in-time compiled code?

TheDTRACE_FUNCTION_ENTRY() andDTRACE_FUNCTION_EXIT() is not working in JIT mode. For now I just compile some empty function into the template to avoid the compile issue.

For now, the JIT is still an experimental feature. I thinkDTRACE_FUNCTION_ENTRY() andDTRACE_FUNCTION_EXIT() are still useful for normal code.

For the future, I think we may need extra dtrace point for JIT module

@markshannon
Copy link
Member

For the future, I think we may need extra dtrace point for JIT module

How?
There's no point in adding back dtrace support for 3.14, if it stops working again in 3.15.

@Zheaoli
Copy link
ContributorAuthor

How?

That would be some different dtrace points, I'm not sure we need to discuss it here.

There's no point in adding back dtrace support for 3.14, if it stops working again in 3.15.

I'm not sure about the JIT roadmap. if here's more than five years before we make the JIT default release, I think it still is worthed adding the dtrace point back.

Otherwise, we need to clean up the docs FYIhttps://docs.python.org/3/howto/instrumentation.html

thesamesam reacted with thumbs up emoji

@Zheaoli
Copy link
ContributorAuthor

@markshannon ping~

@thesamesam
Copy link
Contributor

Also, merging it as it is facilitates backporting.

@Zheaoli
Copy link
ContributorAuthor

@markshannon ping~

x-zvf reacted with thumbs up emoji

@markshannon
Copy link
Member

I'm not sure about the JIT roadmap. if here's more than five years before we make the JIT default release, I think it still is worthed adding the dtrace point back.

The JIT will be included in 3.14, but probably off by default. It will almost certainly on by default for 3.15.

@markshannon
Copy link
Member

I'm not opposed to having dtrace hooks, but I don't see much value in them unless they

  • have tests (which means they built in by default, so CI can run the tests)
  • will work with the JIT
  • have very low overhead when not in use (should be true for the interpreter, but might not be for the JIT)

@Zheaoli
Copy link
ContributorAuthor

Zheaoli commentedOct 23, 2024
edited
Loading

I'm not opposed to having dtrace hooks, but I don't see much value in them unless they

  • have tests (which means they built in by default, so CI can run the tests)
  • will work with the JIT
  • have very low overhead when not in use (should be true for the interpreter, but might not be for the JIT)

OK, I got it. 1 and 3 would not be a big issue, but I need more time about 2. So how about we updatehttps://docs.python.org/3/howto/instrumentation.html and remove the function__return and function__entry part first?

@x-zvf
Copy link

+1
(bpf|d)trace instrumentation is an important feature for us. Is someone still working on it? If not, please add a note in the documentation that USDT probes are broken after python3.10, and users should avoid upgrading.

reffum reacted with thumbs up emoji

@furkanonder
Copy link
Contributor

@Zheaoli Can you resolve the conflicts?

@Zheaoli
Copy link
ContributorAuthor

@Zheaoli Can you resolve the conflicts?

Yes, But I think we need a final call here.

@markshannon Should we recover the USDT probe or we just need to remove it from documentation?

@ajor
Copy link

If the Python JIT can be enabled/disabled without a recompilation of Python, then to me it seems worth it to include this instrumentation for non-JIT codepaths at least.

That way, people who want instrumentation can disable the JIT to get it as needed. It can be documented that the probe points won't work when the JIT is enabled (rather than the alternative of them not working at all!)

@x-zvf
Copy link

x-zvf commentedApr 23, 2025
edited
Loading

If the Python JIT can be enabled/disabled without a recompilation of Python, then to me it seems worth it to include this instrumentation for non-JIT codepaths at least.

That way, people who want instrumentation can disable the JIT to get it as needed. It can be documented that the probe points won't work when the JIT is enabled (rather than the alternative of them not working at all!)

This seems like the easiest path to move forward with, but it defeats the greatest power of Dtrace - intrumentingrunning processes (with low overhead). If you need to restart a process to instrument it (disabling the jit), the probes become much less useful.
That said, this is still much preferable to removing the probes :)

@thesamesam
Copy link
Contributor

thesamesam commentedApr 23, 2025
edited
Loading

I'm also confident that I can get eyes on the general DTrace vs JIT issue in the future, just at the moment, the team working on that is busy with some more fundamental bits on the DTrace side. That is, I don't think this is delaying the inevitable or anything (and therefore PR should ideally go in for now).

x-zvf and reffum reacted with thumbs up emoji

…ka/fix-dtraceSigned-off-by: Manjusaka <me@manjusaka.me>
@Zheaoli
Copy link
ContributorAuthor

I think there are many people still need USDT for non-JIT build. I think this PR is still worth to push forward. I resolve the code conflict

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@picnixzpicnixzpicnixz approved these changes

@Wulian233Wulian233Wulian233 approved these changes

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@brandtbucherbrandtbucherAwaiting requested review from brandtbucherbrandtbucher is a code owner

@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowskisavannahostrowski is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Some DTrace probes are broken in 3.11
8 participants
@Zheaoli@picnixz@markshannon@thesamesam@x-zvf@furkanonder@ajor@Wulian233

[8]ページ先頭

©2009-2025 Movatter.jp