Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: drop async_graphql engine from executing queries #3223

Merged
merged 1 commit into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions benches/handle_request_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@ pub fn benchmark_handle_request(c: &mut Criterion) {
let sdl = std::fs::read_to_string("./ci-benchmark/benchmark.graphql").unwrap();
let config_module: ConfigModule = Config::from_sdl(sdl.as_str()).to_result().unwrap().into();

let mut blueprint = Blueprint::try_from(&config_module).unwrap();
let mut blueprint_clone = blueprint.clone();
let blueprint = Blueprint::try_from(&config_module).unwrap();

let endpoints = config_module.extensions().endpoint_set.clone();
let endpoints_clone = endpoints.clone();

blueprint.server.enable_jit = false;
let server_config = tokio_runtime
.block_on(ServerConfig::new(blueprint.clone(), endpoints.clone()))
.unwrap();
Expand All @@ -47,9 +45,8 @@ pub fn benchmark_handle_request(c: &mut Criterion) {
})
});

blueprint_clone.server.enable_jit = true;
let server_config = tokio_runtime
.block_on(ServerConfig::new(blueprint_clone, endpoints_clone))
.block_on(ServerConfig::new(blueprint, endpoints_clone))
.unwrap();
let server_config = Arc::new(server_config);

Expand Down
6 changes: 0 additions & 6 deletions generated/.tailcallrc.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,6 @@
"null"
]
},
"enableJIT": {
"type": [
"boolean",
"null"
]
},
"globalResponseTimeout": {
"description": "`globalResponseTimeout` sets the maximum query duration before termination, acting as a safeguard against long-running queries.",
"type": [
Expand Down
2 changes: 0 additions & 2 deletions src/core/blueprint/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::core::config::{self, ConfigModule, HttpVersion, PrivateKey, Routes};

#[derive(Clone, Debug, Setters)]
pub struct Server {
pub enable_jit: bool,
pub enable_apollo_tracing: bool,
pub enable_cache_control_header: bool,
pub enable_set_cookie_header: bool,
Expand Down Expand Up @@ -124,7 +123,6 @@ impl TryFrom<crate::core::config::ConfigModule> for Server {
))
.map(
|(hostname, http, response_headers, script, experimental_headers, cors)| Server {
enable_jit: (config_server).enable_jit(),
enable_apollo_tracing: (config_server).enable_apollo_tracing(),
enable_cache_control_header: (config_server).enable_cache_control(),
enable_set_cookie_header: (config_server).enable_set_cookies(),
Expand Down
11 changes: 3 additions & 8 deletions src/core/config/directives/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ 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")]
#[deprecated(note = "No longer used, TODO: drop it")]
#[serde(default, skip_serializing, rename = "enableJIT")]
#[schemars(skip)]
pub enable_jit: Option<bool>,

#[serde(default, skip_serializing_if = "is_default")]
Expand Down Expand Up @@ -262,10 +261,6 @@ impl Server {
self.pipeline_flush.unwrap_or(true)
}

pub fn enable_jit(&self) -> bool {
self.enable_jit.unwrap_or(true)
}

pub fn get_routes(&self) -> Routes {
self.routes.clone().unwrap_or_default()
}
Expand Down
35 changes: 11 additions & 24 deletions src/core/http/request_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,30 +119,17 @@ async fn execute_query<T: DeserializeOwned + GraphQLRequestLike>(
request: T,
req: Parts,
) -> anyhow::Result<Response<Body>> {
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)
Expand Down
20 changes: 9 additions & 11 deletions src/core/jit/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,16 @@ impl Conditions {
}
}

pub struct Builder {
pub struct Builder<'a> {
pub index: Arc<Index>,
pub arg_id: Counter<usize>,
pub field_id: Counter<usize>,
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,
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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());
Expand Down
6 changes: 2 additions & 4 deletions src/core/jit/fixtures/jp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,8 @@ impl<'a, Value: Deserialize<'a> + Clone + 'a + JsonLike<'a> + std::fmt::Debug> J

fn plan(query: &str, variables: &Variables<async_graphql::Value>) -> OperationPlan<Value> {
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)
Expand Down
3 changes: 3 additions & 0 deletions src/core/jit/graphql_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ impl JITExecutor {
&self,
request: async_graphql::Request,
) -> impl Future<Output = AnyResponse<Vec<u8>>> + 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 {
Expand Down Expand Up @@ -135,6 +137,7 @@ impl JITExecutor {
}
}

// TODO: used only for introspection, simplify somehow?
impl From<jit::Request<Value>> for async_graphql::Request {
fn from(value: jit::Request<Value>) -> Self {
let mut request = async_graphql::Request::new(value.query);
Expand Down
2 changes: 1 addition & 1 deletion src/core/jit/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl Request<ConstValue> {
blueprint: &Blueprint,
) -> Result<OperationPlan<async_graphql_value::Value>> {
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()
Expand Down
2 changes: 1 addition & 1 deletion src/core/jit/synth/synth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
6 changes: 1 addition & 5 deletions tests/core/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,7 @@ impl ExecutionSpec {
env: HashMap<String, String>,
http: Arc<Http>,
) -> Arc<AppContext> {
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();

Expand Down
2 changes: 1 addition & 1 deletion tests/core/snapshots/test-enable-jit.md_merged.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion tests/core/snapshots/test-required-fields.md_merged.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source: tests/core/spec.rs
expression: formatter
snapshot_kind: text
---
schema @server(enableJIT: true) @upstream {
schema @server @upstream {
query: Query
}

Expand Down
Loading