Skip to content

Commit 0ac2b02

Browse files
chore(pyo3): fix clippy warnings and complete JIT/Vortex migration (p2)
- Fix manual implementation of is_multiple_of. - Fix manual-flatten and unnecessary if let in ocular telemetry. - Complete Bound API migration for JIT and Vortex features. - Update test suites for JIT and Vortex to align with PyO3 0.28.3. - All core and JIT tests pass; Vortex tests pass. Co-authored-by: Iris Seravelle <iris.seravelle@gmail.com>
1 parent a5dcce9 commit 0ac2b02

22 files changed

Lines changed: 1883 additions & 1776 deletions

src/core/network.rs

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ enum MessageType {
3636
User = 0,
3737
Resolve = 1,
3838
Ping = 2,
39+
PingPid = 3,
3940
}
4041

4142
impl TryFrom<u8> for MessageType {
@@ -45,6 +46,7 @@ impl TryFrom<u8> for MessageType {
4546
0 => Ok(MessageType::User),
4647
1 => Ok(MessageType::Resolve),
4748
2 => Ok(MessageType::Ping),
49+
3 => Ok(MessageType::PingPid),
4850
_ => Err(()),
4951
}
5052
}
@@ -215,6 +217,14 @@ impl NetworkManager {
215217
MessageType::Ping => {
216218
Self::write_all_with_timeout(&mut write_half, &[3u8], io_timeout).await?;
217219
}
220+
MessageType::PingPid => {
221+
let mut pid_buf = [0u8; 8];
222+
Self::read_exact_with_timeout(&mut reader, &mut pid_buf, io_timeout).await?;
223+
let pid = u64::from_be_bytes(pid_buf);
224+
let alive = rt.is_alive(pid);
225+
let resp = if alive { 3u8 } else { 4u8 }; // 3=Pong, 4=Dead
226+
Self::write_all_with_timeout(&mut write_half, &[resp], io_timeout).await?;
227+
}
218228
}
219229
}
220230
Ok(())
@@ -327,6 +337,8 @@ impl NetworkManager {
327337
break;
328338
}
329339

340+
let remote_pid = rt.remote_proxies.get(&pid).map(|e| e.value().1);
341+
330342
tokio::time::sleep(curr).await;
331343
let io_timeout = rt.get_network_io_timeout();
332344

@@ -339,19 +351,59 @@ impl NetworkManager {
339351
)
340352
.await
341353
.is_ok()
342-
&& Self::write_all_with_timeout(
343-
&mut stream,
344-
&[MessageType::Ping as u8],
345-
io_timeout,
346-
)
347-
.await
348-
.is_ok()
349354
{
350-
let mut pong = [0u8; 1];
351-
Self::read_exact_with_timeout(&mut stream, &mut pong, io_timeout)
355+
if let Some(r_pid) = remote_pid {
356+
// Specific actor monitoring
357+
if Self::write_all_with_timeout(
358+
&mut stream,
359+
&[MessageType::PingPid as u8],
360+
io_timeout,
361+
)
352362
.await
353363
.is_ok()
354-
&& pong[0] == 3
364+
&& Self::write_all_with_timeout(
365+
&mut stream,
366+
&r_pid.to_be_bytes(),
367+
io_timeout,
368+
)
369+
.await
370+
.is_ok()
371+
{
372+
let mut resp = [0u8; 1];
373+
Self::read_exact_with_timeout(
374+
&mut stream,
375+
&mut resp,
376+
io_timeout,
377+
)
378+
.await
379+
.is_ok()
380+
&& resp[0] == 3 // 3=Pong (Alive)
381+
} else {
382+
false
383+
}
384+
} else {
385+
// Generic node monitoring
386+
if Self::write_all_with_timeout(
387+
&mut stream,
388+
&[MessageType::Ping as u8],
389+
io_timeout,
390+
)
391+
.await
392+
.is_ok()
393+
{
394+
let mut pong = [0u8; 1];
395+
Self::read_exact_with_timeout(
396+
&mut stream,
397+
&mut pong,
398+
io_timeout,
399+
)
400+
.await
401+
.is_ok()
402+
&& pong[0] == 3
403+
} else {
404+
false
405+
}
406+
}
355407
} else {
356408
false
357409
}

