This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author lukasz.langa
Recipients Francis Deslauriers, jcea, lukasz.langa, pdmccormick
Date 2017-02-21.18:43:49
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1487702629.86.0.123449764178.issue28909@psf.upfronthosting.co.za>
In-reply-to
Content
Thanks for working on this! A few thoughts.

1. Keep the existing names.

"PyTrace" is already a name in use for a different purpose. I understand the itch to make the name more "right" but I am in general not a fan of renaming "PyDTrace" to anything else now. It was a placeholder from day one (SystemTap uses it, too, after all). So, looking closer at the patch now I'd prefer us to keep all existing names and add LTTng as another alternative engine here. That will make the patch much shorter.

A nit: the name LTTng-UST is rather unfriendly, especially when used without the dash and in all lowercase characters. Given that we're using "dtrace" and "systemtap", it would be simpler to just use "lttng" (drop the "-ust").


2. DTrace vs SystemTap vs LTTng.

It's impossible to have DTrace and SystemTap at the same time, so it was natural to choose to auto-detect the engine. With LTTng it becomes less obvious what the configure options should be.

Should it be possible at all to have *both* LTTng and SystemTap compiled in at the same time? Does this make sense?

If yes, then keeping --with-dtrace and --with-lttng as separate options makes sense to me. If it doesn't, we should change the option to look like this: `--with(out)-dtrace=[=detect]`. This way people could pass the following:

    --without-dtrace (the default)
    --with-dtrace  (detects DTrace or SystemTap or LTTng, in that order)
    --with-dtrace=detect  (like the one above)
    --with-dtrace=dtrace  (assumes DTrace, fails if cannot proceed)
    --with-dtrace=systemtap  (assumes SystemTap, fails if cannot proceed)
    --with-dtrace=lttng  (assumes LTTng, fails if cannot proceed)  


3. Other questions.

> Clang on macOS gives `warning: code will never be executed` warnings on the various arms of the `if (PyTraceEnabled(...))` statements, and GCC on Linux warn about unused variables `lineno`, `funcname` and `filename` in `pytrace_function_{entry,return}`, since the actual use of those variables as arguments is preprocessed out of existance.

Do you get unused code warnings without your patch applied? I don't.


> I would be happy to re-post this to GitHub issues if so desired.

CPython is not using GitHub issues.
History
Date User Action Args
2017-02-21 18:43:49lukasz.langasetrecipients: + lukasz.langa, jcea, pdmccormick, Francis Deslauriers
2017-02-21 18:43:49lukasz.langasetmessageid: <1487702629.86.0.123449764178.issue28909@psf.upfronthosting.co.za>
2017-02-21 18:43:49lukasz.langalinkissue28909 messages
2017-02-21 18:43:49lukasz.langacreate