Skip to content

Commit

Permalink
avm2: Do not allocate for Namespace::any()
Browse files Browse the repository at this point in the history
`Namespace` now holds a `Option<Gc<_>>` internally, with `None`
representing the Any namespace.
  • Loading branch information
moulins committed Sep 5, 2024
1 parent 9e18f66 commit 74fc52b
Show file tree
Hide file tree
Showing 19 changed files with 71 additions and 75 deletions.
2 changes: 1 addition & 1 deletion core/src/avm2/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ impl<'gc> Class<'gc> {
// A 'callable' class doesn't have a signature - let the
// method do any needed coercions
vec![],
Multiname::any(activation.context.gc_context),
Multiname::any(),
true,
activation.context.gc_context,
);
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/e4x.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1706,13 +1706,13 @@ pub fn string_to_multiname<'gc>(
) -> Multiname<'gc> {
if let Some(name) = name.strip_prefix(b'@') {
if name == b"*" {
return Multiname::any_attribute(activation.gc());
return Multiname::any_attribute();
}

let name = AvmString::new(activation.context.gc_context, name);
Multiname::attribute(activation.avm2().public_namespace_base_version, name)
} else if &*name == b"*" {
Multiname::any(activation.context.gc_context)
Multiname::any()
} else {
Multiname::new(activation.avm2().public_namespace_base_version, name)
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/globals/avmplus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ fn describe_internal_body<'gc>(
continue;
}
let prop_class_name = vtable
.slot_class_name(*slot_id, activation.context.gc_context)?
.slot_class_name(*slot_id)?
.to_qualified_name_or_star(activation.context.gc_context);

let access = match prop {
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/globals/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,10 @@ pub fn create_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
"<int instance initializer>",
vec![ParamConfig {
param_name: AvmString::new_utf8(activation.context.gc_context, "value"),
param_type_name: Multiname::any(activation.context.gc_context),
param_type_name: Multiname::any(),
default_value: Some(Value::Integer(0)),
}],
Multiname::any(activation.context.gc_context),
Multiname::any(),
true,
mc,
),
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/globals/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub fn instance_init<'gc>(
let ns = qname
.uri()
.map(|uri| Namespace::package(uri, api_version, &mut activation.borrow_gc()))
.unwrap_or_else(|| Namespace::any(activation.context.gc_context));
.unwrap_or_else(Namespace::any);
if ns.as_uri().is_empty() {
(Some("".into()), ns)
} else {
Expand Down
6 changes: 3 additions & 3 deletions core/src/avm2/globals/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub fn create_i_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
has_own_property,
vec![ParamConfig::optional(
"name",
Multiname::any(activation.context.gc_context),
Multiname::any(),
Value::Undefined,
)],
Multiname::new(activation.avm2().public_namespace_base_version, "Boolean"),
Expand All @@ -289,7 +289,7 @@ pub fn create_i_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
is_prototype_of,
vec![ParamConfig::optional(
"theClass",
Multiname::any(activation.context.gc_context),
Multiname::any(),
Value::Undefined,
)],
Multiname::new(activation.avm2().public_namespace_base_version, "Boolean"),
Expand All @@ -299,7 +299,7 @@ pub fn create_i_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
property_is_enumerable,
vec![ParamConfig::optional(
"name",
Multiname::any(activation.context.gc_context),
Multiname::any(),
Value::Undefined,
)],
Multiname::new(activation.avm2().public_namespace_base_version, "Boolean"),
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/globals/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ pub fn create_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
"<uint instance initializer>",
vec![ParamConfig {
param_name: AvmString::new_utf8(activation.context.gc_context, "value"),
param_type_name: Multiname::any(activation.context.gc_context),
param_type_name: Multiname::any(),
default_value: Some(Value::Integer(0)),
}],
Multiname::any(activation.context.gc_context),
Multiname::any(),
true,
mc,
),
Expand Down
8 changes: 4 additions & 4 deletions core/src/avm2/globals/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ pub fn children<'gc>(
activation,
children,
Some(xml.into()),
Some(Multiname::any(activation.gc())),
Some(Multiname::any()),
)
.into())
}
Expand Down Expand Up @@ -646,7 +646,7 @@ pub fn attributes<'gc>(
activation,
attributes,
Some(xml.into()),
Some(Multiname::any_attribute(activation.gc())),
Some(Multiname::any_attribute()),
)
.into())
}
Expand Down Expand Up @@ -733,7 +733,7 @@ pub fn append_child<'gc>(
let child = crate::avm2::e4x::maybe_escape_child(activation, child)?;

// 1. Let children be the result of calling the [[Get]] method of x with argument "*"
let name = Multiname::any(activation.gc());
let name = Multiname::any();
let children = xml.get_property_local(&name, activation)?;

// 2. Call the [[Put]] method of children with arguments children.[[Length]] and child
Expand Down Expand Up @@ -1102,7 +1102,7 @@ pub fn set_children<'gc>(
let value = args.get_value(0);

// 1. Call the [[Put]] method of x with arguments "*" and value
xml.set_property_local(&Multiname::any(activation.gc()), value, activation)?;
xml.set_property_local(&Multiname::any(), value, activation)?;

// 2. Return x
Ok(xml.into())
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/globals/xml_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub fn children<'gc>(
activation,
sub_children,
Some(list.into()),
Some(Multiname::any(activation.gc())),
Some(Multiname::any()),
)
.into())
}
Expand Down Expand Up @@ -320,7 +320,7 @@ pub fn attributes<'gc>(
activation,
child_attrs,
Some(list.into()),
Some(Multiname::any_attribute(activation.gc())),
Some(Multiname::any_attribute()),
)
.into())
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'gc> BytecodeMethod<'gc> {
) -> Result<Self, Error<'gc>> {
let abc = txunit.abc();
let mut signature = Vec::new();
let mut return_type = Multiname::any(activation.gc());
let mut return_type = Multiname::any();

if abc.methods.get(abc_method.0 as usize).is_some() {
let method = &abc.methods[abc_method.0 as usize];
Expand Down Expand Up @@ -429,7 +429,7 @@ impl<'gc> Method<'gc> {
signature: Vec::new(),
resolved_signature: GcCell::new(mc, None),
// FIXME - take in the real return type. This is needed for 'describeType'
return_type: Multiname::any(mc),
return_type: Multiname::any(),
is_variadic: true,
},
))
Expand Down
8 changes: 4 additions & 4 deletions core/src/avm2/multiname.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,19 +328,19 @@ impl<'gc> Multiname<'gc> {
}

