fix: Remove abandoned im crate in favor of Arc::make_mut (#305)

With a populated outer scope, do a few nested `with_scope` calls and add tags / breadcrumbs in those.
Perf numbers are similar to before:
-8% for tags (speedup) and +2% for breadcrumbs, so quite negligible.

sentry-core/Cargo.toml

@@ -20,7 +20,7 @@ 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_"]
@@ -30,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"] }

sentry-core/benches/scope_benchmark.rs

@@ -21,6 +21,7 @@
 //! `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;
 
@@ -34,12 +35,7 @@ use sentry_core as sentry;
 /// 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) {
-    for i in 0..50 {
-        sentry::add_breadcrumb(|| Breadcrumb {
-            message: Some(format!("Breadcrumb {}", i)),
-            ..Default::default()
-        });
-    }
+    add_breadcrumbs(0..50);
 
     if capture {
         sentry::capture_message("capturing on outer scope", sentry::Level::Info);
@@ -49,12 +45,7 @@ fn scope_with_breadcrumbs(capture: bool) {
         |_| (),
         || {
             // 50 + 70 exceeds the default max_breadcrumbs of 100
-            for i in 0..70 {
-                sentry::add_breadcrumb(|| Breadcrumb {
-                    message: Some(format!("Breadcrumb {}", i)),
-                    ..Default::default()
-                });
-            }
+            add_breadcrumbs(50..120);
 
             if capture {
                 sentry::capture_message("capturing within a nested scope", sentry::Level::Info);
@@ -65,27 +56,37 @@ fn scope_with_breadcrumbs(capture: bool) {
     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| {
-        for i in 0..20 {
-            scope.set_tag(&format!("tag {}", i), format!("tag value {}", i));
-        }
-    });
+    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| {
-            for i in 10..30 {
-                // since this is a hashmap, we basically overwrite 10, and add 10 new tags
-                scope.set_tag(&format!("tag {}", i), format!("tag value {}", i));
-            }
+            // since this is a hashmap, we basically overwrite 10, and add 10 new tags
+            add_tags(scope, 10..30)
         },
         || {
             if capture {
@@ -97,6 +98,31 @@ fn scope_with_tags(capture: bool) {
     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 {
@@ -133,6 +159,11 @@ fn scope_benchmark(c: &mut Criterion) {
             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();
@@ -150,6 +181,11 @@ fn scope_benchmark(c: &mut Criterion) {
             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();

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,