From fb0974d14c375eebe1e336705f25720c872dc96b Mon Sep 17 00:00:00 2001 From: laststylebender <43403528+laststylebender14@users.noreply.github.com> Date: Mon, 4 Nov 2024 20:43:05 +0530 Subject: [PATCH] fix: dedupe operations that are cacheable (#3099) Co-authored-by: Tushar Mathur --- src/core/jit/graphql_executor.rs | 2 +- src/core/jit/model.rs | 8 ++++ src/core/jit/request.rs | 1 + src/core/jit/transform/check_cache.rs | 59 ++++++++++++++++++++++++ src/core/jit/transform/input_resolver.rs | 1 + src/core/jit/transform/mod.rs | 2 + 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 src/core/jit/transform/check_cache.rs diff --git a/src/core/jit/graphql_executor.rs b/src/core/jit/graphql_executor.rs index f3346fe988..0ed7e1dc37 100644 --- a/src/core/jit/graphql_executor.rs +++ b/src/core/jit/graphql_executor.rs @@ -113,7 +113,7 @@ impl JITExecutor { let is_const = exec.plan.is_const; let is_protected = exec.plan.is_protected; - let response = if exec.plan.is_query() && (exec.plan.is_dedupe || exec.plan.is_const) { + let response = if exec.plan.can_dedupe() { self.dedupe_and_exec(exec, jit_request).await } else { self.exec(exec, jit_request).await diff --git a/src/core/jit/model.rs b/src/core/jit/model.rs index 6996f7bfcb..f0b30ce21c 100644 --- a/src/core/jit/model.rs +++ b/src/core/jit/model.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::fmt::{Debug, Formatter}; +use std::num::NonZeroU64; use std::sync::Arc; use async_graphql::parser::types::{ConstDirective, OperationType}; @@ -287,6 +288,7 @@ pub struct OperationPlan { pub is_dedupe: bool, pub is_const: bool, pub is_protected: bool, + pub min_cache_ttl: Option, pub selection: Vec>, } @@ -317,6 +319,7 @@ impl OperationPlan { is_introspection_query: self.is_introspection_query, is_dedupe: self.is_dedupe, is_const: self.is_const, + min_cache_ttl: self.min_cache_ttl, is_protected: self.is_protected, }) } @@ -342,6 +345,7 @@ impl OperationPlan { is_introspection_query, is_dedupe: false, is_const: false, + min_cache_ttl: None, is_protected: false, } } @@ -409,6 +413,10 @@ impl OperationPlan { None => true, } } + /// returns true if plan is dedupable + pub fn can_dedupe(&self) -> bool { + self.is_query() && (self.is_dedupe || self.is_const || self.min_cache_ttl.is_some()) + } } #[derive(Clone, Debug)] diff --git a/src/core/jit/request.rs b/src/core/jit/request.rs index 0caf3d4ee2..459ccf7162 100644 --- a/src/core/jit/request.rs +++ b/src/core/jit/request.rs @@ -48,6 +48,7 @@ impl Request { transform::CheckConst::new() .pipe(transform::CheckDedupe::new()) .pipe(transform::CheckProtected::new()) + .pipe(transform::CheckCache::new()) .transform(plan) .to_result() // both transformers are infallible right now diff --git a/src/core/jit/transform/check_cache.rs b/src/core/jit/transform/check_cache.rs new file mode 100644 index 0000000000..2bc82e2637 --- /dev/null +++ b/src/core/jit/transform/check_cache.rs @@ -0,0 +1,59 @@ +use std::convert::Infallible; +use std::num::NonZeroU64; + +use tailcall_valid::Valid; + +use crate::core::ir::model::IR; +use crate::core::jit::OperationPlan; +use crate::core::Transform; + +/// A transformer that sets the minimum cache TTL for the operation plan based +/// on the IR. +pub struct CheckCache(std::marker::PhantomData); +impl CheckCache { + pub fn new() -> Self { + Self(std::marker::PhantomData) + } +} + +#[inline] +fn check_cache(ir: &IR) -> Option { + match ir { + IR::IO(_) => None, + IR::Cache(cache) => Some(cache.max_age), + IR::Path(ir, _) => check_cache(ir), + IR::Protect(ir) => check_cache(ir), + IR::Pipe(ir, ir1) => match (check_cache(ir), check_cache(ir1)) { + (Some(age1), Some(age2)) => Some(age1.min(age2)), + _ => None, + }, + IR::Discriminate(_, ir) => check_cache(ir), + IR::Entity(hash_map) => { + let mut ttl = Some(NonZeroU64::MAX); + for ir in hash_map.values() { + ttl = std::cmp::min(ttl, check_cache(ir)); + } + ttl + } + IR::Dynamic(_) | IR::ContextPath(_) | IR::Map(_) | IR::Service(_) => None, + } +} + +impl Transform for CheckCache { + type Value = OperationPlan; + type Error = Infallible; + + fn transform(&self, mut plan: Self::Value) -> Valid { + let mut ttl = Some(NonZeroU64::MAX); + + for field in plan.selection.iter() { + if let Some(ir) = field.ir.as_ref() { + ttl = std::cmp::min(ttl, check_cache(ir)); + } + } + + plan.min_cache_ttl = ttl; + + Valid::succeed(plan) + } +} diff --git a/src/core/jit/transform/input_resolver.rs b/src/core/jit/transform/input_resolver.rs index 3fb3072805..b83d524238 100644 --- a/src/core/jit/transform/input_resolver.rs +++ b/src/core/jit/transform/input_resolver.rs @@ -76,6 +76,7 @@ where is_dedupe: self.plan.is_dedupe, is_const: self.plan.is_const, is_protected: self.plan.is_protected, + min_cache_ttl: self.plan.min_cache_ttl, selection, }) } diff --git a/src/core/jit/transform/mod.rs b/src/core/jit/transform/mod.rs index 1f5c515f80..986e5c695c 100644 --- a/src/core/jit/transform/mod.rs +++ b/src/core/jit/transform/mod.rs @@ -1,9 +1,11 @@ +mod check_cache; mod check_const; mod check_dedupe; mod check_protected; mod input_resolver; mod skip; +pub use check_cache::*; pub use check_const::*; pub use check_dedupe::*; pub use check_protected::*;