From bc8b1040099d1dae6329354b2c89f9b3a3669a5b Mon Sep 17 00:00:00 2001 From: meskill <8974488+meskill@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:41:44 +0000 Subject: [PATCH] refactor: drop async_graphql engine from executing queries --- generated/.tailcallrc.schema.json | 1 + src/core/config/directives/server.rs | 6 ++-- src/core/http/request_handler.rs | 35 ++++++------------- src/core/jit/builder.rs | 20 +++++------ src/core/jit/fixtures/jp.rs | 6 ++-- src/core/jit/graphql_executor.rs | 3 ++ src/core/jit/request.rs | 2 +- src/core/jit/synth/synth.rs | 2 +- tests/core/parse.rs | 6 +--- .../snapshots/test-enable-jit.md_merged.snap | 2 +- .../test-required-fields.md_merged.snap | 2 +- 11 files changed, 33 insertions(+), 52 deletions(-) diff --git a/generated/.tailcallrc.schema.json b/generated/.tailcallrc.schema.json index 55fbf80df19..fe98371146a 100644 --- a/generated/.tailcallrc.schema.json +++ b/generated/.tailcallrc.schema.json @@ -477,6 +477,7 @@ ] }, "enableJIT": { + "writeOnly": true, "type": [ "boolean", "null" diff --git a/src/core/config/directives/server.rs b/src/core/config/directives/server.rs index ec006e20233..644e84f3288 100644 --- a/src/core/config/directives/server.rs +++ b/src/core/config/directives/server.rs @@ -29,10 +29,8 @@ use crate::core::macros::MergeRight; /// comprehensive set of server configurations. It dictates how the server /// behaves and helps tune tailcall for various use-cases. pub struct Server { - // The `enableJIT` option activates Just-In-Time (JIT) compilation. When set to true, it - // optimizes execution of each incoming request independently, resulting in significantly - // better performance in most cases, it's enabled by default. - #[serde(default, skip_serializing_if = "is_default", rename = "enableJIT")] + // TODO: drop this option + #[serde(default, skip_serializing, rename = "enableJIT")] pub enable_jit: Option, #[serde(default, skip_serializing_if = "is_default")] diff --git a/src/core/http/request_handler.rs b/src/core/http/request_handler.rs index 712929ee0d9..e7ab5efae2e 100644 --- a/src/core/http/request_handler.rs +++ b/src/core/http/request_handler.rs @@ -119,30 +119,17 @@ async fn execute_query( request: T, req: Parts, ) -> anyhow::Result> { - let mut response = if app_ctx.blueprint.server.enable_jit { - let operation_id = request.operation_id(&req.headers); - let exec = JITExecutor::new(app_ctx.clone(), req_ctx.clone(), operation_id); - request - .execute_with_jit(exec) - .await - .set_cache_control( - app_ctx.blueprint.server.enable_cache_control_header, - req_ctx.get_min_max_age().unwrap_or(0), - req_ctx.is_cache_public().unwrap_or(true), - ) - .into_response()? - } else { - request - .data(req_ctx.clone()) - .execute(&app_ctx.schema) - .await - .set_cache_control( - app_ctx.blueprint.server.enable_cache_control_header, - req_ctx.get_min_max_age().unwrap_or(0), - req_ctx.is_cache_public().unwrap_or(true), - ) - .into_response()? - }; + let operation_id = request.operation_id(&req.headers); + let exec = JITExecutor::new(app_ctx.clone(), req_ctx.clone(), operation_id); + let mut response = request + .execute_with_jit(exec) + .await + .set_cache_control( + app_ctx.blueprint.server.enable_cache_control_header, + req_ctx.get_min_max_age().unwrap_or(0), + req_ctx.is_cache_public().unwrap_or(true), + ) + .into_response()?; update_response_headers(&mut response, req_ctx, app_ctx); Ok(response) diff --git a/src/core/jit/builder.rs b/src/core/jit/builder.rs index 26ab6d1f66f..04f99c45cb1 100644 --- a/src/core/jit/builder.rs +++ b/src/core/jit/builder.rs @@ -64,16 +64,16 @@ impl Conditions { } } -pub struct Builder { +pub struct Builder<'a> { pub index: Arc, pub arg_id: Counter, pub field_id: Counter, - pub document: ExecutableDocument, + pub document: &'a ExecutableDocument, } // TODO: make generic over Value (Input) type -impl Builder { - pub fn new(blueprint: &Blueprint, document: ExecutableDocument) -> Self { +impl<'a> Builder<'a> { + pub fn new(blueprint: &Blueprint, document: &'a ExecutableDocument) -> Self { let index = Arc::new(blueprint.index()); Self { document, @@ -372,7 +372,7 @@ mod tests { let config = Config::from_sdl(CONFIG).to_result().unwrap(); let blueprint = Blueprint::try_from(&config.into()).unwrap(); let document = async_graphql::parser::parse_query(query).unwrap(); - Builder::new(&blueprint, document).build(None).unwrap() + Builder::new(&blueprint, &document).build(None).unwrap() } #[tokio::test] @@ -640,25 +640,23 @@ mod tests { let config = Config::from_sdl(CONFIG).to_result().unwrap(); let blueprint = Blueprint::try_from(&config.into()).unwrap(); let document = async_graphql::parser::parse_query(query).unwrap(); - let error = Builder::new(&blueprint, document.clone()) - .build(None) - .unwrap_err(); + let error = Builder::new(&blueprint, &document).build(None).unwrap_err(); assert_eq!(error, BuildError::OperationNameRequired); - let error = Builder::new(&blueprint, document.clone()) + let error = Builder::new(&blueprint, &document) .build(Some("unknown")) .unwrap_err(); assert_eq!(error, BuildError::OperationNotFound("unknown".to_string())); - let plan = Builder::new(&blueprint, document.clone()) + let plan = Builder::new(&blueprint, &document) .build(Some("GetPosts")) .unwrap(); assert!(plan.is_query()); insta::assert_debug_snapshot!(plan.selection); - let plan = Builder::new(&blueprint, document.clone()) + let plan = Builder::new(&blueprint, &document) .build(Some("CreateNewPost")) .unwrap(); assert!(!plan.is_query()); diff --git a/src/core/jit/fixtures/jp.rs b/src/core/jit/fixtures/jp.rs index 2d0c99b45c8..f743fdc2815 100644 --- a/src/core/jit/fixtures/jp.rs +++ b/src/core/jit/fixtures/jp.rs @@ -88,10 +88,8 @@ impl<'a, Value: Deserialize<'a> + Clone + 'a + JsonLike<'a> + std::fmt::Debug> J fn plan(query: &str, variables: &Variables) -> OperationPlan { let config = ConfigModule::from(Config::from_sdl(Self::CONFIG).to_result().unwrap()); - let builder = Builder::new( - &Blueprint::try_from(&config).unwrap(), - async_graphql::parser::parse_query(query).unwrap(), - ); + let doc = async_graphql::parser::parse_query(query).unwrap(); + let builder = Builder::new(&Blueprint::try_from(&config).unwrap(), &doc); let plan = builder.build(None).unwrap(); let plan = transform::Skip::new(variables) diff --git a/src/core/jit/graphql_executor.rs b/src/core/jit/graphql_executor.rs index 31afb8af0fe..e1c131c3747 100644 --- a/src/core/jit/graphql_executor.rs +++ b/src/core/jit/graphql_executor.rs @@ -75,6 +75,8 @@ impl JITExecutor { &self, request: async_graphql::Request, ) -> impl Future>> + Send + '_ { + // TODO: hash considering only the query itself ignoring specified operation and + // variables that could differ for the same query let hash = Self::req_hash(&request); async move { @@ -135,6 +137,7 @@ impl JITExecutor { } } +// TODO: used only for introspection, simplify somehow? impl From> for async_graphql::Request { fn from(value: jit::Request) -> Self { let mut request = async_graphql::Request::new(value.query); diff --git a/src/core/jit/request.rs b/src/core/jit/request.rs index cbd3bc0faf2..f37c4a721e4 100644 --- a/src/core/jit/request.rs +++ b/src/core/jit/request.rs @@ -42,7 +42,7 @@ impl Request { blueprint: &Blueprint, ) -> Result> { let doc = async_graphql::parser::parse_query(&self.query)?; - let builder = Builder::new(blueprint, doc); + let builder = Builder::new(blueprint, &doc); let plan = builder.build(self.operation_name.as_deref())?; transform::CheckConst::new() diff --git a/src/core/jit/synth/synth.rs b/src/core/jit/synth/synth.rs index e24b6c50a04..221fa95da2b 100644 --- a/src/core/jit/synth/synth.rs +++ b/src/core/jit/synth/synth.rs @@ -345,7 +345,7 @@ mod tests { let config = Config::from_sdl(CONFIG).to_result().unwrap(); let config = ConfigModule::from(config); - let builder = Builder::new(&Blueprint::try_from(&config).unwrap(), doc); + let builder = Builder::new(&Blueprint::try_from(&config).unwrap(), &doc); let plan = builder.build(None).unwrap(); let plan = plan .try_map(|v| { diff --git a/tests/core/parse.rs b/tests/core/parse.rs index a438a31e89d..6246c400087 100644 --- a/tests/core/parse.rs +++ b/tests/core/parse.rs @@ -273,11 +273,7 @@ impl ExecutionSpec { env: HashMap, http: Arc, ) -> Arc { - let mut blueprint = Blueprint::try_from(config).unwrap(); - - if cfg!(feature = "force_jit") { - blueprint.server.enable_jit = true; - } + let blueprint = Blueprint::try_from(config).unwrap(); let script = blueprint.server.script.clone(); diff --git a/tests/core/snapshots/test-enable-jit.md_merged.snap b/tests/core/snapshots/test-enable-jit.md_merged.snap index 3d61f7941a0..99a8a80fcba 100644 --- a/tests/core/snapshots/test-enable-jit.md_merged.snap +++ b/tests/core/snapshots/test-enable-jit.md_merged.snap @@ -3,7 +3,7 @@ source: tests/core/spec.rs expression: formatter snapshot_kind: text --- -schema @server(enableJIT: true, hostname: "0.0.0.0", port: 8000) @upstream { +schema @server(hostname: "0.0.0.0", port: 8000) @upstream { query: Query } diff --git a/tests/core/snapshots/test-required-fields.md_merged.snap b/tests/core/snapshots/test-required-fields.md_merged.snap index 32c7a45a633..5cb6c7f4f33 100644 --- a/tests/core/snapshots/test-required-fields.md_merged.snap +++ b/tests/core/snapshots/test-required-fields.md_merged.snap @@ -3,7 +3,7 @@ source: tests/core/spec.rs expression: formatter snapshot_kind: text --- -schema @server(enableJIT: true) @upstream { +schema @server @upstream { query: Query }