Skip to content

Commit

Permalink
refactor: simplify Eval trait (#2207)
Browse files Browse the repository at this point in the history
  • Loading branch information
meskill authored Jun 15, 2024
1 parent 2a49da9 commit 6e89264
Show file tree
Hide file tree
Showing 13 changed files with 224 additions and 230 deletions.
12 changes: 6 additions & 6 deletions benches/impl_path_string_for_evaluation_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,24 +174,24 @@ fn to_bench_id(input: &[&str]) -> BenchmarkId {
#[derive(Clone)]
struct MockGraphqlContext;

impl<'a> ResolverContextLike<'a> for MockGraphqlContext {
fn value(&'a self) -> Option<&'a Value> {
impl ResolverContextLike for MockGraphqlContext {
fn value(&self) -> Option<&Value> {
Some(&TEST_VALUES)
}

fn args(&'a self) -> Option<&'a IndexMap<Name, Value>> {
fn args(&self) -> Option<&IndexMap<Name, Value>> {
Some(&TEST_ARGS)
}

fn field(&'a self) -> Option<SelectionField> {
fn field(&self) -> Option<SelectionField> {
None
}

fn is_query(&'a self) -> bool {
fn is_query(&self) -> bool {
false
}

fn add_error(&'a self, _: async_graphql::ServerError) {}
fn add_error(&self, _: async_graphql::ServerError) {}
}

// assert that everything was set up correctly for the benchmark
Expand Down
8 changes: 5 additions & 3 deletions src/core/blueprint/into_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,12 @@ fn to_type(def: &Definition) -> dynamic::Type {
FieldFuture::new(
async move {
let ctx: ResolverContext = ctx.into();
let ctx = EvaluationContext::new(req_ctx, &ctx);
let mut ctx = EvaluationContext::new(req_ctx, &ctx);

let const_value =
expr.eval(ctx).await.map_err(|err| err.extend())?;
let const_value = expr
.eval(&mut ctx)
.await
.map_err(|err| err.extend())?;
let p = match const_value {
ConstValue::List(a) => Some(FieldValue::list(a)),
ConstValue::Null => FieldValue::NONE,
Expand Down
2 changes: 1 addition & 1 deletion src/core/has_headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub trait HasHeaders {
fn headers(&self) -> &HeaderMap;
}

impl<'a, Ctx: ResolverContextLike<'a>> HasHeaders for EvaluationContext<'a, Ctx> {
impl<'a, Ctx: ResolverContextLike> HasHeaders for EvaluationContext<'a, Ctx> {
fn headers(&self) -> &HeaderMap {
self.headers()
}
Expand Down
47 changes: 23 additions & 24 deletions src/core/ir/cache.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use core::future::Future;
use std::num::NonZeroU64;
use std::ops::Deref;
use std::pin::Pin;

use async_graphql_value::ConstValue;

Expand Down Expand Up @@ -42,31 +40,32 @@ impl Cache {
}

impl Eval for Cache {
fn eval<'a, Ctx: ResolverContextLike<'a> + Sync + Send>(
&'a self,
ctx: EvaluationContext<'a, Ctx>,
) -> Pin<Box<dyn Future<Output = Result<ConstValue, EvaluationError>> + 'a + Send>> {
Box::pin(async move {
if let IR::IO(io) = self.expr.deref() {
let key = io.cache_key(&ctx);
if let Some(key) = key {
if let Some(val) = ctx.request_ctx.runtime.cache.get(&key).await? {
Ok(val)
} else {
let val = self.expr.eval(ctx.clone()).await?;
ctx.request_ctx
.runtime
.cache
.set(key, val.clone(), self.max_age)
.await?;
Ok(val)
}
async fn eval<Ctx>(
&self,
ctx: &mut EvaluationContext<'_, Ctx>,
) -> Result<ConstValue, EvaluationError>
where
Ctx: ResolverContextLike + Sync,
{
if let IR::IO(io) = self.expr.deref() {
let key = io.cache_key(ctx);
if let Some(key) = key {
if let Some(val) = ctx.request_ctx.runtime.cache.get(&key).await? {
Ok(val)
} else {
self.expr.eval(ctx).await
let val = self.expr.eval(ctx).await?;
ctx.request_ctx
.runtime
.cache
.set(key, val.clone(), self.max_age)
.await?;
Ok(val)
}
} else {
Ok(self.expr.eval(ctx).await?)
self.expr.eval(ctx).await
}
})
} else {
Ok(self.expr.eval(ctx).await?)
}
}
}
18 changes: 7 additions & 11 deletions src/core/ir/eval.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
use core::future::Future;
use std::pin::Pin;
use std::future::Future;

use super::{EvaluationContext, EvaluationError, ResolverContextLike};

pub trait Eval<Output = async_graphql::Value>
where
Self: Send + Sync,
{
fn eval<'a, Ctx: ResolverContextLike<'a> + Sync + Send>(
&'a self,
ctx: EvaluationContext<'a, Ctx>,
) -> Pin<Box<dyn Future<Output = Result<Output, EvaluationError>> + 'a + Send>>
pub trait Eval<Output = async_graphql::Value> {
fn eval<Ctx>(
&self,
ctx: &mut EvaluationContext<'_, Ctx>,
) -> impl Future<Output = Result<Output, EvaluationError>>
where
Output: 'a;
Ctx: ResolverContextLike + Sync;
}
8 changes: 4 additions & 4 deletions src/core/ir/evaluation_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::core::http::RequestContext;

// TODO: rename to ResolverContext
#[derive(Clone)]
pub struct EvaluationContext<'a, Ctx: ResolverContextLike<'a>> {
pub struct EvaluationContext<'a, Ctx: ResolverContextLike> {
// Context create for each GraphQL Request
pub request_ctx: &'a RequestContext,

Expand All @@ -25,7 +25,7 @@ pub struct EvaluationContext<'a, Ctx: ResolverContextLike<'a>> {
graphql_ctx_args: Option<Arc<Value>>,
}

impl<'a, A: ResolverContextLike<'a>> EvaluationContext<'a, A> {
impl<'a, A: ResolverContextLike> EvaluationContext<'a, A> {
pub fn with_value(&self, value: Value) -> EvaluationContext<'a, A> {
let mut ctx = self.clone();
ctx.graphql_ctx_value = Some(Arc::new(value));
Expand All @@ -43,7 +43,7 @@ impl<'a, A: ResolverContextLike<'a>> EvaluationContext<'a, A> {
}
}

impl<'a, Ctx: ResolverContextLike<'a>> EvaluationContext<'a, Ctx> {
impl<'a, Ctx: ResolverContextLike> EvaluationContext<'a, Ctx> {
pub fn new(req_ctx: &'a RequestContext, graphql_ctx: &'a Ctx) -> EvaluationContext<'a, Ctx> {
Self {
request_ctx: req_ctx,
Expand Down Expand Up @@ -109,7 +109,7 @@ impl<'a, Ctx: ResolverContextLike<'a>> EvaluationContext<'a, Ctx> {
}
}

impl<'a, Ctx: ResolverContextLike<'a>> GraphQLOperationContext for EvaluationContext<'a, Ctx> {
impl<'a, Ctx: ResolverContextLike> GraphQLOperationContext for EvaluationContext<'a, Ctx> {
fn selection_set(&self) -> Option<String> {
let selection_set = self.graphql_ctx.field()?.selection_set();

Expand Down
Loading

1 comment on commit 6e89264

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running 30s test @ http://localhost:8000/graphql

4 threads and 100 connections

Thread Stats Avg Stdev Max +/- Stdev
Latency 6.62ms 3.35ms 132.29ms 76.37%
Req/Sec 3.83k 339.01 14.71k 98.08%

457498 requests in 30.10s, 2.29GB read

Requests/sec: 15199.42

Transfer/sec: 78.01MB

Please sign in to comment.