ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix to invalidate execution cache when top-level annotations change (#5238)

Browse Source 
* Fix to invalidate cache when top-level annotations change

* Change to BTreeMap to allow deterministic iteration

* Fix execution cache to look at annotations
This commit is contained in:
Jonathan Tran
2025-02-05 20:23:29 -05:00
committed by GitHub
parent 2b7325655b
commit fed922cfb8
5 changed files with 193 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/wasm-lib/kcl/Cargo.toml
View File

@ -32,6 +32,7 @@ gltf-json = "1.4.1"
http = { workspace = true }
image = { version = "0.25.5", default-features = false, features = ["png"] }
indexmap = { version = "2.7.0", features = ["serde"] }
itertools = "0.13.0"
kittycad = { workspace = true }
kittycad-modeling-cmds = { workspace = true }
lazy_static = "1.5.0"
@ -115,7 +116,6 @@ expectorate = "1.1.0"
handlebars = "6.3.0"
image = { version = "0.25.5", default-features = false, features = ["png"] }
insta = { version = "1.41.1", features = ["json", "filters", "redactions"] }
itertools = "0.13.0"
miette = { version = "7.2.0", features = ["fancy"] }
pretty_assertions = "1.4.1"
tokio = { version = "1.41.1", features = ["rt-multi-thread", "macros", "time"] }

81
src/wasm-lib/kcl/src/execution/cache.rs
View File

@ -2,11 +2,12 @@
use std::sync::Arc;
use itertools::{EitherOrBoth, Itertools};
use tokio::sync::RwLock;
use crate::{
execution::{ExecState, ExecutorSettings},
parsing::ast::types::{Node, Program},
parsing::ast::types::{Node, NonCodeValue, Program},
walk::Node as WalkNode,
};
@ -113,6 +114,44 @@ pub(super) async fn get_changed_program(old: CacheInformation<'_>, new: CacheInf
return CacheResult::NoAction(try_reapply_settings);
}
// Check if the annotations are different.
if !old_ast.annotations().zip_longest(new_ast.annotations()).all(|pair| {
match pair {
EitherOrBoth::Both(old, new) => {
// Compare annotations, ignoring source ranges. Digests must
// have been computed before this.
match (&old.value, &new.value) {
(
NonCodeValue::Annotation { name, properties },
NonCodeValue::Annotation {
name: new_name,
properties: new_properties,
},
) => {
name.digest == new_name.digest
&& properties
.as_ref()
.map(|props| props.iter().map(|p| p.digest).collect::<Vec<_>>())
== new_properties
.as_ref()
.map(|props| props.iter().map(|p| p.digest).collect::<Vec<_>>())
}
_ => false,
}
}
_ => false,
}
}) {
// If any of the annotations are different at the beginning of the
// program, it's likely the settings, and we have to bust the cache and
// re-execute the whole thing.
return CacheResult::ReExecute {
clear_scene: true,
reapply_settings: true,
program: new.ast.clone(),
};
}
// Check if the changes were only to Non-code areas, like comments or whitespace.
let (clear_scene, program) = generate_changed_program(old_ast, new_ast);
CacheResult::ReExecute {
@ -126,6 +165,8 @@ pub(super) async fn get_changed_program(old: CacheInformation<'_>, new: CacheInf
/// way in which this gets invoked should always be through
/// [get_changed_program]. This is purely to contain the logic on
/// how we construct a new [CacheResult].
///
/// Digests *must* be computed before calling this.
fn generate_changed_program(old_ast: Node<Program>, mut new_ast: Node<Program>) -> (bool, Node<Program>) {
if !old_ast.body.iter().zip(new_ast.body.iter()).all(|(old, new)| {
let old_node: WalkNode = old.into();
@ -466,4 +507,42 @@ shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
assert_eq!(result, CacheResult::NoAction(true));
}
// Changing the units settings using an annotation with the exact same file
// should bust the cache.
#[tokio::test(flavor = "multi_thread")]
async fn test_get_changed_program_same_code_but_different_unit_setting_using_annotation() {
let old_code = r#"@settings(defaultLengthUnit = in)
startSketchOn('XY')
"#;
let new_code = r#"@settings(defaultLengthUnit = mm)
startSketchOn('XY')
"#;
let (program, ctx, _) = parse_execute(old_code).await.unwrap();
let mut new_program = crate::Program::parse_no_errs(new_code).unwrap();
new_program.compute_digest();
let result = get_changed_program(
CacheInformation {
ast: &program.ast,
settings: &ctx.settings,
},
CacheInformation {
ast: &new_program.ast,
settings: &ctx.settings,
},
)
.await;
assert_eq!(
result,
CacheResult::ReExecute {
clear_scene: true,
reapply_settings: true,
program: new_program.ast
}
);
}
}

69
src/wasm-lib/kcl/src/parsing/ast/digest.rs
View File

@ -1,6 +1,9 @@
use sha2::{Digest as DigestTrait, Sha256};
use super::types::{DefaultParamVal, ItemVisibility, LabelledExpression, LiteralValue, VariableKind};
use super::types::{
DefaultParamVal, ItemVisibility, LabelledExpression, LiteralValue, NonCodeMeta, NonCodeNode, NonCodeValue,
VariableKind,
};
use crate::parsing::ast::types::{
ArrayExpression, ArrayRangeExpression, BinaryExpression, BinaryPart, BodyItem, CallExpression, CallExpressionKw,
ElseIf, Expr, ExpressionStatement, FnArgType, FunctionExpression, Identifier, IfExpression, ImportItem,
@ -79,12 +82,60 @@ impl Program {
for body_item in slf.body.iter_mut() {
hasher.update(body_item.compute_digest());
}
// This contains settings annotations.
hasher.update(slf.non_code_meta.compute_digest());
if let Some(shebang) = &slf.shebang {
hasher.update(&shebang.inner.content);
}
});
}
impl NonCodeMeta {
compute_digest!(|slf, hasher| {
for non_code_node in slf.start_nodes.iter_mut() {
hasher.update(non_code_node.compute_digest());
}
for (_, non_code_nodes) in slf.non_code_nodes.iter_mut() {
for non_code_node in non_code_nodes.iter_mut() {
hasher.update(non_code_node.compute_digest());
}
}
});
}
impl NonCodeNode {
compute_digest!(|slf, hasher| {
hasher.update(slf.value.compute_digest());
});
}
impl NonCodeValue {
pub fn compute_digest(&mut self) -> Digest {
let mut hasher = Sha256::new();
match self {
NonCodeValue::InlineComment { .. } => {}
NonCodeValue::BlockComment { .. } => {}
NonCodeValue::NewLineBlockComment { .. } => {}
NonCodeValue::NewLine => {}
NonCodeValue::Annotation {
ref mut name,
properties,
} => {
hasher.update(name.compute_digest());
if let Some(properties) = properties {
hasher.update(properties.len().to_ne_bytes());
for property in properties.iter_mut() {
hasher.update(property.compute_digest());
}
} else {
hasher.update("no_properties");
}
}
}
hasher.finalize().into()
}
}
impl BodyItem {
pub fn compute_digest(&mut self) -> Digest {
match self {
@ -460,4 +511,20 @@ mod test {
assert_eq!(prog1_digest, prog3_digest);
}
#[tokio::test(flavor = "multi_thread")]
async fn test_annotations_digest() {
// Settings annotations should be included in the digest.
let prog1_string = r#"@settings(defaultLengthUnit = in)
startSketchOn('XY')
"#;
let prog1_digest = crate::parsing::top_level_parse(prog1_string).unwrap().compute_digest();
let prog2_string = r#"@settings(defaultLengthUnit = mm)
startSketchOn('XY')
"#;
let prog2_digest = crate::parsing::top_level_parse(prog2_string).unwrap().compute_digest();
assert!(prog1_digest != prog2_digest);
}
}

59
src/wasm-lib/kcl/src/parsing/ast/types/mod.rs
View File

@ -2,7 +2,7 @@
use std::{
cell::RefCell,
collections::HashMap,
collections::{BTreeMap, HashMap},
fmt,
ops::{Deref, DerefMut, RangeInclusive},
rc::Rc,
@ -254,15 +254,26 @@ impl Node<Program> {
Ok(findings)
}
/// Get the annotations for the meta settings from the kcl file.
pub fn get_meta_settings(&self) -> Result<Option<crate::execution::MetaSettings>, KclError> {
let annotations = self
.non_code_meta
pub fn annotations(&self) -> impl Iterator<Item = &Node<NonCodeNode>> {
self.non_code_meta
.start_nodes
.iter()
.filter_map(|n| n.annotation().map(|result| (result, n.as_source_range())));
for (annotation, source_range) in annotations {
.filter(|n| n.value_is_annotation())
}
pub fn annotations_mut(&mut self) -> impl Iterator<Item = &mut Node<NonCodeNode>> {
self.non_code_meta
.start_nodes
.iter_mut()
.filter(|n| n.value_is_annotation())
}
/// Get the annotations for the meta settings from the kcl file.
pub fn get_meta_settings(&self) -> Result<Option<crate::execution::MetaSettings>, KclError> {
for annotation_node in self.annotations() {
let annotation = &annotation_node.value;
if annotation.annotation_name() == Some(annotations::SETTINGS) {
let source_range = annotation_node.as_source_range();
let mut meta_settings = crate::execution::MetaSettings::default();
meta_settings.update_from_annotation(annotation, source_range)?;
return Ok(Some(meta_settings));
@ -275,17 +286,15 @@ impl Node<Program> {
pub fn change_meta_settings(&mut self, settings: crate::execution::MetaSettings) -> Result<Self, KclError> {
let mut new_program = self.clone();
let mut found = false;
for node in &mut new_program.non_code_meta.start_nodes {
if let Some(annotation) = node.annotation() {
if annotation.annotation_name() == Some(annotations::SETTINGS) {
let annotation = NonCodeValue::new_from_meta_settings(&settings);
*node = Node::no_src(NonCodeNode {
value: annotation,
digest: None,
});
found = true;
break;
}
for node in new_program.annotations_mut() {
if node.value.annotation_name() == Some(annotations::SETTINGS) {
let annotation = NonCodeValue::new_from_meta_settings(&settings);
*node = Node::no_src(NonCodeNode {
value: annotation,
digest: None,
});
found = true;
break;
}
}
@ -1090,6 +1099,16 @@ impl NonCodeNode {
_ => None,
}
}
pub fn value_is_annotation(&self) -> bool {
match self.value {
NonCodeValue::InlineComment { .. }
| NonCodeValue::BlockComment { .. }
| NonCodeValue::NewLineBlockComment { .. }
| NonCodeValue::NewLine => false,
NonCodeValue::Annotation { .. } => true,
}
}
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
@ -1175,7 +1194,7 @@ impl NonCodeValue {
#[ts(export)]
#[serde(rename_all = "camelCase")]
pub struct NonCodeMeta {
pub non_code_nodes: HashMap<usize, NodeList<NonCodeNode>>,
pub non_code_nodes: BTreeMap<usize, NodeList<NonCodeNode>>,
pub start_nodes: NodeList<NonCodeNode>,
#[serde(default, skip_serializing_if = "Option::is_none")]
@ -1214,7 +1233,7 @@ impl<'de> Deserialize<'de> for NonCodeMeta {
.non_code_nodes
.into_iter()
.map(|(key, value)| Ok((key.parse().map_err(serde::de::Error::custom)?, value)))
.collect::<Result<HashMap<_, _>, _>>()?;
.collect::<Result<BTreeMap<_, _>, _>>()?;
Ok(NonCodeMeta {
non_code_nodes,
start_nodes: helper.start_nodes,

10
src/wasm-lib/kcl/src/parsing/parser.rs
View File

@ -1,7 +1,7 @@
// TODO optimise size of CompilationError
#![allow(clippy::result_large_err)]
use std::{cell::RefCell, collections::HashMap, str::FromStr};
use std::{cell::RefCell, collections::BTreeMap, str::FromStr};
use winnow::{
combinator::{alt, delimited, opt, peek, preceded, repeat, separated, separated_pair, terminated},
@ -755,8 +755,8 @@ pub(crate) fn array_elem_by_elem(i: &mut TokenSlice) -> PResult<Node<ArrayExpres
.end;
// Sort the array's elements (i.e. expression nodes) from the noncode nodes.
let (elements, non_code_nodes): (Vec<_>, HashMap<usize, _>) = elements.into_iter().enumerate().fold(
(Vec::new(), HashMap::new()),
let (elements, non_code_nodes): (Vec<_>, BTreeMap<usize, _>) = elements.into_iter().enumerate().fold(
(Vec::new(), BTreeMap::new()),
|(mut elements, mut non_code_nodes), (i, e)| {
match e {
NonCodeOr::NonCode(x) => {
@ -898,8 +898,8 @@ pub(crate) fn object(i: &mut TokenSlice) -> PResult<Node<ObjectExpression>> {
.parse_next(i)?;
// Sort the object's properties from the noncode nodes.
let (properties, non_code_nodes): (Vec<_>, HashMap<usize, _>) = properties.into_iter().enumerate().fold(
(Vec::new(), HashMap::new()),
let (properties, non_code_nodes): (Vec<_>, BTreeMap<usize, _>) = properties.into_iter().enumerate().fold(
(Vec::new(), BTreeMap::new()),
|(mut properties, mut non_code_nodes), (i, e)| {
match e {
NonCodeOr::NonCode(x) => {