Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Removed some unsafe_empty_trace!() calls to improve performance #2233

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions boa_engine/src/bigint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! This module implements the JavaScript bigint primitive rust type.

use crate::{builtins::Number, Context, JsValue};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use num_integer::Integer;
use num_traits::{pow::Pow, FromPrimitive, One, ToPrimitive, Zero};
use std::{
Expand All @@ -18,17 +17,11 @@ use serde::{Deserialize, Serialize};

/// JavaScript bigint primitive rust type.
#[cfg_attr(feature = "deser", derive(Serialize, Deserialize))]
#[derive(Debug, Finalize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct JsBigInt {
inner: Rc<RawBigInt>,
}

// Safety: BigInt does not contain any objects which needs to be traced,
// so this is safe.
unsafe impl Trace for JsBigInt {
unsafe_empty_trace!();
}

impl JsBigInt {
/// Create a new [`JsBigInt`].
#[inline]
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/builtins/array/array_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use boa_profiler::Profiler;
pub struct ArrayIterator {
array: JsObject,
next_index: u64,
#[unsafe_ignore_trace]
kind: PropertyNameKind,
done: bool,
}
Expand Down
9 changes: 1 addition & 8 deletions boa_engine/src/builtins/date/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use crate::{
value::{JsValue, PreferredType},
Context, JsResult, JsString,
};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_profiler::Profiler;
use chrono::{prelude::*, Duration, LocalResult};
use std::fmt::Display;
Expand Down Expand Up @@ -61,7 +60,7 @@ macro_rules! getter_method {
}};
}

#[derive(Debug, Finalize, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Date(Option<NaiveDateTime>);

impl Display for Date {
Expand All @@ -73,12 +72,6 @@ impl Display for Date {
}
}

unsafe impl Trace for Date {
// Date is a stack value, it doesn't require tracing.
// only safe if `chrono` never implements `Trace` for `NaiveDateTime`
unsafe_empty_trace!();
}

