fix: Remove abandoned im crate in favor of Arc::make_mut

sentry-core/Cargo.toml

@@ -14,9 +14,13 @@ edition = "2018"
 [package.metadata.docs.rs]
 all-features = true
 
+[[bench]]
+name = "scope_benchmark"
+harness = false
+
 [features]
 default = []
-client = ["im", "rand"]
+client = ["rand"]
 # I would love to just have a `log` feature, but this is used inside a macro,
 # and macros actually expand features (and extern crate) where they are used!
 debug-logs = ["log_"]
@@ -26,7 +30,6 @@ test = ["client"]
 sentry-types = { version = "0.21.0", path = "../sentry-types" }
 serde = { version = "1.0.104", features = ["derive"] }
 lazy_static = "1.4.0"
-im = { version = "15.0.0", optional = true }
 rand = { version = "0.7.3", optional = true }
 serde_json = "1.0.46"
 log_ = { package = "log", version = "0.4.8", optional = true, features = ["std"] }
@@ -38,4 +41,5 @@ log_ = { package = "log", version = "0.4.8", optional = true, features = ["std"]
 sentry = { version = "0.21.0", path = "../sentry", default-features = false, features = ["test"] }
 thiserror = "1.0.15"
 anyhow = "1.0.30"
-tokio = { version = "1.0", features = ["rt", "macros"] }
+tokio = { version = "1.0", features = ["rt", "rt-multi-thread", "macros"] }
+criterion = "0.3"

sentry-core/benches/scope_benchmark.rs

@@ -0,0 +1,195 @@
+//! Sentry Scope Benchmarks
+//!
+//! Run the benchmarks with:
+//!
+//! ```text
+//! $ cargo bench -p sentry-core
+//! ```
+//!
+//! We have the following tests:
+//! * [`scope_with_breadcrumbs`]
+//! * [`scope_with_tags`]
+//!
+//! We do test the following permutations:
+//! * No active Hub is bound, meaning most API functions will just noop
+//! * Doing scope manipulation with an active Hub, but *not* capturing any messages
+//! * Doing scope manipulation with an active Hub, and capturing messages, discarding
+//!   them in the transport layer.
+//!
+//! # Testing the minimal API
+//! Due to our circular dev-dependency on `sentry`, we will *always* run with the
+//! `client` feature. To test without it, one needs to comment the circular dependency
+//! before running the benchmark.
+
+use std::ops::Range;
+#[cfg(feature = "client")]
+use std::sync::Arc;
+
+use criterion::{criterion_group, criterion_main, Criterion};
+use sentry::protocol::Breadcrumb;
+#[cfg(not(feature = "client"))]
+use sentry_core as sentry;
+
+/// Tests Scopes with Breadcrumbs
+///
+/// This uses the [`sentry::add_breadcrumb`] API in *callback mode*, which means
+/// it is essentially a noop when the current Hub is inactive.
+fn scope_with_breadcrumbs(capture: bool) {
+    add_breadcrumbs(0..50);
+
+    if capture {
+        sentry::capture_message("capturing on outer scope", sentry::Level::Info);
+    }
+
+    sentry::with_scope(
+        |_| (),
+        || {
+            // 50 + 70 exceeds the default max_breadcrumbs of 100
+            add_breadcrumbs(50..120);
+
+            if capture {
+                sentry::capture_message("capturing within a nested scope", sentry::Level::Info);
+            }
+        },
+    );
+
+    sentry::configure_scope(|scope| scope.clear());
+}
+
+/// This does multiple nested [`sentry::with_scope`] calls that manipulates breadcrumbs
+fn outer_scope_with_breadcrumbs() {
+    sentry::with_scope(
+        |_| (),
+        || {
+            add_breadcrumbs(20..50);
+            sentry::with_scope(
+                |_| (),
+                || {
+                    add_breadcrumbs(50..80);
+                },
+            )
+        },
+    )
+}
+
+/// Tests Scopes with Tags
+///
+/// This uses the [`sentry::Scope::set_tag`] function to define, and then overwrite/extend
+/// the set of tags.
+fn scope_with_tags(capture: bool) {
+    sentry::configure_scope(|scope| add_tags(scope, 0..20));
+
+    if capture {
+        sentry::capture_message("capturing on outer scope", sentry::Level::Info);
+    }
+
+    sentry::with_scope(
+        |scope| {
+            // since this is a hashmap, we basically overwrite 10, and add 10 new tags
+            add_tags(scope, 10..30)
+        },
+        || {
+            if capture {
+                sentry::capture_message("capturing within a nested scope", sentry::Level::Info);
+            }
+        },
+    );
+
+    sentry::configure_scope(|scope| scope.clear());
+}
+
+/// This does multiple nested [`sentry::with_scope`] calls that manipulates tags
+fn outer_scope_with_tags() {
+    sentry::with_scope(
+        |scope| add_tags(scope, 20..22),
+        || sentry::with_scope(|scope| add_tags(scope, 22..24), || {}),
+    )
+}
+
+/// Adds a bunch of breadcrumbs
+fn add_breadcrumbs(range: Range<usize>) {
+    for i in range {
+        sentry::add_breadcrumb(|| Breadcrumb {
+            message: Some(format!("Breadcrumb {}", i)),
+            ..Default::default()
+        });
+    }
+}
+
+/// Adds a bunch of tags
+fn add_tags(scope: &mut sentry::Scope, range: Range<usize>) {
+    for i in range {
+        scope.set_tag(&format!("tag {}", i), format!("tag value {}", i));
+    }
+}
+
+/// Returns a new *active* [`sentry::Hub`] which discards Events in the Transport.
+#[cfg(feature = "client")]
+fn discarding_hub() -> sentry::Hub {
+    struct NoopTransport;
+
+    impl sentry::Transport for NoopTransport {
+        fn send_envelope(&self, envelope: sentry::Envelope) {
+            drop(envelope)
+        }
+    }
+
+    let client = Arc::new(sentry::Client::from(sentry::ClientOptions {
+        dsn: Some("https://public@sentry.invalid/1".parse().unwrap()),
+        // lol, this double arcing -_-
+        transport: Some(Arc::new(Arc::new(NoopTransport))),
+        // before_send: Some(Arc::new(|_| None)),
+        ..Default::default()
+    }));
+    let scope = Arc::new(sentry::Scope::default());
+    sentry::Hub::new(Some(client), scope)
+}
+
+fn scope_benchmark(c: &mut Criterion) {
+    let mut group = c.benchmark_group("scoped-tags");
+
+    group.bench_function("no-client", |b| b.iter(|| scope_with_tags(true)));
+    #[cfg(feature = "client")]
+    {
+        group.bench_function("with-client", |b| {
+            let hub = Arc::new(discarding_hub());
+            sentry::Hub::run(hub, || b.iter(|| scope_with_tags(false)))
+        });
+        group.bench_function("dropping-client", |b| {
+            let hub = Arc::new(discarding_hub());
+            sentry::Hub::run(hub, || b.iter(|| scope_with_tags(true)))
+        });
+        group.bench_function("outer-scope", |b| {
+            let hub = Arc::new(discarding_hub());
+            sentry::configure_scope(|scope| add_tags(scope, 0..20));
+            sentry::Hub::run(hub, || b.iter(outer_scope_with_tags))
+        });
+    }
+
+    group.finish();
+
+    let mut group = c.benchmark_group("scoped-breadcrumbs");
+
+    group.bench_function("no-client", |b| b.iter(|| scope_with_breadcrumbs(true)));
+    #[cfg(feature = "client")]
+    {
+        group.bench_function("with-client", |b| {
+            let hub = Arc::new(discarding_hub());
+            sentry::Hub::run(hub, || b.iter(|| scope_with_breadcrumbs(false)))
+        });
+        group.bench_function("dropping-client", |b| {
+            let hub = Arc::new(discarding_hub());
+            sentry::Hub::run(hub, || b.iter(|| scope_with_breadcrumbs(true)))
+        });
+        group.bench_function("outer-scope", |b| {
+            let hub = Arc::new(discarding_hub());
+            add_breadcrumbs(0..20);
+            sentry::Hub::run(hub, || b.iter(outer_scope_with_breadcrumbs))
+        });
+    }
+
+    group.finish();
+}
+
+criterion_group!(benches, scope_benchmark);
+criterion_main!(benches);

sentry-core/src/hub.rs

@@ -427,16 +427,17 @@ impl Hub {
                 if let Some(ref client) = top.client {
                     let scope = Arc::make_mut(&mut top.scope);
                     let options = client.options();
+                    let breadcrumbs = Arc::make_mut(&mut scope.breadcrumbs);
                     for breadcrumb in breadcrumb.into_breadcrumbs() {
                         let breadcrumb_opt = match options.before_breadcrumb {
                             Some(ref callback) => callback(breadcrumb),
                             None => Some(breadcrumb)
                         };
                         if let Some(breadcrumb) = breadcrumb_opt {
-                            scope.breadcrumbs.push_back(breadcrumb);
+                            breadcrumbs.push_back(breadcrumb);
                         }
-                        while scope.breadcrumbs.len() > options.max_breadcrumbs {
-                            scope.breadcrumbs.pop_front();
+                        while breadcrumbs.len() > options.max_breadcrumbs {
+                            breadcrumbs.pop_front();
                         }
                     }
                 }

sentry-core/src/scope/real.rs

@@ -1,4 +1,5 @@
 use std::borrow::Cow;
+use std::collections::{HashMap, VecDeque};
 use std::fmt;
 use std::sync::{Arc, Mutex, PoisonError, RwLock};
 
@@ -36,12 +37,12 @@ pub struct Scope {
     pub(crate) level: Option<Level>,
     pub(crate) fingerprint: Option<Arc<[Cow<'static, str>]>>,
     pub(crate) transaction: Option<Arc<str>>,
-    pub(crate) breadcrumbs: im::Vector<Breadcrumb>,
+    pub(crate) breadcrumbs: Arc<VecDeque<Breadcrumb>>,
     pub(crate) user: Option<Arc<User>>,
-    pub(crate) extra: im::HashMap<String, Value>,
-    pub(crate) tags: im::HashMap<String, String>,
-    pub(crate) contexts: im::HashMap<String, Context>,
-    pub(crate) event_processors: im::Vector<Arc<EventProcessor>>,
+    pub(crate) extra: Arc<HashMap<String, Value>>,
+    pub(crate) tags: Arc<HashMap<String, String>>,
+    pub(crate) contexts: Arc<HashMap<String, Context>>,
+    pub(crate) event_processors: Arc<Vec<Arc<EventProcessor>>>,
     pub(crate) session: Arc<Mutex<Option<Session>>>,
 }
 
@@ -157,7 +158,7 @@ impl Scope {
 
     /// Deletes current breadcrumbs from the scope.
     pub fn clear_breadcrumbs(&mut self) {
-        self.breadcrumbs.clear();
+        self.breadcrumbs = Default::default();
     }
 
     /// Sets a level override.
@@ -183,42 +184,42 @@ impl Scope {
 
     /// Sets a tag to a specific value.
     pub fn set_tag<V: ToString>(&mut self, key: &str, value: V) {
-        self.tags.insert(key.to_string(), value.to_string());
+        Arc::make_mut(&mut self.tags).insert(key.to_string(), value.to_string());
     }
 
     /// Removes a tag.
     ///
     /// If the tag is not set, does nothing.
     pub fn remove_tag(&mut self, key: &str) {
-        self.tags.remove(key);
+        Arc::make_mut(&mut self.tags).remove(key);
     }
 
     /// Sets a context for a key.
     pub fn set_context<C: Into<Context>>(&mut self, key: &str, value: C) {
-        self.contexts.insert(key.to_string(), value.into());
+        Arc::make_mut(&mut self.contexts).insert(key.to_string(), value.into());
     }
 
     /// Removes a context for a key.
     pub fn remove_context(&mut self, key: &str) {
-        self.contexts.remove(key);
+        Arc::make_mut(&mut self.contexts).remove(key);
     }
 
     /// Sets a extra to a specific value.
     pub fn set_extra(&mut self, key: &str, value: Value) {
-        self.extra.insert(key.to_string(), value);
+        Arc::make_mut(&mut self.extra).insert(key.to_string(), value);
     }
 
     /// Removes a extra.
     pub fn remove_extra(&mut self, key: &str) {
-        self.extra.remove(key);
+        Arc::make_mut(&mut self.extra).remove(key);
     }
 
     /// Add an event processor to the scope.
     pub fn add_event_processor(
         &mut self,
         f: Box<dyn Fn(Event<'static>) -> Option<Event<'static>> + Send + Sync>,
     ) {
-        self.event_processors.push_back(Arc::new(f));
+        Arc::make_mut(&mut self.event_processors).push(Arc::new(f));
     }
 
     /// Applies the contained scoped data to fill an event.
@@ -234,12 +235,18 @@ impl Scope {
             }
         }
 
+        event.breadcrumbs.extend(self.breadcrumbs.iter().cloned());
         event
-            .breadcrumbs
-            .extend(self.breadcrumbs.clone().into_iter());
-        event.extra.extend(self.extra.clone().into_iter());
-        event.tags.extend(self.tags.clone().into_iter());
-        event.contexts.extend(self.contexts.clone().into_iter());
+            .extra
+            .extend(self.extra.iter().map(|(k, v)| (k.to_owned(), v.to_owned())));
+        event
+            .tags
+            .extend(self.tags.iter().map(|(k, v)| (k.to_owned(), v.to_owned())));
+        event.contexts.extend(
+            self.contexts
+                .iter()
+                .map(|(k, v)| (k.to_owned(), v.to_owned())),
+        );
 
         if event.transaction.is_none() {
             if let Some(txn) = self.transaction.as_deref() {
@@ -255,7 +262,7 @@ impl Scope {
             }
         }
 
-        for processor in &self.event_processors {
+        for processor in self.event_processors.as_ref() {
             let id = event.event_id;
             event = match processor(event) {
                 Some(event) => event,