Łukasz Langa added the comment:

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.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue28909>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to