src/core/spawn.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,6 @@ use tokio::time::Duration;
99

1010
impl Runtime {
1111
pub fn is_alive(&self, pid: Pid) -> bool {
12-
// proxies are normal actors so the slab check would cover them, but
13-
// we also treat any registered proxy as alive even if its mailbox has
14-
// been removed but the slab entry hasn't been cleaned up yet.
15-
if self.remote_proxies.contains_key(&pid) {
16-
return true;
17-
}
1812
let slab = self.slab.lock().unwrap();
1913
slab.is_valid(pid)
2014
}

src/core/vortex/ocular/callbacks.rs

Lines changed: 56 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use crate::vortex::ocular::state::{
77
use crate::vortex::ocular::telemetry::telemetry_worker;
88
use crossbeam_queue::ArrayQueue;
99
use pyo3::prelude::*;
10-
use pyo3::types::PyAny;
1110
use std::cell::RefCell;
1211
use std::collections::{HashMap, HashSet};
1312
use std::sync::atomic::Ordering;
@@ -16,7 +15,7 @@ use std::thread;
1615

1716
const BATCH_SIZE: usize = 1024;
1817

19-
static DISABLE_OBJ: OnceLock<PyObject> = OnceLock::new();
18+
static DISABLE_OBJ: OnceLock<Py<PyAny>> = OnceLock::new();
2019

2120
thread_local! {
2221
static LOCAL_BATCH: RefCell<Vec<TraceEvent>> = RefCell::new(Vec::with_capacity(BATCH_SIZE));
@@ -128,7 +127,11 @@ fn enqueue_event(event: TraceEvent) {
128127
}
129128

130129
#[pyfunction]
131-
pub fn instruction_callback(py: Python<'_>, code: &PyAny, instruction_offset: i32) -> PyObject {
130+
pub fn instruction_callback(
131+
py: Python<'_>,
132+
code: &Bound<'_, PyAny>,
133+
instruction_offset: i32,
134+
) -> Py<PyAny> {
132135
let code_ptr = code.as_ptr() as usize;
133136
let trace_allowed = match code_ptr_allowed(code_ptr) {
134137
Some(allowed) => allowed,
@@ -146,7 +149,7 @@ pub fn instruction_callback(py: Python<'_>, code: &PyAny, instruction_offset: i3
146149
};
147150

148151
if !trace_allowed {
149-
return py.None().to_object(py);
152+
return py.None();
150153
}
151154

152155
enqueue_event(TraceEvent::Instruction {
@@ -173,11 +176,11 @@ pub fn instruction_callback(py: Python<'_>, code: &PyAny, instruction_offset: i3
173176
}
174177
}
175178

176-
py.None().to_object(py)
179+
py.None()
177180
}
178181

179182
#[pyfunction]
180-
pub fn py_start_callback(code: &PyAny, instruction_offset: i32) {
183+
pub fn py_start_callback(code: &Bound<'_, PyAny>, instruction_offset: i32) {
181184
let ts = get_ts();
182185
let code_ptr = code.as_ptr() as usize;
183186

@@ -201,14 +204,12 @@ pub fn py_start_callback(code: &PyAny, instruction_offset: i32) {
201204
return;
202205
}
203206

204-
let code_opt = Python::with_gil(|py| {
205-
SEEN_CODE_PTRS.with(|seen| {
206-
if seen.borrow_mut().insert(code_ptr) {
207-
Some(code.to_object(py))
208-
} else {
209-
None
210-
}
211-
})
207+
let code_opt = SEEN_CODE_PTRS.with(|seen| {
208+
if seen.borrow_mut().insert(code_ptr) {
209+
Some(code.clone().unbind())
210+
} else {
211+
None
212+
}
212213
});
213214

214215
enqueue_event(TraceEvent::PyStart {
@@ -223,10 +224,10 @@ pub fn py_start_callback(code: &PyAny, instruction_offset: i32) {
223224
#[pyfunction]
224225
pub fn jump_callback(
225226
py: Python<'_>,
226-
code: &PyAny,
227+
code: &Bound<'_, PyAny>,
227228
instruction_offset: i32,
228229
destination_offset: i32,
229-
) -> PyObject {
230+
) -> Py<PyAny> {
230231
let ts = get_ts();
231232
let code_ptr = code.as_ptr() as usize;
232233

@@ -246,7 +247,7 @@ pub fn jump_callback(
246247
};
247248

248249
if !trace_allowed {
249-
return py.None().to_object(py);
250+
return py.None();
250251
}
251252

252253
enqueue_event(TraceEvent::Jump {
@@ -275,11 +276,15 @@ pub fn jump_callback(
275276
}
276277
}
277278

278-
py.None().to_object(py)
279+
py.None()
279280
}
280281

281282
#[pyfunction]
282-
pub fn py_return_callback(code: &PyAny, _instruction_offset: i32, _retval: &PyAny) {
283+
pub fn py_return_callback(
284+
code: &Bound<'_, PyAny>,
285+
_instruction_offset: i32,
286+
_retval: &Bound<'_, PyAny>,
287+
) {
283288
let code_ptr = code.as_ptr() as usize;
284289

285290
let trace_allowed = match code_ptr_allowed(code_ptr) {
@@ -310,14 +315,14 @@ pub fn py_return_callback(code: &PyAny, _instruction_offset: i32, _retval: &PyAn
310315
}
311316

312317
#[pyfunction]
313-
pub fn get_tracing_stats(py: Python<'_>) -> PyResult<PyObject> {
318+
pub fn get_tracing_stats(py: Python<'_>) -> PyResult<Py<PyAny>> {
314319
let summary = crate::vortex::ocular::state::get_summary();
315320
let out = pyo3::types::PyDict::new(py);
316321
out.set_item("processed_events", summary.processed_events)?;
317322
out.set_item("instruction_events", summary.instruction_events)?;
318323
out.set_item("unique_instruction_sites", summary.unique_instruction_sites)?;
319324
out.set_item("loop_trace_count", summary.loop_trace_count)?;
320-
Ok(out.to_object(py))
325+
Ok(out.unbind().into_any())
321326
}
322327

323328
#[pyfunction]
@@ -328,7 +333,7 @@ pub fn is_tracing_active() -> bool {
328333
#[pyfunction]
329334
#[pyo3(signature = (mode="precise", deinstrument_threshold=0, exclude=None, include_only=None))]
330335
pub fn start_tracing(
331-
py: Python,
336+
py: Python<'_>,
332337
mode: &str,
333338
deinstrument_threshold: u32,
334339
exclude: Option<Vec<String>>,
@@ -358,18 +363,18 @@ pub fn start_tracing(
358363
}
359364
}
360365

361-
let sys = py.import("sys")?;
366+
let sys = py.import(pyo3::ffi::c_str!("sys"))?;
362367
let sys_mon = sys.getattr("monitoring")?;
363368

364369
DISABLE_OBJ.get_or_init(|| {
365370
sys_mon
366371
.getattr("DISABLE")
367-
.map(|o| o.to_object(py))
368-
.unwrap_or_else(|_| py.None().to_object(py))
372+
.map(|o| o.unbind())
373+
.unwrap_or_else(|_| py.None())
369374
});
370375

371376
let tool_id = sys_mon.getattr("DEBUGGER_ID")?;
372-
let tool_id_obj = tool_id.to_object(py);
377+
let tool_id_obj = tool_id.clone().unbind();
373378
sys_mon.call_method0("restart_events")?;
374379
sys_mon.call_method1("use_tool_id", (tool_id_obj.clone_ref(py), "ocular"))?;
375380

@@ -380,57 +385,65 @@ pub fn start_tracing(
380385
let branch_event = events.getattr("BRANCH")?;
381386
let py_return_event = events.getattr("PY_RETURN")?;
382387

383-
let start_cb = pyo3::wrap_pyfunction!(py_start_callback)(py)?;
388+
// We can use a dummy module or the parent module here for wrap_pyfunction.
389+
// However, since we are inside a function and just need a PyFunction,
390+
// we can use wrap_pyfunction! with the current python handle's module?
391+
// Actually, in 0.28, we can just use Bound::new(py, PyFunction::...) or similar.
392+
// But the easiest way is to use wrap_pyfunction! and provide a Bound<PyModule>.
393+
// Since we don't have one here, we'll create a temporary one.
394+
let m = PyModule::new(py, "ocular_internal")?;
395+
396+
let start_cb = pyo3::wrap_pyfunction!(py_start_callback, &m)?;
384397
sys_mon.call_method1(
385398
"register_callback",
386399
(
387400
tool_id_obj.clone_ref(py),
388-
py_start_event.to_object(py),
401+
py_start_event.clone().unbind(),
389402
start_cb,
390403
),
391404
)?;
392405

393-
let jump_cb = pyo3::wrap_pyfunction!(jump_callback)(py)?;
394-
let jump_cb_obj = jump_cb.to_object(py);
406+
let jump_cb = pyo3::wrap_pyfunction!(jump_callback, &m)?;
407+
let jump_cb_obj = jump_cb.clone().unbind();
395408
sys_mon.call_method1(
396409
"register_callback",
397410
(
398411
tool_id_obj.clone_ref(py),
399-
jump_event.to_object(py),
400-
jump_cb_obj,
412+
jump_event.clone().unbind(),
413+
jump_cb_obj.clone_ref(py),
401414
),
402415
)?;
403416
sys_mon.call_method1(
404417
"register_callback",
405418
(
406419
tool_id_obj.clone_ref(py),
407-
branch_event.to_object(py),
420+
branch_event.clone().unbind(),
408421
jump_cb,
409422
),
410423
)?;
411424

412-
let return_cb = pyo3::wrap_pyfunction!(py_return_callback)(py)?;
425+
let return_cb = pyo3::wrap_pyfunction!(py_return_callback, &m)?;
413426
sys_mon.call_method1(
414427
"register_callback",
415428
(
416429
tool_id_obj.clone_ref(py),
417-
py_return_event.to_object(py),
430+
py_return_event.clone().unbind(),
418431
return_cb,
419432
),
420433
)?;
421434

422-
let mut combined_events = py_start_event.extract::<i32>()?
435+
let mut combined_events: i32 = py_start_event.extract::<i32>()?
423436
| jump_event.extract::<i32>()?
424437
| branch_event.extract::<i32>()?
425438
| py_return_event.extract::<i32>()?;
426439

427440
if is_precise {
428-
let inst_cb = pyo3::wrap_pyfunction!(instruction_callback)(py)?;
441+
let inst_cb = pyo3::wrap_pyfunction!(instruction_callback, &m)?;
429442
sys_mon.call_method1(
430443
"register_callback",
431444
(
432445
tool_id_obj.clone_ref(py),
433-
instruction_event.to_object(py),
446+
instruction_event.clone().unbind(),
434447
inst_cb,
435448
),
436449
)?;
@@ -440,7 +453,7 @@ pub fn start_tracing(
440453
"register_callback",
441454
(
442455
tool_id_obj.clone_ref(py),
443-
instruction_event.to_object(py),
456+
instruction_event.clone().unbind(),
444457
py.None(),
445458
),
446459
)?;
@@ -452,11 +465,11 @@ pub fn start_tracing(
452465
}
453466

454467
#[pyfunction]
455-
pub fn stop_tracing(py: Python) -> PyResult<()> {
456-
let sys = py.import("sys")?;
468+
pub fn stop_tracing(py: Python<'_>) -> PyResult<()> {
469+
let sys = py.import(pyo3::ffi::c_str!("sys"))?;
457470
let sys_mon = sys.getattr("monitoring")?;
458471
let tool_id = sys_mon.getattr("DEBUGGER_ID")?;
459-
let tool_id_obj = tool_id.to_object(py);
472+
let tool_id_obj = tool_id.clone().unbind();
460473

461474
let events = sys_mon.getattr("events")?;
462475
let instruction_event = events.getattr("INSTRUCTION")?;
@@ -512,7 +525,7 @@ pub fn stop_tracing(py: Python) -> PyResult<()> {
512525
if IS_RUNNING.swap(false, Ordering::Relaxed) {
513526
if let Ok(mut thread_guard) = WORKER_THREAD.lock() {
514527
if let Some(handle) = thread_guard.take() {
515-
py.allow_threads(|| {
528+
py.detach(|| {
516529
let _ = handle.join();
517530
});
518531
}

0 commit comments

Comments
 (0)