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
Implement a safe API wrapping PyEval_SetProfile #4008
Comments
I'm sure there's some stuff that wouldn't actually work as I've written it, but I hope it's clear enough. We could also support a safe version of |
Huh, what a coincidence, I just wanted to use this, and you gave me something to start with. Thank you! I added a test implementation: https://github.com/flying-sheep/fine-coverage/blob/4412d07e69d0c8ef9ca3b2267940459d1e2dfb19/src/lib.rs#L27-L56 Also I modified your code to be more generic: https://github.com/flying-sheep/fine-coverage/blob/4412d07e69d0c8ef9ca3b2267940459d1e2dfb19/src/tracer.rs
Just registering it and quitting the interpreter currently logs this:
Rust continues to be wonderful: This thing ran on the first attempt after compiling for the first time. next step would be to eliminate the |
That's an awesome coincidence! I also like the look of some of your improvements. One small thing to note is that I'm also not certain we need to make any code generic over |
Thanks, I didn’t realize that the events have mostly disjoint types!
hm, how do I get code reuse then? I’m gunning for these ergonomics here: #[pyclass]
struct MyTracer {
...state
}
impl tracer::Tracer<tracer::TraceEvent> for MyTracer {
fn trace(&mut self, frame: Py<PyFrame>, event: tracer::TraceEvent, py: Python) -> PyResult<()> {
...
}
}
fn main() {
let tracer = Tracer { ... };
// TODO: figure out how to do threads
Python::with_gil(|py| Bound::new(py, tracer)?.register())?;
} and being able to simply replace |
My instinct is that profile and trace are sufficiently different that code re-use might be an attractive nuisance - I'd expect swapping from one to another to require internal refactoring of |
That’s the idea! pyo3 would provide the trait (I called it The boilerplate to implement either would look similar, the struct data and function body very different: - impl tracer::Tracer<tracer::TraceEvent> for MyTracer {
+ impl tracer::Tracer<tracer::ProfileEvent> for MyProfiler {
- fn trace(&mut self, frame: Py<PyFrame>, event: tracer::TraceEvent, py: Python) -> PyResult<()> {
+ fn trace(&mut self, frame: Py<PyFrame>, event: tracer::ProfileEvent, py: Python) -> PyResult<()> {
...
}
} |
Having thought a little about this, I'm ok to see this added. Seems like at least both of you would use this API and I think others also contributed the original FFI bindings. Some thoughts:
|
Makes sense to me! Do you have any thoughts about where to put the code? Some ideas:
As for |
How about It seems to me like So 👍 from me to move ahead with just |
PyEval_SetProfile
is the C-api equivalent tosys.set_profile
. UsingPyEval_SetProfile
from Rust is preferable to usingsys.set_profile
because it avoids overhead, but currently requires usingunsafe
. I would like to add a new safe api to replace code like the following:Using the safe API could look something like:
In the C api it is also possible to omit the
Profiler
struct and just pass a profiling callback. It would be nice to support this too, but I'm not sure there's a clean way without a separate function:The text was updated successfully, but these errors were encountered: