diff --git a/core/src/avm1/value.rs b/core/src/avm1/value.rs index 0a4cd5206..12e828db7 100644 --- a/core/src/avm1/value.rs +++ b/core/src/avm1/value.rs @@ -29,12 +29,11 @@ pub enum Value<'gc> { } // This type is used very frequently, so make sure it doesn't unexpectedly grow. -// TODO(moulins): shrink `Value` down again. -// #[cfg(target_pointer_width = "32")] -// const _: () = assert!(size_of::>() == 16); +#[cfg(target_pointer_width = "32")] +const _: () = assert!(size_of::>() == 16); -// #[cfg(target_pointer_width = "64")] -// const _: () = assert!(size_of::>() == 24); +#[cfg(target_pointer_width = "64")] +const _: () = assert!(size_of::>() == 24); impl<'gc> From> for Value<'gc> { fn from(string: AvmString<'gc>) -> Self { diff --git a/core/src/string.rs b/core/src/string.rs index 8ce233b15..1a9d49ae5 100644 --- a/core/src/string.rs +++ b/core/src/string.rs @@ -1,10 +1,10 @@ -use std::borrow::{Borrow, Cow}; -use std::ops::Deref; - -use gc_arena::Collect; +use std::borrow::Cow; mod avm_string; mod interner; +mod repr; + +use repr::AvmStringRepr; pub use ruffle_wstr::*; @@ -24,31 +24,3 @@ impl SwfStrExt for swf::SwfStr { } } } - -/// This type only exists because `WString` doesn't implement `Collect` -#[derive(Collect, Eq, PartialEq, Hash)] -#[collect(require_static)] -struct OwnedWStr(WString); - -impl Deref for OwnedWStr { - type Target = WStr; - - #[inline(always)] - fn deref(&self) -> &WStr { - &self.0 - } -} - -impl Borrow for OwnedWStr { - #[inline(always)] - fn borrow(&self) -> &WStr { - &self.0 - } -} - -impl Default for OwnedWStr { - #[inline(always)] - fn default() -> Self { - OwnedWStr(WString::new()) - } -} diff --git a/core/src/string/avm_string.rs b/core/src/string/avm_string.rs index beaace156..0103035f4 100644 --- a/core/src/string/avm_string.rs +++ b/core/src/string/avm_string.rs @@ -4,13 +4,12 @@ use std::ops::Deref; use gc_arena::{Collect, Gc, MutationContext}; use ruffle_wstr::{wstr_impl_traits, WStr, WString}; -use crate::string::{AvmAtom, OwnedWStr}; +use crate::string::{AvmAtom, AvmStringRepr}; #[derive(Clone, Copy, Collect)] #[collect(no_drop)] enum Source<'gc> { - Owned(Gc<'gc, OwnedWStr>), - Interned(AvmAtom<'gc>), + Owned(Gc<'gc, AvmStringRepr>), Static(&'static WStr), } @@ -21,11 +20,13 @@ pub struct AvmString<'gc> { } impl<'gc> AvmString<'gc> { - pub(super) fn to_owned(self, gc_context: MutationContext<'gc, '_>) -> Gc<'gc, OwnedWStr> { + pub(super) fn to_owned(self, gc_context: MutationContext<'gc, '_>) -> Gc<'gc, AvmStringRepr> { match self.source { Source::Owned(s) => s, - Source::Interned(s) => s.as_owned(), - Source::Static(s) => Gc::allocate(gc_context, OwnedWStr(s.into())), + Source::Static(s) => { + let repr = AvmStringRepr::from_raw(s.into(), false); + Gc::allocate(gc_context, repr) + } } } @@ -37,8 +38,9 @@ impl<'gc> AvmString<'gc> { Cow::Owned(utf8) => WString::from_utf8_owned(utf8), Cow::Borrowed(utf8) => WString::from_utf8(utf8), }; + let repr = AvmStringRepr::from_raw(buf, false); Self { - source: Source::Owned(Gc::allocate(gc_context, OwnedWStr(buf))), + source: Source::Owned(Gc::allocate(gc_context, repr)), } } @@ -48,24 +50,23 @@ impl<'gc> AvmString<'gc> { } pub fn new>(gc_context: MutationContext<'gc, '_>, string: S) -> Self { + let repr = AvmStringRepr::from_raw(string.into(), false); Self { - source: Source::Owned(Gc::allocate(gc_context, OwnedWStr(string.into()))), + source: Source::Owned(Gc::allocate(gc_context, repr)), } } pub fn as_wstr(&self) -> &WStr { match &self.source { - Source::Owned(s) => &s.0, - Source::Interned(s) => s.as_wstr(), + Source::Owned(s) => s, Source::Static(s) => s, } } pub fn as_interned(&self) -> Option> { - if let Source::Interned(s) = self.source { - Some(s) - } else { - None + match self.source { + Source::Owned(s) if s.is_interned() => Some(AvmAtom(s)), + _ => None, } } @@ -95,7 +96,7 @@ impl<'gc> From> for AvmString<'gc> { #[inline] fn from(atom: AvmAtom<'gc>) -> Self { Self { - source: Source::Interned(atom), + source: Source::Owned(atom.0), } } } @@ -135,4 +136,41 @@ impl<'gc> Deref for AvmString<'gc> { } } -wstr_impl_traits!(impl['gc] for AvmString<'gc>); +// Manual equality implementation with fast paths for owned strings. +impl<'gc> PartialEq for AvmString<'gc> { + fn eq(&self, other: &Self) -> bool { + if let (Source::Owned(left), Source::Owned(right)) = (self.source, other.source) { + // Fast accept for identical strings. + if Gc::ptr_eq(left, right) { + return true; + // Fast reject for distinct interned strings. + } else if left.is_interned() && right.is_interned() { + return false; + } + } + + // Fallback case. + self.as_wstr() == other.as_wstr() + } +} + +impl<'gc> PartialEq> for AvmAtom<'gc> { + fn eq(&self, other: &AvmString<'gc>) -> bool { + if let Some(atom) = other.as_interned() { + *self == atom + } else { + self.as_wstr() == other.as_wstr() + } + } +} + +impl<'gc> PartialEq> for AvmString<'gc> { + #[inline(always)] + fn eq(&self, other: &AvmAtom<'gc>) -> bool { + PartialEq::eq(other, self) + } +} + +impl<'gc> Eq for AvmString<'gc> {} + +wstr_impl_traits!(impl['gc] manual_eq for AvmString<'gc>); diff --git a/core/src/string/interner.rs b/core/src/string/interner.rs index c65360d33..8af923900 100644 --- a/core/src/string/interner.rs +++ b/core/src/string/interner.rs @@ -8,12 +8,12 @@ use std::ops::DerefMut; use gc_arena::{Collect, CollectionContext, Gc, GcWeak, MutationContext}; use hashbrown::raw::{Bucket, RawTable}; -use crate::string::{AvmString, OwnedWStr, WStr}; +use crate::string::{AvmString, AvmStringRepr, WStr}; // An interned `AvmString`, with fast by-pointer equality and hashing. #[derive(Copy, Clone, Collect)] #[collect(no_drop)] -pub struct AvmAtom<'gc>(Gc<'gc, OwnedWStr>); +pub struct AvmAtom<'gc>(pub(super) Gc<'gc, AvmStringRepr>); impl<'gc> PartialEq for AvmAtom<'gc> { fn eq(&self, other: &Self) -> bool { @@ -21,23 +21,6 @@ impl<'gc> PartialEq for AvmAtom<'gc> { } } -impl<'gc> PartialEq> for AvmAtom<'gc> { - fn eq(&self, other: &AvmString<'gc>) -> bool { - if let Some(atom) = other.as_interned() { - *self == atom - } else { - self.as_wstr() == other.as_wstr() - } - } -} - -impl<'gc> PartialEq> for AvmString<'gc> { - #[inline(always)] - fn eq(&self, other: &AvmAtom<'gc>) -> bool { - PartialEq::eq(other, self) - } -} - impl<'gc> Eq for AvmAtom<'gc> {} impl<'gc> Hash for AvmAtom<'gc> { @@ -62,16 +45,12 @@ impl<'gc> AvmAtom<'gc> { pub fn as_wstr(&self) -> &WStr { &self.0 } - - pub(super) fn as_owned(self) -> Gc<'gc, OwnedWStr> { - self.0 - } } #[derive(Collect, Default)] #[collect(no_drop)] pub struct AvmStringInterner<'gc> { - interned: WeakSet<'gc, OwnedWStr>, + interned: WeakSet<'gc, AvmStringRepr>, } impl<'gc> AvmStringInterner<'gc> { @@ -79,8 +58,9 @@ impl<'gc> AvmStringInterner<'gc> { Self::default() } - fn alloc(mc: MutationContext<'gc, '_>, s: Cow<'_, WStr>) -> Gc<'gc, OwnedWStr> { - Gc::allocate(mc, OwnedWStr(s.into_owned())) + fn alloc(mc: MutationContext<'gc, '_>, s: Cow<'_, WStr>) -> Gc<'gc, AvmStringRepr> { + let repr = AvmStringRepr::from_raw(s.into_owned(), true); + Gc::allocate(mc, repr) } #[must_use] @@ -110,7 +90,11 @@ impl<'gc> AvmStringInterner<'gc> { let atom = match self.interned.entry(mc, s.as_wstr()) { (Some(atom), _) => atom, - (None, h) => self.interned.insert_fresh(mc, h, s.to_owned(mc)), + (None, h) => { + let repr = s.to_owned(mc); + repr.mark_interned(); + self.interned.insert_fresh(mc, h, repr) + } }; AvmAtom(atom) diff --git a/core/src/string/repr.rs b/core/src/string/repr.rs new file mode 100644 index 000000000..0f8214414 --- /dev/null +++ b/core/src/string/repr.rs @@ -0,0 +1,73 @@ +use std::cell::Cell; +use std::ops::Deref; + +use gc_arena::Collect; +use ruffle_wstr::{ptr as wptr, wstr_impl_traits, WStr, WString}; + +/// Internal representation of `AvmAtom`s and (owned) `AvmString`. +/// +/// Using this type directly is dangerous, as it can be used to violate +/// the interning invariants. +#[derive(Collect)] +#[collect(require_static)] +pub struct AvmStringRepr { + ptr: *mut (), + meta: wptr::WStrMetadata, + // We abuse the 'is_wide' bit for interning. + capacity: Cell, +} + +impl AvmStringRepr { + pub fn from_raw(s: WString, interned: bool) -> Self { + let (ptr, meta, cap) = s.into_raw_parts(); + let capacity = Cell::new(wptr::WStrMetadata::new32(cap, interned)); + Self { + ptr, + meta, + capacity, + } + } + + #[inline] + pub fn as_wstr(&self) -> &WStr { + // SAFETY: we own a `WString`. + unsafe { &*wptr::from_raw_parts(self.ptr, self.meta) } + } + + pub fn is_interned(&self) -> bool { + self.capacity.get().is_wide() + } + + pub fn mark_interned(&self) { + let cap = self.capacity.get(); + let new_cap = wptr::WStrMetadata::new32(cap.len32(), true); + self.capacity.set(new_cap); + } +} + +impl Drop for AvmStringRepr { + fn drop(&mut self) { + // SAFETY: we drop the `WString` we logically own. + unsafe { + let cap = self.capacity.get().len32(); + let _ = WString::from_raw_parts(self.ptr, self.meta, cap); + } + } +} + +impl Deref for AvmStringRepr { + type Target = WStr; + #[inline] + fn deref(&self) -> &WStr { + self.as_wstr() + } +} + +impl Default for AvmStringRepr { + #[inline] + fn default() -> Self { + Self::from_raw(WString::new(), false) + } +} + +wstr_impl_traits!(impl for AvmStringRepr); diff --git a/wstr/src/common.rs b/wstr/src/common.rs index 659d73ddc..358cc45e1 100644 --- a/wstr/src/common.rs +++ b/wstr/src/common.rs @@ -492,6 +492,8 @@ impl core::cmp::PartialEq for WStr { } } +impl core::cmp::Eq for WStr {} + impl core::cmp::Ord for WStr { #[inline] fn cmp(&self, other: &Self) -> core::cmp::Ordering { @@ -580,9 +582,6 @@ macro_rules! __wstr_impl_internal { }; (@eq_ord_self [$($generics:tt)*] for $ty:ty) => { - - impl<$($generics)*> ::core::cmp::Eq for $ty {} - impl<$($generics)*> ::core::cmp::PartialOrd<$ty> for $ty { #[inline] fn partial_cmp(&self, other: &$ty) -> Option<::core::cmp::Ordering> { @@ -596,6 +595,17 @@ macro_rules! __wstr_impl_internal { $crate::__wstr_impl_internal! { @eq_ord_units [$($generics)*] for $ty, [u16] } }; + (@eq_base [$($generics:tt)*] for $ty:ty) => { + impl<$($generics)*> ::core::cmp::PartialEq<$ty> for $ty { + #[inline] + fn eq(&self, other: &$ty) -> bool { + ::core::cmp::PartialEq::eq(::core::ops::Deref::deref(self), ::core::ops::Deref::deref(other)) + } + } + + impl<$($generics)*> ::core::cmp::Eq for $ty {} + }; + (@base [$($generics:tt)*] for $ty:ty) => { impl<$($generics)*> ::core::convert::AsRef<$crate::WStr> for $ty { #[inline] @@ -611,13 +621,6 @@ macro_rules! __wstr_impl_internal { } } - impl<$($generics)*> ::core::cmp::PartialEq<$ty> for $ty { - #[inline] - fn eq(&self, other: &$ty) -> bool { - ::core::cmp::PartialEq::eq(::core::ops::Deref::deref(self), ::core::ops::Deref::deref(other)) - } - } - impl<$($generics)*> ::core::cmp::Ord for $ty { #[inline] fn cmp(&self, other: &Self) -> ::core::cmp::Ordering { @@ -666,7 +669,7 @@ macro_rules! __wstr_impl_internal { } }; - (@full [$($generics:tt)*] for $ty:ty) => { + (@full_no_eq [$($generics:tt)*] for $ty:ty) => { $crate::__wstr_impl_internal!(@base [$($generics)*] for $ty); $crate::__wstr_impl_internal!(@eq_ord_self [$($generics)*] for $ty); $crate::__wstr_impl_internal!(@eq_ord [$($generics)*] for $ty, $crate::WStr); @@ -682,7 +685,8 @@ macro_rules! __wstr_impl_internal { /// delegating impls for the following traits: /// /// - [`AsRef`], [`Borrow`][core::borrow::Borrow]; -/// - [`Eq`], [`PartialEq`], [`Ord`], [`PartialOrd`]; +/// - [`Eq`], [`PartialEq`] (this can be disabled with `manual_eq`); +/// - [`Ord`], [`PartialOrd`], /// - [`PartialEq<_>`], [`PartialOrd<_>`] for [`WStr`], [`&WStr`][WStr], /// `[u8]`, `[u16]`, `[u8; N]` and `[u16; N]`; /// - [`Display`][core::fmt::Display], [`Debug`][core::fmt::Debug]; @@ -690,11 +694,20 @@ macro_rules! __wstr_impl_internal { /// - [`IntoIterator`][IntoIterator]. #[macro_export] macro_rules! wstr_impl_traits { + (impl manual_eq for $ty_name:ty) => { + $crate::__wstr_impl_internal!(@full_no_eq [] for $ty_name); + }; (impl for $ty_name:ty) => { - $crate::__wstr_impl_internal!(@full [] for $ty_name); + $crate::__wstr_impl_internal!(@full_no_eq [] for $ty_name); + $crate::__wstr_impl_internal!(@eq_base [] for $ty_name); + }; + (impl[$($generics:tt)+] manual_eq for $ty_name:ty) => { + $crate::__wstr_impl_internal!(@full_no_eq [$($generics)*,] for $ty_name); }; (impl [$($generics:tt)+] for $ty_name:ty) => { - $crate::__wstr_impl_internal!(@full [$($generics)*,] for $ty_name); + $crate::__wstr_impl_internal!(@full_no_eq [$($generics)*,] for $ty_name); + $crate::__wstr_impl_internal!(@eq_base [$($generics)*,] for $ty_name); + }; }