From b7e0828ced75bbdb6625df4f5ad18bc4cf8e6237 Mon Sep 17 00:00:00 2001 From: Jeff Date: Mon, 12 Feb 2024 15:07:41 -0500 Subject: [PATCH] Make maps multi-threaded again --- src/abstract_tree/for.rs | 26 +++++++------- src/abstract_tree/index.rs | 2 ++ src/abstract_tree/value_node.rs | 6 ++-- src/built_in_functions/mod.rs | 2 +- src/main.rs | 2 +- src/value/map.rs | 63 ++++++++++++++++++++++++--------- src/value/mod.rs | 22 ++++++++---- tests/for_loop.rs | 12 +++---- tests/functions.rs | 4 +-- tests/value.rs | 14 ++++---- 10 files changed, 99 insertions(+), 54 deletions(-) diff --git a/src/abstract_tree/for.rs b/src/abstract_tree/for.rs index 63aacf0..8ba104d 100644 --- a/src/abstract_tree/for.rs +++ b/src/abstract_tree/for.rs @@ -102,23 +102,23 @@ impl AbstractTree for For { return Ok(Value::none()); } - let values = expression_run.as_list()?.items(); + if let Value::List(list) = expression_run { + if self.is_async { + list.items().par_iter().try_for_each(|value| { + let iter_context = Context::with_variables_from(context)?; - if self.is_async { - values.par_iter().try_for_each(|value| { - let iter_context = Context::with_variables_from(context)?; + iter_context.set_value(key.clone(), value.clone())?; - iter_context.set_value(key.clone(), value.clone())?; + self.block.run(source, &iter_context).map(|_value| ()) + })?; + } else { + let loop_context = Context::with_variables_from(context)?; - self.block.run(source, &iter_context).map(|_value| ()) - })?; - } else { - let loop_context = Context::with_variables_from(context)?; + for value in list.items().iter() { + loop_context.set_value(key.clone(), value.clone())?; - for value in values.iter() { - loop_context.set_value(key.clone(), value.clone())?; - - self.block.run(source, &loop_context)?; + self.block.run(source, &loop_context)?; + } } } diff --git a/src/abstract_tree/index.rs b/src/abstract_tree/index.rs index 987e10f..853d153 100644 --- a/src/abstract_tree/index.rs +++ b/src/abstract_tree/index.rs @@ -84,6 +84,8 @@ impl AbstractTree for Index { Ok(item) } Value::Map(map) => { + let map = map.inner()?; + let (key, value) = if let IndexExpression::Identifier(identifier) = &self.index { let key = identifier.inner(); let value = map.get(key).unwrap_or_default(); diff --git a/src/abstract_tree/value_node.rs b/src/abstract_tree/value_node.rs index 5a3ff38..1a15b9d 100644 --- a/src/abstract_tree/value_node.rs +++ b/src/abstract_tree/value_node.rs @@ -294,17 +294,17 @@ impl AbstractTree for ValueNode { Value::Option(option_value) } ValueNode::Map(key_statement_pairs, _) => { - let mut map = Map::new(); + let mut map = BTreeMap::new(); { for (key, (statement, _)) in key_statement_pairs { let value = statement.run(source, context)?; - map.set(key.clone(), value); + map.insert(key.clone(), value); } } - Value::Map(map) + Value::Map(Map::with_values(map)) } ValueNode::BuiltInValue(built_in_value) => built_in_value.run(source, context)?, ValueNode::Structure(node_map) => { diff --git a/src/built_in_functions/mod.rs b/src/built_in_functions/mod.rs index 600efba..fd7b575 100644 --- a/src/built_in_functions/mod.rs +++ b/src/built_in_functions/mod.rs @@ -111,7 +111,7 @@ impl Callable for BuiltInFunction { let length = if let Ok(list) = value.as_list() { list.items().len() } else if let Ok(map) = value.as_map() { - map.len() + map.inner()?.len() } else if let Ok(str) = value.as_string() { str.chars().count() } else { diff --git a/src/main.rs b/src/main.rs index 018715f..2bafb38 100644 --- a/src/main.rs +++ b/src/main.rs @@ -293,7 +293,7 @@ impl Completer for DustCompleter { } if let Value::Map(map) = built_in_value.get() { - for (key, value) in map.iter() { + for (key, value) in map.inner().unwrap().iter() { if key.contains(last_word) { suggestions.push(Suggestion { value: format!("{name}:{key}"), diff --git a/src/value/map.rs b/src/value/map.rs index 8a1eb34..f90ea27 100644 --- a/src/value/map.rs +++ b/src/value/map.rs @@ -1,4 +1,3 @@ -use serde::{Deserialize, Serialize}; use stanza::{ renderer::{console::Console, Renderer}, style::{HAlign, Styles}, @@ -7,51 +6,52 @@ use stanza::{ use std::{ collections::BTreeMap, fmt::{self, Display, Formatter}, + sync::{Arc, RwLock, RwLockReadGuard}, }; -use crate::value::Value; +use crate::{error::rw_lock_error::RwLockError, value::Value}; /// A collection dust variables comprised of key-value pairs. /// /// The inner value is a BTreeMap in order to allow VariableMap instances to be sorted and compared /// to one another. -#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Clone, Debug)] pub struct Map { - inner: BTreeMap, + inner: Arc>>, } impl Map { /// Creates a new instace. pub fn new() -> Self { Map { - inner: BTreeMap::new(), + inner: Arc::new(RwLock::new(BTreeMap::new())), } } pub fn with_values(variables: BTreeMap) -> Self { - Map { inner: variables } + Map { + inner: Arc::new(RwLock::new(variables)), + } } - pub fn len(&self) -> usize { - self.inner.len() + pub fn inner(&self) -> Result>, RwLockError> { + Ok(self.inner.read()?) } - pub fn iter(&self) -> impl Iterator { - self.inner.iter() + pub fn get(&self, key: &str) -> Result, RwLockError> { + Ok(self.inner()?.get(key).cloned()) } - pub fn get(&self, key: &str) -> Option<&Value> { - self.inner.get(key) - } + pub fn set(&self, key: String, value: Value) -> Result<(), RwLockError> { + self.inner.write()?.insert(key, value); - pub fn set(&mut self, key: String, value: Value) { - self.inner.insert(key, value); + Ok(()) } pub fn as_text_table(&self) -> Table { let mut table = Table::with_styles(Styles::default().with(HAlign::Centred)); - for (key, value) in &self.inner { + for (key, value) in self.inner().unwrap().iter() { if let Value::Map(map) = value { table.push_row(Row::new( Styles::default(), @@ -92,3 +92,34 @@ impl Display for Map { f.write_str(&renderer.render(&self.as_text_table())) } } + +impl Eq for Map {} + +impl PartialEq for Map { + fn eq(&self, other: &Self) -> bool { + let left = self.inner().unwrap(); + let right = other.inner().unwrap(); + + if left.len() != right.len() { + return false; + } + + left.iter() + .zip(right.iter()) + .all(|((left_key, left_value), (right_key, right_value))| { + left_key == right_key && left_value == right_value + }) + } +} + +impl PartialOrd for Map { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Map { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.inner().unwrap().cmp(&other.inner().unwrap()) + } +} diff --git a/src/value/mod.rs b/src/value/mod.rs index 3342ea6..0b43767 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -3,12 +3,13 @@ use crate::{error::RuntimeError, Identifier, Type, TypeSpecification}; use serde::{ de::{MapAccess, SeqAccess, Visitor}, - ser::SerializeTuple, + ser::{SerializeMap, SerializeTuple}, Deserialize, Serialize, Serializer, }; use std::{ cmp::Ordering, + collections::BTreeMap, convert::TryFrom, fmt::{self, Display, Formatter}, marker::PhantomData, @@ -82,7 +83,7 @@ impl Value { Value::Map(map) => { let mut identifier_types = Vec::new(); - for (key, value) in map.iter() { + for (key, value) in map.inner().unwrap().iter() { identifier_types.push(( Identifier::new(key.clone()), TypeSpecification::new(value.r#type()), @@ -513,7 +514,16 @@ impl Serialize for Value { list.end() } Value::Option(inner) => inner.serialize(serializer), - Value::Map(inner) => inner.serialize(serializer), + Value::Map(map) => { + let entries = map.inner().unwrap(); + let mut map = serializer.serialize_map(Some(entries.len()))?; + + for (key, value) in entries.iter() { + map.serialize_entry(key, value)?; + } + + map.end() + } Value::Function(inner) => inner.serialize(serializer), Value::Structure(inner) => inner.serialize(serializer), Value::Range(range) => range.serialize(serializer), @@ -858,13 +868,13 @@ impl<'de> Visitor<'de> for ValueVisitor { where M: MapAccess<'de>, { - let mut map = Map::new(); + let mut map = BTreeMap::new(); while let Some((key, value)) = access.next_entry::()? { - map.set(key, value); + map.insert(key, value); } - Ok(Value::Map(map)) + Ok(Value::Map(Map::with_values(map))) } fn visit_enum(self, data: A) -> std::result::Result diff --git a/tests/for_loop.rs b/tests/for_loop.rs index 3a4bcec..44a774a 100644 --- a/tests/for_loop.rs +++ b/tests/for_loop.rs @@ -92,10 +92,10 @@ fn modify_map() { ", ); - let mut map = Map::new(); + let map = Map::new(); - map.set("x".to_string(), Value::Integer(1)); - map.set("y".to_string(), Value::Integer(2)); + map.set("x".to_string(), Value::Integer(1)).unwrap(); + map.set("y".to_string(), Value::Integer(2)).unwrap(); assert_eq!(Ok(Value::Map(map)), result); } @@ -137,10 +137,10 @@ fn modify_map_values() { ", ); - let mut map = Map::new(); + let map = Map::new(); - map.set("x".to_string(), Value::Integer(1)); - map.set("y".to_string(), Value::Integer(2)); + map.set("x".to_string(), Value::Integer(1)).unwrap(); + map.set("y".to_string(), Value::Integer(2)).unwrap(); assert_eq!(Ok(Value::Map(map)), result); } diff --git a/tests/functions.rs b/tests/functions.rs index acb656f..3734ae6 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -89,9 +89,9 @@ fn function_context_captures_functions() { #[test] fn function_context_captures_structure_definitions() { - let mut map = Map::new(); + let map = Map::new(); - map.set("name".to_string(), Value::string("bob")); + map.set("name".to_string(), Value::string("bob")).unwrap(); assert_eq!( interpret( diff --git a/tests/value.rs b/tests/value.rs index 8ec6ef0..e9bcb4c 100644 --- a/tests/value.rs +++ b/tests/value.rs @@ -69,10 +69,11 @@ fn empty_list() { #[test] fn map() { - let mut map = Map::new(); + let map = Map::new(); - map.set("x".to_string(), Value::Integer(1)); - map.set("foo".to_string(), Value::string("bar".to_string())); + map.set("x".to_string(), Value::Integer(1)).unwrap(); + map.set("foo".to_string(), Value::string("bar".to_string())) + .unwrap(); assert_eq!(interpret("{ x = 1, foo = 'bar' }"), Ok(Value::Map(map))); } @@ -84,10 +85,11 @@ fn empty_map() { #[test] fn map_types() { - let mut map = Map::new(); + let map = Map::new(); - map.set("x".to_string(), Value::Integer(1)); - map.set("foo".to_string(), Value::string("bar".to_string())); + map.set("x".to_string(), Value::Integer(1)).unwrap(); + map.set("foo".to_string(), Value::string("bar".to_string())) + .unwrap(); assert_eq!( interpret("{ x = 1, foo = 'bar' }"),