Browse Source

Replace `FxHashMap` with `IndexMap` in object properties (#1547)

* Replace `FxHashMap` with `IndexMap` in object properties

* Add test for property order

* Remove unneeded sort from `JSON.stringify`
pull/1551/head
raskad 3 years ago committed by GitHub
parent
commit
6115182ab9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 9
      boa/src/builtins/json/mod.rs
  2. 82
      boa/src/object/property_map.rs
  3. 29
      boa/src/object/tests.rs

9
boa/src/builtins/json/mod.rs

@ -13,8 +13,6 @@
//! [json]: https://www.json.org/json-en.html
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON
use std::collections::HashSet;
use crate::{
builtins::{
string::{is_leading_surrogate, is_trailing_surrogate},
@ -172,7 +170,7 @@ impl Json {
// ii. If isArray is true, then
if replacer_obj.is_array() {
// 1. Set PropertyList to a new empty List.
let mut property_set = HashSet::new();
let mut property_set = indexmap::IndexSet::new();
// 2. Let len be ? LengthOfArrayLike(replacer).
let len = replacer_obj.length_of_array_like(context)?;
@ -477,7 +475,7 @@ impl Json {
state.indent = JsString::concat(&state.indent, &state.gap);
// 5. If state.[[PropertyList]] is not undefined, then
let mut k = if let Some(p) = &state.property_list {
let k = if let Some(p) = &state.property_list {
// a. Let K be state.[[PropertyList]].
p.clone()
// 6. Else,
@ -488,9 +486,6 @@ impl Json {
keys.iter().map(|v| v.to_string(context).unwrap()).collect()
};
// Sort the property key list, because the internal property hashmap of objects does not sort the properties.
k.sort();
// 7. Let partial be a new empty List.
let mut partial = Vec::new();

82
boa/src/object/property_map.rs

@ -1,18 +1,38 @@
use super::{PropertyDescriptor, PropertyKey};
use crate::{
gc::{Finalize, Trace},
gc::{custom_trace, Finalize, Trace},
JsString, JsSymbol,
};
use rustc_hash::FxHashMap;
use std::{collections::hash_map, iter::FusedIterator};
use indexmap::IndexMap;
use rustc_hash::{FxHashMap, FxHasher};
use std::{collections::hash_map, hash::BuildHasherDefault, iter::FusedIterator};
/// Wrapper around indexmap::IndexMap for usage in PropertyMap
#[derive(Debug, Finalize)]
struct OrderedHashMap<K: Trace>(IndexMap<K, PropertyDescriptor, BuildHasherDefault<FxHasher>>);
impl<K: Trace> Default for OrderedHashMap<K> {
fn default() -> Self {
Self(IndexMap::with_hasher(BuildHasherDefault::default()))
}
}
unsafe impl<K: Trace> Trace for OrderedHashMap<K> {
custom_trace!(this, {
for (k, v) in this.0.iter() {
mark(k);
mark(v);
}
});
}
#[derive(Default, Debug, Trace, Finalize)]
pub struct PropertyMap {
indexed_properties: FxHashMap<u32, PropertyDescriptor>,
/// Properties
string_properties: FxHashMap<JsString, PropertyDescriptor>,
string_properties: OrderedHashMap<JsString>,
/// Symbol Properties
symbol_properties: FxHashMap<JsSymbol, PropertyDescriptor>,
symbol_properties: OrderedHashMap<JsSymbol>,
}
impl PropertyMap {
@ -22,8 +42,8 @@ impl PropertyMap {
pub fn get(&self, key: &PropertyKey) -> Option<&PropertyDescriptor> {
match key {
PropertyKey::Index(index) => self.indexed_properties.get(index),
PropertyKey::String(string) => self.string_properties.get(string),
PropertyKey::Symbol(symbol) => self.symbol_properties.get(symbol),
PropertyKey::String(string) => self.string_properties.0.get(string),
PropertyKey::Symbol(symbol) => self.symbol_properties.0.get(symbol),
}
}
@ -34,16 +54,20 @@ impl PropertyMap {
) -> Option<PropertyDescriptor> {
match &key {
PropertyKey::Index(index) => self.indexed_properties.insert(*index, property),
PropertyKey::String(string) => self.string_properties.insert(string.clone(), property),
PropertyKey::Symbol(symbol) => self.symbol_properties.insert(symbol.clone(), property),
PropertyKey::String(string) => {
self.string_properties.0.insert(string.clone(), property)
}
PropertyKey::Symbol(symbol) => {
self.symbol_properties.0.insert(symbol.clone(), property)
}
}
}
pub fn remove(&mut self, key: &PropertyKey) -> Option<PropertyDescriptor> {
match key {
PropertyKey::Index(index) => self.indexed_properties.remove(index),
PropertyKey::String(string) => self.string_properties.remove(string),
PropertyKey::Symbol(symbol) => self.symbol_properties.remove(symbol),
PropertyKey::String(string) => self.string_properties.0.shift_remove(string),
PropertyKey::Symbol(symbol) => self.symbol_properties.0.shift_remove(symbol),
}
}
@ -54,8 +78,8 @@ impl PropertyMap {
pub fn iter(&self) -> Iter<'_> {
Iter {
indexed_properties: self.indexed_properties.iter(),
string_properties: self.string_properties.iter(),
symbol_properties: self.symbol_properties.iter(),
string_properties: self.string_properties.0.iter(),
symbol_properties: self.symbol_properties.0.iter(),
}
}
@ -81,7 +105,7 @@ impl PropertyMap {
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn symbol_properties(&self) -> SymbolProperties<'_> {
SymbolProperties(self.symbol_properties.iter())
SymbolProperties(self.symbol_properties.0.iter())
}
/// An iterator visiting all symbol keys in arbitrary order. The iterator element type is `&'a RcSymbol`.
@ -89,7 +113,7 @@ impl PropertyMap {
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn symbol_property_keys(&self) -> SymbolPropertyKeys<'_> {
SymbolPropertyKeys(self.symbol_properties.keys())
SymbolPropertyKeys(self.symbol_properties.0.keys())
}
/// An iterator visiting all symbol values in arbitrary order. The iterator element type is `&'a Property`.
@ -97,7 +121,7 @@ impl PropertyMap {
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn symbol_property_values(&self) -> SymbolPropertyValues<'_> {
SymbolPropertyValues(self.symbol_properties.values())
SymbolPropertyValues(self.symbol_properties.0.values())
}
/// An iterator visiting all indexed key-value pairs in arbitrary order. The iterator element type is `(&'a u32, &'a Property)`.
@ -129,7 +153,7 @@ impl PropertyMap {
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn string_properties(&self) -> StringProperties<'_> {
StringProperties(self.string_properties.iter())
StringProperties(self.string_properties.0.iter())
}
/// An iterator visiting all string keys in arbitrary order. The iterator element type is `&'a RcString`.
@ -137,7 +161,7 @@ impl PropertyMap {
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn string_property_keys(&self) -> StringPropertyKeys<'_> {
StringPropertyKeys(self.string_properties.keys())
StringPropertyKeys(self.string_properties.0.keys())
}
/// An iterator visiting all string values in arbitrary order. The iterator element type is `&'a Property`.
@ -145,15 +169,15 @@ impl PropertyMap {
/// This iterator does not recurse down the prototype chain.
#[inline]
pub fn string_property_values(&self) -> StringPropertyValues<'_> {
StringPropertyValues(self.string_properties.values())
StringPropertyValues(self.string_properties.0.values())
}
#[inline]
pub fn contains_key(&self, key: &PropertyKey) -> bool {
match key {
PropertyKey::Index(index) => self.indexed_properties.contains_key(index),
PropertyKey::String(string) => self.string_properties.contains_key(string),
PropertyKey::Symbol(symbol) => self.symbol_properties.contains_key(symbol),
PropertyKey::String(string) => self.string_properties.0.contains_key(string),
PropertyKey::Symbol(symbol) => self.symbol_properties.0.contains_key(symbol),
}
}
}
@ -162,8 +186,8 @@ impl PropertyMap {
#[derive(Debug, Clone)]
pub struct Iter<'a> {
indexed_properties: hash_map::Iter<'a, u32, PropertyDescriptor>,
string_properties: hash_map::Iter<'a, JsString, PropertyDescriptor>,
symbol_properties: hash_map::Iter<'a, JsSymbol, PropertyDescriptor>,
string_properties: indexmap::map::Iter<'a, JsString, PropertyDescriptor>,
symbol_properties: indexmap::map::Iter<'a, JsSymbol, PropertyDescriptor>,
}
impl<'a> Iterator for Iter<'a> {
@ -233,7 +257,7 @@ impl FusedIterator for Values<'_> {}
/// An iterator over the `Symbol` property entries of an `Object`
#[derive(Debug, Clone)]
pub struct SymbolProperties<'a>(hash_map::Iter<'a, JsSymbol, PropertyDescriptor>);
pub struct SymbolProperties<'a>(indexmap::map::Iter<'a, JsSymbol, PropertyDescriptor>);
impl<'a> Iterator for SymbolProperties<'a> {
type Item = (&'a JsSymbol, &'a PropertyDescriptor);
@ -260,7 +284,7 @@ impl FusedIterator for SymbolProperties<'_> {}
/// An iterator over the keys (`RcSymbol`) of an `Object`.
#[derive(Debug, Clone)]
pub struct SymbolPropertyKeys<'a>(hash_map::Keys<'a, JsSymbol, PropertyDescriptor>);
pub struct SymbolPropertyKeys<'a>(indexmap::map::Keys<'a, JsSymbol, PropertyDescriptor>);
impl<'a> Iterator for SymbolPropertyKeys<'a> {
type Item = &'a JsSymbol;
@ -287,7 +311,7 @@ impl FusedIterator for SymbolPropertyKeys<'_> {}
/// An iterator over the `Symbol` values (`Property`) of an `Object`.
#[derive(Debug, Clone)]
pub struct SymbolPropertyValues<'a>(hash_map::Values<'a, JsSymbol, PropertyDescriptor>);
pub struct SymbolPropertyValues<'a>(indexmap::map::Values<'a, JsSymbol, PropertyDescriptor>);
impl<'a> Iterator for SymbolPropertyValues<'a> {
type Item = &'a PropertyDescriptor;
@ -395,7 +419,7 @@ impl FusedIterator for IndexPropertyValues<'_> {}
/// An iterator over the `String` property entries of an `Object`
#[derive(Debug, Clone)]
pub struct StringProperties<'a>(hash_map::Iter<'a, JsString, PropertyDescriptor>);
pub struct StringProperties<'a>(indexmap::map::Iter<'a, JsString, PropertyDescriptor>);
impl<'a> Iterator for StringProperties<'a> {
type Item = (&'a JsString, &'a PropertyDescriptor);
@ -422,7 +446,7 @@ impl FusedIterator for StringProperties<'_> {}
/// An iterator over the string keys (`RcString`) of an `Object`.
#[derive(Debug, Clone)]
pub struct StringPropertyKeys<'a>(hash_map::Keys<'a, JsString, PropertyDescriptor>);
pub struct StringPropertyKeys<'a>(indexmap::map::Keys<'a, JsString, PropertyDescriptor>);
impl<'a> Iterator for StringPropertyKeys<'a> {
type Item = &'a JsString;
@ -449,7 +473,7 @@ impl FusedIterator for StringPropertyKeys<'_> {}
/// An iterator over the string values (`Property`) of an `Object`.
#[derive(Debug, Clone)]
pub struct StringPropertyValues<'a>(hash_map::Values<'a, JsString, PropertyDescriptor>);
pub struct StringPropertyValues<'a>(indexmap::map::Values<'a, JsString, PropertyDescriptor>);
impl<'a> Iterator for StringPropertyValues<'a> {
type Item = &'a PropertyDescriptor;

29
boa/src/object/tests.rs

@ -1,4 +1,4 @@
use crate::exec;
use crate::{check_output, exec, TestAction};
#[test]
fn ordinary_has_instance_nonobject_prototype() {
@ -17,3 +17,30 @@ fn ordinary_has_instance_nonobject_prototype() {
"\"TypeError: function has non-object prototype in instanceof check\""
);
}
#[test]
fn object_properties_return_order() {
let scenario = r#"
var o = {
p1: 'v1',
p2: 'v2',
p3: 'v3',
};
o.p4 = 'v4';
o[2] = 'iv2';
o[0] = 'iv0';
o[1] = 'iv1';
delete o.p1;
delete o.p3;
o.p1 = 'v1';
"#;
check_output(&[
TestAction::Execute(scenario),
TestAction::TestEq("Object.keys(o)", r#"[ "0", "1", "2", "p2", "p4", "p1" ]"#),
TestAction::TestEq(
"Object.values(o)",
r#"[ "iv0", "iv1", "iv2", "v2", "v4", "v1" ]"#,
),
]);
}

Loading…
Cancel
Save