Skip to content

Commit

Permalink
avm2: use RefLock instead of GcCell in avm2's scope chain
Browse files Browse the repository at this point in the history
Also bumps gc-arena to be able to use `Write<Option<_>>::as_write`
  • Loading branch information
moulins authored and torokati44 committed Aug 26, 2023
1 parent ecb5036 commit 87437c1
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 44 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
48 changes: 24 additions & 24 deletions core/src/avm2/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<PropertyMap<'gc, Object<'gc>>>,
cache: Option<RefLock<PropertyMap<'gc, Object<'gc>>>>,
}

impl<'gc> ScopeContainer<'gc> {
fn new(scopes: Vec<Scope<'gc>>) -> 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 }
}

Expand Down Expand Up @@ -91,7 +93,7 @@ impl<'gc> ScopeContainer<'gc> {
#[derive(Collect, Clone, Copy)]
#[collect(no_drop)]
pub struct ScopeChain<'gc> {
container: Option<GcCell<'gc, ScopeContainer<'gc>>>,
container: Option<Gc<'gc, ScopeContainer<'gc>>>,
domain: Domain<'gc>,
}

Expand Down Expand Up @@ -123,32 +125,31 @@ 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,
}
}
None => {
// 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,
}
}
}
}

pub fn get(&self, index: usize) -> Option<Scope<'gc>> {
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)
}

Expand All @@ -164,7 +165,7 @@ impl<'gc> ScopeChain<'gc> {
) -> Result<Option<(Option<Namespace<'gc>>, 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();
Expand Down Expand Up @@ -199,25 +200,24 @@ impl<'gc> ScopeChain<'gc> {
) -> Result<Option<Object<'gc>>, 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))
Expand Down
25 changes: 11 additions & 14 deletions core/src/player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
)
}))),
})
});

Expand Down
2 changes: 1 addition & 1 deletion ruffle_gc_arena/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

0 comments on commit 87437c1

Please sign in to comment.