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 reference counted pointers from JsValue variants #1866

Closed
wants to merge 1 commit 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
12 changes: 10 additions & 2 deletions boa_engine/src/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
use crate::{builtins::Number, Context, JsValue};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use num_integer::Integer;
use num_traits::pow::Pow;
use num_traits::{FromPrimitive, One, ToPrimitive, Zero};
use num_traits::{pow::Pow, FromPrimitive, One, ToPrimitive, Zero};
use std::{
fmt::{self, Display},
ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Neg, Rem, Shl, Shr, Sub},
Expand Down Expand Up @@ -290,6 +289,15 @@ impl From<RawBigInt> for JsBigInt {
}
}

impl From<Box<RawBigInt>> for JsBigInt {
#[inline]
fn from(value: Box<RawBigInt>) -> Self {
Self {
inner: value.into(),
}
}
}

impl From<i8> for JsBigInt {
#[inline]
fn from(value: i8) -> Self {
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/bytecompiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ impl<'b> ByteCompiler<'b> {
)),
Const::Int(v) => self.emit_push_integer(*v),
Const::Num(v) => self.emit_push_rational(*v),
Const::BigInt(v) => self.emit_push_literal(Literal::BigInt(v.clone())),
Const::BigInt(v) => self.emit_push_literal(Literal::BigInt(v.clone().into())),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place where I see that it will actually be less performant.

Const::Bool(true) => self.emit(Opcode::PushTrue, &[]),
Const::Bool(false) => self.emit(Opcode::PushFalse, &[]),
Const::Null => self.emit(Opcode::PushNull, &[]),
Expand Down
16 changes: 10 additions & 6 deletions boa_engine/src/object/jsobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@ pub type RefMut<'a, T, U> = boa_gc::RefMut<'a, T, U>;

/// Garbage collected `Object`.
#[derive(Trace, Finalize, Clone, Default)]
pub struct JsObject(Gc<boa_gc::Cell<Object>>);
pub struct JsObject {
inner: Gc<boa_gc::Cell<Object>>,
}

impl JsObject {
/// Create a new `JsObject` from an internal `Object`.
#[inline]
fn from_object(object: Object) -> Self {
Self(Gc::new(boa_gc::Cell::new(object)))
Self {
inner: Gc::new(boa_gc::Cell::new(object)),
}
}

/// Create a new empty `JsObject`, with `prototype` set to `JsValue::Null`
Expand Down Expand Up @@ -90,7 +94,7 @@ impl JsObject {
/// This is the non-panicking variant of [`borrow`](#method.borrow).
#[inline]
pub fn try_borrow(&self) -> StdResult<Ref<'_, Object>, BorrowError> {
self.0.try_borrow().map_err(|_| BorrowError)
self.inner.try_borrow().map_err(|_| BorrowError)
}

/// Mutably borrows the object, returning an error if the value is currently borrowed.
Expand All @@ -101,7 +105,7 @@ impl JsObject {
/// This is the non-panicking variant of [`borrow_mut`](#method.borrow_mut).
#[inline]
pub fn try_borrow_mut(&self) -> StdResult<RefMut<'_, Object, Object>, BorrowMutError> {
self.0.try_borrow_mut().map_err(|_| BorrowMutError)
self.inner.try_borrow_mut().map_err(|_| BorrowMutError)
}

/// Checks if the garbage collected memory is the same.
Expand Down Expand Up @@ -669,7 +673,7 @@ Cannot both specify accessors and a value or writable attribute",
impl AsRef<boa_gc::Cell<Object>> for JsObject {
#[inline]
fn as_ref(&self) -> &boa_gc::Cell<Object> {
&*self.0
&*self.inner
}
}

Expand Down Expand Up @@ -798,7 +802,7 @@ impl Debug for JsObject {
// Instead, we check if the object has appeared before in the entire graph. This means that objects will appear
// at most once, hopefully making things a bit clearer.
if !limiter.visited && !limiter.live {
f.debug_tuple("JsObject").field(&self.0).finish()
f.debug_tuple("JsObject").field(&self.inner).finish()
} else {
f.write_str("{ ... }")
}
Expand Down
3 changes: 2 additions & 1 deletion boa_engine/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{
marker::PhantomData,
ops::Deref,
ptr::{copy_nonoverlapping, NonNull},
rc::Rc,
};

const CONSTANTS_ARRAY: [&str; 127] = [
Expand Down Expand Up @@ -314,7 +315,7 @@ impl Inner {
#[derive(Finalize)]
pub struct JsString {
inner: NonNull<Inner>,
_marker: PhantomData<std::rc::Rc<str>>,
_marker: PhantomData<Rc<str>>,
}

impl Default for JsString {
Expand Down
25 changes: 18 additions & 7 deletions boa_engine/src/syntax/ast/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@
//! [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 crate::JsBigInt;
use boa_gc::{Finalize, Trace};
use boa_gc::{unsafe_empty_trace, Finalize, Trace};
use boa_interner::{Interner, Sym, ToInternedString};

use num_bigint::BigInt;
#[cfg(feature = "deser")]
use serde::{Deserialize, Serialize};

Expand All @@ -25,7 +24,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, Trace, Finalize, PartialEq)]
#[derive(Clone, Debug, Finalize, PartialEq)]
pub enum Const {
/// A string literal is zero or more characters enclosed in double (`"`) or single (`'`) quotation marks.
///
Expand Down Expand Up @@ -74,7 +73,7 @@ pub enum Const {
///
/// [spec]: https://tc39.es/ecma262/#sec-terms-and-definitions-bigint-value
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Numeric_literals
BigInt(JsBigInt),
BigInt(Box<BigInt>),

/// The Boolean type has two literal values: `true` and `false`.
///
Expand Down Expand Up @@ -113,6 +112,12 @@ 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 All @@ -131,8 +136,14 @@ impl From<i32> for Const {
}
}

impl From<JsBigInt> for Const {
fn from(i: JsBigInt) -> Self {
impl From<BigInt> for Const {
fn from(i: BigInt) -> Self {
Self::BigInt(Box::new(i))
}
}

impl From<Box<BigInt>> for Const {
fn from(i: Box<BigInt>) -> Self {
Self::BigInt(i)
}
}
Expand Down
6 changes: 4 additions & 2 deletions boa_engine/src/syntax/lexer/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use crate::{
};
use boa_interner::Interner;
use boa_profiler::Profiler;
use num_bigint::BigInt;
use num_traits::Zero;
use std::{io::Read, str};

/// Number literal lexing.
Expand Down Expand Up @@ -251,7 +253,7 @@ impl<R> Tokenizer<R> for NumberLiteral {

// DecimalBigIntegerLiteral '0n'
return Ok(Token::new(
TokenKind::NumericLiteral(Numeric::BigInt(0.into())),
TokenKind::NumericLiteral(Numeric::BigInt(BigInt::zero().into())),
Span::new(start_pos, cursor.pos()),
));
}
Expand Down Expand Up @@ -380,7 +382,7 @@ impl<R> Tokenizer<R> for NumberLiteral {
let num = match kind {
NumericKind::BigInt(base) => {
Numeric::BigInt(
JsBigInt::from_string_radix(num_str, base).expect("Could not convert to BigInt")
BigInt::parse_bytes(num_str.as_bytes(), base).expect("Could not convert to BigInt").into()
)
}
NumericKind::Rational /* base: 10 */ => {
Expand Down
17 changes: 8 additions & 9 deletions boa_engine/src/syntax/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@
//!
//! [spec]: https://tc39.es/ecma262/#sec-tokens

use crate::{
syntax::ast::{Keyword, Punctuator, Span},
syntax::lexer::template::TemplateString,
JsBigInt,
use crate::syntax::{
ast::{Keyword, Punctuator, Span},
lexer::template::TemplateString,
};
use boa_interner::{Interner, Sym};

use num_bigint::BigInt;
#[cfg(feature = "deser")]
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -71,7 +70,7 @@ pub enum Numeric {
Integer(i32),

// A BigInt
BigInt(JsBigInt),
BigInt(Box<BigInt>),
}

impl From<f64> for Numeric {
Expand All @@ -88,10 +87,10 @@ impl From<i32> for Numeric {
}
}

impl From<JsBigInt> for Numeric {
impl From<BigInt> for Numeric {
#[inline]
fn from(n: JsBigInt) -> Self {
Self::BigInt(n)
fn from(n: BigInt) -> Self {
Self::BigInt(Box::new(n))
}
}

Expand Down