From 87437c1d6b278b3c6c741b7194379c69852fc3e6 Mon Sep 17 00:00:00 2001 From: Moulins Date: Fri, 25 Aug 2023 15:24:51 +0200 Subject: [PATCH] avm2: use `RefLock` instead of `GcCell` in avm2's scope chain Also bumps gc-arena to be able to use `Write>::as_write` --- Cargo.lock | 8 +++---- Cargo.toml | 2 +- core/src/avm2/scope.rs | 48 +++++++++++++++++++------------------- core/src/player.rs | 25 +++++++++----------- ruffle_gc_arena/Cargo.toml | 2 +- 5 files changed, 41 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e35e28a07e8f..3858690910f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1828,8 +1828,8 @@ dependencies = [ [[package]] name = "gc-arena" -version = "0.3.1" -source = "git+https://github.com/kyren/gc-arena?rev=ad3e24f89c78d8bb94db18383987290f88e4edcb#ad3e24f89c78d8bb94db18383987290f88e4edcb" +version = "0.3.3" +source = "git+https://github.com/kyren/gc-arena?rev=efd89fc683c6bb456af3e226c33763cb822645e9#efd89fc683c6bb456af3e226c33763cb822645e9" dependencies = [ "gc-arena-derive", "sptr", @@ -1837,8 +1837,8 @@ dependencies = [ [[package]] name = "gc-arena-derive" -version = "0.3.1" -source = "git+https://github.com/kyren/gc-arena?rev=ad3e24f89c78d8bb94db18383987290f88e4edcb#ad3e24f89c78d8bb94db18383987290f88e4edcb" +version = "0.3.3" +source = "git+https://github.com/kyren/gc-arena?rev=efd89fc683c6bb456af3e226c33763cb822645e9#efd89fc683c6bb456af3e226c33763cb822645e9" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 1c0a6b5da30d..844e8e73d0c7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ repository = "https://github.com/ruffle-rs/ruffle" version = "0.1.0" [workspace.dependencies] -# gc-arena = { git = "https://github.com/kyren/gc-arena", rev = "ad3e24f89c78d8bb94db18383987290f88e4edcb" } +# gc-arena = { git = "https://github.com/kyren/gc-arena", rev = "efd89fc683c6bb456af3e226c33763cb822645e9" } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } naga = { version = "0.12.3", features = ["validate", "wgsl-out"] } diff --git a/core/src/avm2/scope.rs b/core/src/avm2/scope.rs index 232d1b37d882..9d94f26ba547 100644 --- a/core/src/avm2/scope.rs +++ b/core/src/avm2/scope.rs @@ -7,7 +7,9 @@ use crate::avm2::value::Value; use crate::avm2::Error; use crate::avm2::{Multiname, Namespace}; use core::fmt; -use gc_arena::{Collect, GcCell, Mutation}; +use gc_arena::barrier::field; +use gc_arena::lock::RefLock; +use gc_arena::{Collect, Gc, Mutation}; use super::property_map::PropertyMap; @@ -57,12 +59,12 @@ struct ScopeContainer<'gc> { /// The cache of this ScopeChain. A value of None indicates that caching is disabled /// for this ScopeChain. - cache: Option>>, + cache: Option>>>, } impl<'gc> ScopeContainer<'gc> { fn new(scopes: Vec>) -> Self { - let cache = (!scopes.iter().any(|scope| scope.with)).then(PropertyMap::default); + let cache = (!scopes.iter().any(|scope| scope.with)).then(RefLock::default); Self { scopes, cache } } @@ -91,7 +93,7 @@ impl<'gc> ScopeContainer<'gc> { #[derive(Collect, Clone, Copy)] #[collect(no_drop)] pub struct ScopeChain<'gc> { - container: Option>>, + container: Option>>, domain: Domain<'gc>, } @@ -123,10 +125,10 @@ impl<'gc> ScopeChain<'gc> { Some(container) => { // The new ScopeChain is created by cloning the scopes of this ScopeChain, // and pushing the new scopes on top of that. - let mut cloned = container.read().scopes.clone(); + let mut cloned = container.scopes.clone(); cloned.extend_from_slice(new_scopes); Self { - container: Some(GcCell::new(mc, ScopeContainer::new(cloned))), + container: Some(Gc::new(mc, ScopeContainer::new(cloned))), domain: self.domain, } } @@ -134,7 +136,7 @@ impl<'gc> ScopeChain<'gc> { // We are chaining on top of an empty ScopeChain, so we don't actually // need to chain anything. Self { - container: Some(GcCell::new(mc, ScopeContainer::new(new_scopes.to_vec()))), + container: Some(Gc::new(mc, ScopeContainer::new(new_scopes.to_vec()))), domain: self.domain, } } @@ -142,13 +144,12 @@ impl<'gc> ScopeChain<'gc> { } pub fn get(&self, index: usize) -> Option> { - self.container - .and_then(|container| container.read().get(index)) + self.container.and_then(|container| container.get(index)) } pub fn is_empty(&self) -> bool { self.container - .map(|container| container.read().is_empty()) + .map(|container| container.is_empty()) .unwrap_or(true) } @@ -164,7 +165,7 @@ impl<'gc> ScopeChain<'gc> { ) -> Result>, Object<'gc>)>, Error<'gc>> { if let Some(container) = self.container { // We skip the scope at depth 0 (the global scope). The global scope will be checked in a different phase. - for scope in container.read().scopes.iter().skip(1).rev() { + for scope in container.scopes.iter().skip(1).rev() { // NOTE: We are manually searching the vtable's traits so we can figure out which namespace the trait // belongs to. let values = scope.values(); @@ -199,25 +200,24 @@ impl<'gc> ScopeChain<'gc> { ) -> Result>, Error<'gc>> { // First we check the cache of our container if let Some(container) = self.container { - if let Some(cache) = &container.read().cache { - let cached = cache.get_for_multiname(multiname); - if cached.is_some() { - return Ok(cached.cloned()); + if let Some(cache) = &container.cache { + if let Some(cached) = cache.borrow().get_for_multiname(multiname) { + return Ok(Some(*cached)); } } } let found = self.find_internal(multiname, activation)?; if let (Some((Some(ns), obj)), Some(container)) = (found, self.container) { // We found a value that hasn't been cached yet, so let's try to cache it now - let mut write = container.write(activation.context.gc_context); - if let Some(ref mut cache) = write.cache { - cache.insert_with_namespace( - ns, - multiname - .local_name() - .expect("Resolvable multinames should always have a local name"), - obj, - ); + let cache = field!(Gc::write(activation.gc(), container), ScopeContainer, cache); + if let Some(cache) = cache.as_write() { + let name = multiname + .local_name() + .expect("Resolvable multinames should always have a local name"); + cache + .unlock() + .borrow_mut() + .insert_with_namespace(ns, name, obj); } } Ok(found.map(|o| o.1)) diff --git a/core/src/player.rs b/core/src/player.rs index e5154298c547..2d23dfcaccc1 100644 --- a/core/src/player.rs +++ b/core/src/player.rs @@ -47,7 +47,7 @@ use crate::stub::StubCollection; use crate::tag_utils::SwfMovie; use crate::timer::Timers; use crate::vminterface::Instantiator; -use gc_arena::{ArenaParameters, Collect, DynamicRootSet, GcCell, Rootable}; +use gc_arena::{Collect, DynamicRootSet, GcCell, Rootable}; use instant::Instant; use rand::{rngs::SmallRng, SeedableRng}; use ruffle_render::backend::{null::NullRenderer, RenderBackend, ViewportDimensions}; @@ -2513,19 +2513,16 @@ impl PlayerBuilder { debug_ui: Default::default(), // GC data - gc_arena: Rc::new(RefCell::new(GcArena::new( - ArenaParameters::default(), - |gc_context| { - Self::create_gc_root( - gc_context, - player_version, - self.fullscreen, - fake_movie.clone(), - self.external_interface_providers, - self.fs_command_provider, - ) - }, - ))), + gc_arena: Rc::new(RefCell::new(GcArena::new(|gc_context| { + Self::create_gc_root( + gc_context, + player_version, + self.fullscreen, + fake_movie.clone(), + self.external_interface_providers, + self.fs_command_provider, + ) + }))), }) }); diff --git a/ruffle_gc_arena/Cargo.toml b/ruffle_gc_arena/Cargo.toml index b8183b281d9b..aa08f8dd49f8 100644 --- a/ruffle_gc_arena/Cargo.toml +++ b/ruffle_gc_arena/Cargo.toml @@ -9,4 +9,4 @@ license.workspace = true repository.workspace = true [dependencies] -gc-arena = { git = "https://github.com/kyren/gc-arena", rev = "ad3e24f89c78d8bb94db18383987290f88e4edcb" } +gc-arena = { git = "https://github.com/kyren/gc-arena", rev = "efd89fc683c6bb456af3e226c33763cb822645e9" }