/// Indicates the any type (any name in any namespace).
pub fn any(mc: &Mutation<'gc>) -> Self {
pub fn any() -> Self {
Self {
ns: NamespaceSet::single(Namespace::any(mc)),
ns: NamespaceSet::single(Namespace::any()),
name: None,
param: None,
flags: Default::default(),
}
}

/// Indicates the any attribute type (any attribute in any namespace).
pub fn any_attribute(mc: &Mutation<'gc>) -> Self {
pub fn any_attribute() -> Self {
Self {
ns: NamespaceSet::single(Namespace::any(mc)),
ns: NamespaceSet::single(Namespace::any()),
name: None,
param: None,
flags: MultinameFlags::ATTRIBUTE,
Expand Down
69 changes: 35 additions & 34 deletions core/src/avm2/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::avm2::Error;
use crate::context::UpdateContext;
use crate::string::{AvmAtom, AvmString};
use crate::{avm2::script::TranslationUnit, context::GcContext};
use gc_arena::{Collect, Gc, Mutation};
use gc_arena::{Collect, Gc};
use num_traits::FromPrimitive;
use ruffle_wstr::WStr;
use std::fmt::Debug;
Expand All @@ -12,7 +12,10 @@ use super::api_version::ApiVersion;

#[derive(Clone, Copy, Collect, Debug, PartialEq)]
#[collect(no_drop)]
pub struct Namespace<'gc>(Gc<'gc, NamespaceData<'gc>>);
pub struct Namespace<'gc>(
// `None` represents the wildcard namespace `Namespace::any()`.
Option<Gc<'gc, NamespaceData<'gc>>>,
);

/// Represents the name of a namespace.
#[allow(clippy::enum_variant_names)]
Expand All @@ -29,7 +32,6 @@ enum NamespaceData<'gc> {
// note: private namespaces are always compared by pointer identity
// of the enclosing `Gc`.
Private(AvmAtom<'gc>),
Any,
}

fn strip_version_mark(url: &WStr, is_playerglobals: bool) -> Option<(&WStr, ApiVersion)> {
Expand Down Expand Up @@ -85,7 +87,7 @@ impl<'gc> Namespace<'gc> {
context: &mut UpdateContext<'gc>,
) -> Result<Self, Error<'gc>> {
if namespace_index.0 == 0 {
return Ok(Self::any(context.gc_context));
return Ok(Self::any());
}

let actual_index = namespace_index.0 as usize - 1;
Expand All @@ -111,10 +113,10 @@ impl<'gc> Namespace<'gc> {

// Private namespaces don't get any of the namespace version checks
if let AbcNamespace::Private(_) = abc_namespace {
return Ok(Self(Gc::new(
return Ok(Self(Some(Gc::new(
context.gc_context,
NamespaceData::Private(namespace_name),
)));
))));
}

// FIXME - AvmCore gets this from an external source. I'm not exactly sure
Expand Down Expand Up @@ -212,11 +214,11 @@ impl<'gc> Namespace<'gc> {
AbcNamespace::StaticProtected(_) => NamespaceData::StaticProtected(namespace_name),
AbcNamespace::Private(_) => unreachable!(),
};
Ok(Self(Gc::new(context.gc_context, ns)))
Ok(Self(Some(Gc::new(context.gc_context, ns))))
}

pub fn any(mc: &Mutation<'gc>) -> Self {
Self(Gc::new(mc, NamespaceData::Any))
pub fn any() -> Self {
Self(None)
}

// TODO(moulins): allow passing an AvmAtom or a non-static `&WStr` directly
Expand All @@ -228,10 +230,10 @@ impl<'gc> Namespace<'gc> {
let atom = context
.interner
.intern(context.gc_context, package_name.into());
Self(Gc::new(
Self(Some(Gc::new(
context.gc_context,
NamespaceData::Namespace(atom, api_version),
))
)))
}

// TODO(moulins): allow passing an AvmAtom or a non-static `&WStr` directly
Expand All @@ -242,42 +244,41 @@ impl<'gc> Namespace<'gc> {
let atom = context
.interner
.intern(context.gc_context, package_name.into());
Self(Gc::new(
Self(Some(Gc::new(
context.gc_context,
NamespaceData::PackageInternal(atom),
))
)))
}

pub fn is_public(&self) -> bool {
matches!(*self.0, NamespaceData::Namespace(name, _) if name.as_wstr().is_empty())
matches!(self.0.as_deref(), Some(NamespaceData::Namespace(name, _)) if name.as_wstr().is_empty())
}

pub fn is_public_ignoring_ns(&self) -> bool {
matches!(*self.0, NamespaceData::Namespace(_, _))
matches!(self.0.as_deref(), Some(NamespaceData::Namespace(_, _)))
}

pub fn is_any(&self) -> bool {
matches!(*self.0, NamespaceData::Any)
self.0.is_none()
}

pub fn is_private(&self) -> bool {
matches!(*self.0, NamespaceData::Private(_))
matches!(self.0.as_deref(), Some(NamespaceData::Private(_)))
}

pub fn is_namespace(&self) -> bool {
matches!(*self.0, NamespaceData::Namespace(_, _))
matches!(self.0.as_deref(), Some(NamespaceData::Namespace(_, _)))
}

pub fn as_uri_opt(&self) -> Option<AvmString<'gc>> {
match *self.0 {
NamespaceData::Namespace(a, _) => Some(a.into()),
NamespaceData::PackageInternal(a) => Some(a.into()),
NamespaceData::Protected(a) => Some(a.into()),
NamespaceData::Explicit(a) => Some(a.into()),
NamespaceData::StaticProtected(a) => Some(a.into()),
NamespaceData::Private(a) => Some(a.into()),
NamespaceData::Any => None,
}
self.0.map(|data| match *data {
NamespaceData::Namespace(a, _) => a.into(),
NamespaceData::PackageInternal(a) => a.into(),
NamespaceData::Protected(a) => a.into(),
NamespaceData::Explicit(a) => a.into(),
NamespaceData::StaticProtected(a) => a.into(),
NamespaceData::Private(a) => a.into(),
})
}

/// Get the string value of this namespace, ignoring its type.
Expand All @@ -294,12 +295,12 @@ impl<'gc> Namespace<'gc> {
/// Namespace does not implement `PartialEq`, so that each caller is required
/// to explicitly choose either `exact_version_match` or `matches_ns`.
pub fn exact_version_match(&self, other: Self) -> bool {
if Gc::as_ptr(self.0) == Gc::as_ptr(other.0) {
if self.0.map(Gc::as_ptr) == other.0.map(Gc::as_ptr) {
true
} else if self.is_private() || other.is_private() {
false
} else {
*self.0 == *other.0
self.0 == other.0
}
}

Expand All @@ -312,10 +313,10 @@ impl<'gc> Namespace<'gc> {
if self.exact_version_match(other) {
return true;
}
match (&*self.0, &*other.0) {
match (self.0.as_deref(), other.0.as_deref()) {
(
NamespaceData::Namespace(name1, version1),
NamespaceData::Namespace(name2, version2),
Some(NamespaceData::Namespace(name1, version1)),
Some(NamespaceData::Namespace(name2, version2)),
) => {
let name_matches = name1 == name2;
let version_matches = version1 <= version2;
Expand All @@ -326,8 +327,8 @@ impl<'gc> Namespace<'gc> {
}
}
pub fn matches_api_version(&self, match_version: ApiVersion) -> bool {
match &*self.0 {
NamespaceData::Namespace(_, namespace_version) => namespace_version <= &match_version,
match self.0.as_deref() {
Some(NamespaceData::Namespace(_, version)) => version <= &match_version,
_ => true,
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/object/function_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn function_allocator<'gc>(
name: "<Empty Function>",
signature: vec![],
resolved_signature: GcCell::new(activation.context.gc_context, None),
return_type: Multiname::any(activation.context.gc_context),
return_type: Multiname::any(),
is_variadic: true,
},
);
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/object/qname_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn q_name_allocator<'gc>(
activation.context.gc_context,
QNameObjectData {
base,
name: RefLock::new(Multiname::any(activation.context.gc_context)),
name: RefLock::new(Multiname::any()),
},
))
.into())
Expand Down
6 changes: 1 addition & 5 deletions core/src/avm2/object/xml_list_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,11 +991,7 @@ impl<'gc> TObject<'gc> for XmlListObject<'gc> {
// 2.h.i. Call the [[Put]] method of x[i] with arguments "*" and V
self.xml_object_child(index, activation)
.unwrap()
.set_property_local(
&Multiname::any(activation.gc()),
value,
activation,
)?;
.set_property_local(&Multiname::any(), value, activation)?;
}

// NOTE: Not specified in the spec, but avmplus returns here, so we do the same.
Expand Down
Loading

0 comments on commit 74fc52b

Please sign in to comment.