Skip to content

Commit

Permalink
Change thread_local RefCells to Cells (#32351)
Browse files Browse the repository at this point in the history
GitOrigin-RevId: 2a3c4720eab817983fba6f93e0fdbf7e5d6f7f8f
  • Loading branch information
goffrie authored and Convex, Inc. committed Dec 17, 2024
1 parent 7e9aa71 commit b2d46fd
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 23 deletions.
20 changes: 6 additions & 14 deletions crates/application/src/function_log.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
cell::RefCell,
cell::Cell,
cmp::Ordering,
collections::{
BTreeMap,
Expand Down Expand Up @@ -1323,24 +1323,16 @@ impl<RT: Runtime> Inner<RT> {
// Only log an error to tracing and/or Sentry at most once every 10 seconds per
// thread.
thread_local! {
static LAST_LOGGED_ERROR: RefCell<Option<SystemTime>> = const { RefCell::new(None) };
static LAST_LOGGED_ERROR: Cell<Option<SystemTime>> = const { Cell::new(None) };
}
let should_log = LAST_LOGGED_ERROR.with_borrow_mut(|cell| {
let now = SystemTime::now();
if let Some(ref mut last_logged_error) = *cell {
let duration = now
.duration_since(*last_logged_error)
.unwrap_or(Duration::ZERO);
if duration < Duration::from_secs(10) {
return false;
}
}
*cell = Some(now);
true
let now = SystemTime::now();
let should_log = LAST_LOGGED_ERROR.get().map_or(true, |last_logged| {
now.duration_since(last_logged).unwrap_or(Duration::ZERO) >= Duration::from_secs(10)
});
if !should_log {
return;
}
LAST_LOGGED_ERROR.set(Some(now));
tracing::error!("Failed to log execution metrics: {}", error);
if let UdfMetricsError::InternalError(mut e) = error {
report_error_sync(&mut e);
Expand Down
14 changes: 5 additions & 9 deletions crates/common/src/tracing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub use cstr::cstr;
#[cfg(feature = "tracy-tracing")]
mod tracing_on {
use std::{
cell::RefCell,
cell::Cell,
cmp::Reverse,
collections::BinaryHeap,
ffi::{
Expand Down Expand Up @@ -95,7 +95,7 @@ mod tracing_on {
// into the non-overlapping fibers Tracy expects by using a thread-local to
// keep track of the current fiber, stash it when entering a new one, and
// restore it whne existing the old one.
thread_local!(static CURRENT_FIBER: RefCell<Option<usize>> = RefCell::new(None));
thread_local!(static CURRENT_FIBER: Cell<Option<usize>> = Cell::new(None));

#[pin_project(PinnedDrop)]
pub struct InstrumentedFuture<F: Future> {
Expand Down Expand Up @@ -136,7 +136,7 @@ mod tracing_on {

// First, leave our parent fiber's execution, and stash its number on the stack
// in `current_fiber`.
let current_fiber = CURRENT_FIBER.with(|f| f.borrow_mut().take());
let current_fiber = CURRENT_FIBER.take();
if current_fiber.is_some() {
unsafe {
sys::___tracy_fiber_leave();
Expand Down Expand Up @@ -190,7 +190,7 @@ mod tracing_on {
};

// Set ourselves as the current fiber in the thread local.
CURRENT_FIBER.with(|f| *f.borrow_mut() = Some(fiber_ix));
CURRENT_FIBER.set(Some(fiber_ix));

// Poll our future, which may end up polling other instrumented futures.
let r = this.inner.poll(cx);
Expand All @@ -214,11 +214,7 @@ mod tracing_on {
}

// Restore our parent fiber in the thread local.
CURRENT_FIBER.with(|f| {
let mut f = f.borrow_mut();
assert_eq!(*f, Some(fiber_ix));
*f = current_fiber;
});
assert_eq!(CURRENT_FIBER.replace(current_fiber), Some(fiber_ix));

r
}
Expand Down

0 comments on commit b2d46fd

Please sign in to comment.