impl Default for Date {
fn default() -> Self {
Self(Some(Utc::now().naive_utc()))
Expand Down
3 changes: 1 addition & 2 deletions boa_engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ pub enum ClassFieldDefinition {
unsafe impl Trace for ClassFieldDefinition {
custom_trace! {this, {
match this {
Self::Public(key, func) => {
mark(key);
Self::Public(_key, func) => {
mark(func);
}
Self::Private(_, func) => {
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/builtins/map/map_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use boa_profiler::Profiler;
pub struct MapIterator {
iterated_map: Option<JsObject>,
map_next_index: usize,
#[unsafe_ignore_trace]
map_iteration_kind: PropertyNameKind,
lock: MapLock,
}
Expand Down
8 changes: 1 addition & 7 deletions boa_engine/src/builtins/regexp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use crate::{
value::{IntegerOrInfinity, JsValue},
Context, JsResult, JsString,
};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_profiler::Profiler;
use regress::Regex;
use std::str::FromStr;
Expand All @@ -36,7 +35,7 @@ use tap::{Conv, Pipe};
mod tests;

/// The internal representation on a `RegExp` object.
#[derive(Debug, Clone, Finalize)]
#[derive(Debug, Clone)]
pub struct RegExp {
/// Regex matcher.
matcher: Regex,
Expand All @@ -45,11 +44,6 @@ pub struct RegExp {
original_flags: JsString,
}

// Only safe while regress::Regex doesn't implement Trace itself.
unsafe impl Trace for RegExp {
unsafe_empty_trace!();
}

impl BuiltIn for RegExp {
const NAME: &'static str = "RegExp";

Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/builtins/set/set_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use boa_profiler::Profiler;
pub struct SetIterator {
iterated_set: JsValue,
next_index: usize,
#[unsafe_ignore_trace]
iteration_kind: PropertyNameKind,
}

Expand Down
10 changes: 3 additions & 7 deletions boa_engine/src/builtins/typed_array/integer_indexed_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,20 @@ use crate::{
object::{JsObject, ObjectData},
Context,
};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_gc::{Finalize, Trace};

/// Type of the array content.
#[derive(Debug, Clone, Copy, Finalize, PartialEq)]
#[derive(Debug, Clone, Copy, PartialEq)]
pub(crate) enum ContentType {
Number,
BigInt,
}

unsafe impl Trace for ContentType {
// safe because `ContentType` is `Copy`
unsafe_empty_trace!();
}

/// <https://tc39.es/ecma262/#integer-indexed-exotic-object>
#[derive(Debug, Clone, Trace, Finalize)]
pub struct IntegerIndexed {
viewed_array_buffer: Option<JsObject>,
#[unsafe_ignore_trace]
typed_array_name: TypedArrayKind,
byte_offset: u64,
byte_length: u64,
Expand Down
8 changes: 1 addition & 7 deletions boa_engine/src/builtins/typed_array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use crate::{
value::{IntegerOrInfinity, JsValue},
Context, JsResult, JsString,
};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_profiler::Profiler;
use num_traits::{Signed, Zero};
use std::cmp::Ordering;
Expand Down Expand Up @@ -3388,7 +3387,7 @@ impl TypedArray {
}

/// Names of all the typed arrays.
#[derive(Debug, Clone, Copy, Finalize, PartialEq)]
#[derive(Debug, Clone, Copy, PartialEq)]
pub(crate) enum TypedArrayKind {
Int8,
Uint8,
Expand All @@ -3403,11 +3402,6 @@ pub(crate) enum TypedArrayKind {
Float64,
}

unsafe impl Trace for TypedArrayKind {
// Safe because `TypedArrayName` is `Copy`
unsafe_empty_trace!();
}

impl TypedArrayKind {
/// Gets the element size of the given typed array name, as per the [spec].
///
Expand Down
43 changes: 41 additions & 2 deletions boa_engine/src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use crate::{
Context, JsBigInt, JsResult, JsString, JsSymbol, JsValue,
};

use boa_gc::{Finalize, Trace};
use boa_gc::{custom_trace, Finalize, Trace};
use boa_interner::Sym;
use rustc_hash::FxHashMap;
use std::{
Expand Down Expand Up @@ -163,7 +163,7 @@ pub struct ObjectData {
}

/// Defines the different types of objects.
#[derive(Debug, Trace, Finalize)]
#[derive(Debug, Finalize)]
pub enum ObjectKind {
AsyncGenerator(AsyncGenerator),
AsyncGeneratorFunction(Function),
Expand Down Expand Up @@ -201,6 +201,45 @@ pub enum ObjectKind {
Promise(Promise),
}

unsafe impl Trace for ObjectKind {
custom_trace! {this, {
match this {
Self::ArrayIterator(i) => mark(i),
Self::ArrayBuffer(b) => mark(b),
Self::Map(m) => mark(m),
Self::MapIterator(i) => mark(i),
Self::RegExpStringIterator(i) => mark(i),
Self::DataView(v) => mark(v),
Self::ForInIterator(i) => mark(i),
Self::Function(f) | Self::GeneratorFunction(f) | Self::AsyncGeneratorFunction(f) => mark(f),
Self::BoundFunction(f) => mark(f),
Self::Generator(g) => mark(g),
Self::Set(s) => mark(s),
Self::SetIterator(i) => mark(i),
Self::StringIterator(i) => mark(i),
Self::Proxy(p) => mark(p),
Self::Arguments(a) => mark(a),
Self::NativeObject(o) => mark(o),
Self::IntegerIndexed(i) => mark(i),
#[cfg(feature = "intl")]
Self::DateTimeFormat(f) => mark(f),
Self::Promise(p) => mark(p),
Self::AsyncGenerator(g) => mark(g),
Self::RegExp(_)
| Self::BigInt(_)
| Self::Boolean(_)
| Self::String(_)
| Self::Date(_)
| Self::Array
| Self::Error
| Self::Ordinary
| Self::Global
| Self::Number(_)
| Self::Symbol(_) => {}
}
}}
}

impl ObjectData {
/// Create the `AsyncGenerator` object data
pub fn async_generator(async_generator: AsyncGenerator) -> Self {
Expand Down
11 changes: 0 additions & 11 deletions boa_engine/src/property/attribute/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! This module implements the `Attribute` struct which contains the attibutes for property descriptors.

use bitflags::bitflags;
use boa_gc::{unsafe_empty_trace, Finalize, Trace};

#[cfg(test)]
mod tests;
Expand All @@ -16,7 +15,6 @@ bitflags! {
/// - `[[Configurable]]` (`CONFIGURABLE`) - If `false`, attempts to delete the property,
/// change the property to be an `accessor property`, or change its attributes (other than `[[Value]]`,
/// or changing `[[Writable]]` to `false`) will fail.
#[derive(Finalize)]
pub struct Attribute: u8 {
/// The `Writable` attribute decides whether the value associated with the property can be changed or not, from its initial value.
const WRITABLE = 0b0000_0001;
Expand All @@ -38,15 +36,6 @@ bitflags! {
}
}

// We implement `Trace` manualy rather that wih derive, beacuse `rust-gc`,
// derive `Trace` does not allow `Copy` and `Trace` to be both implemented.
//
// SAFETY: The `Attribute` struct only contains an `u8`
// and therefore it should be safe to implement an empty trace.
unsafe impl Trace for Attribute {
unsafe_empty_trace!();
}

impl Attribute {
/// Clear all flags.
#[inline]
Expand Down
10 changes: 3 additions & 7 deletions boa_engine/src/property/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
//! [section]: https://tc39.es/ecma262/#sec-property-attributes

use crate::{JsString, JsSymbol, JsValue};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_gc::{Finalize, Trace};
use std::fmt;

mod attribute;
Expand Down Expand Up @@ -488,7 +488,7 @@ impl From<PropertyDescriptorBuilder> for PropertyDescriptor {
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-ispropertykey
#[derive(Trace, Finalize, PartialEq, Debug, Clone, Eq, Hash)]
#[derive(PartialEq, Debug, Clone, Eq, Hash)]
pub enum PropertyKey {
String(JsString),
Symbol(JsSymbol),
Expand Down Expand Up @@ -673,13 +673,9 @@ impl PartialEq<&str> for PropertyKey {
}
}

#[derive(Debug, Clone, Copy, Finalize)]
#[derive(Debug, Clone, Copy)]
pub(crate) enum PropertyNameKind {
Key,
Value,
KeyAndValue,
}

unsafe impl Trace for PropertyNameKind {
unsafe_empty_trace!();
}
12 changes: 6 additions & 6 deletions boa_engine/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,12 @@ pub struct JsString {
_marker: PhantomData<Rc<str>>,
}

// Safety: JsString does not contain any objects which needs to be traced,
// so this is safe.
unsafe impl Trace for JsString {
unsafe_empty_trace!();
}

/// This struct uses a technique called tagged pointer to benefit from the fact that newly
/// allocated pointers are always word aligned on 64-bits platforms, making it impossible
/// to have a LSB equal to 1. More details about this technique on the article of Wikipedia
Expand Down Expand Up @@ -942,12 +948,6 @@ impl JsString {
}
}

// Safety: [`JsString`] does not contain any objects which recquire trace,
// so this is safe.
unsafe impl Trace for JsString {
unsafe_empty_trace!();
}

impl Clone for JsString {
#[inline]
fn clone(&self) -> Self {
Expand Down
16 changes: 7 additions & 9 deletions boa_engine/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,17 @@ struct Inner {
}

/// This represents a JavaScript symbol primitive.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Finalize)]
pub struct JsSymbol {
inner: Rc<Inner>,
}

// Safety: JsSymbol does not contain any objects which needs to be traced,
// so this is safe.
unsafe impl Trace for JsSymbol {
unsafe_empty_trace!();
}

impl JsSymbol {
/// Create a new symbol.
#[inline]
Expand Down Expand Up @@ -301,14 +307,6 @@ impl JsSymbol {
}
}

impl Finalize for JsSymbol {}

// Safety: `JsSymbol` does not contain any object that require trace,
// so this is safe.
unsafe impl Trace for JsSymbol {
unsafe_empty_trace!();
}

impl Display for JsSymbol {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down
9 changes: 1 addition & 8 deletions boa_engine/src/syntax/ast/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
//! [spec]: https://tc39.es/ecma262/#sec-primary-expression-literals
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Literals

use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_interner::{Interner, Sym, ToInternedString};
use num_bigint::BigInt;
#[cfg(feature = "deser")]
Expand All @@ -24,7 +23,7 @@ use serde::{Deserialize, Serialize};
/// [spec]: https://tc39.es/ecma262/#sec-primary-expression-literals
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Literals
#[cfg_attr(feature = "deser", derive(Serialize, Deserialize))]
#[derive(Clone, Debug, Finalize, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub enum Const {
/// A string literal is zero or more characters enclosed in double (`"`) or single (`'`) quotation marks.
///
Expand Down Expand Up @@ -112,12 +111,6 @@ pub enum Const {
Undefined,
}

// Safety: Const does not contain any objects which needs to be traced,
// so this is safe.
unsafe impl Trace for Const {
unsafe_empty_trace!();
}

impl From<Sym> for Const {
fn from(string: Sym) -> Self {
Self::String(string)
Expand Down
Loading