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

fix(compiler): cache the implementers map per executable document #863

Merged
merged 3 commits into from
Jun 5, 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
5 changes: 5 additions & 0 deletions crates/apollo-compiler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ name = "fields-validation"
path = "benches/fields_validation.rs"
harness = false

[[bench]]
name = "fragments-validation"
path = "benches/fragments_validation.rs"
harness = false

[[test]]
name = "main"

Expand Down
51 changes: 51 additions & 0 deletions crates/apollo-compiler/benches/fragments_validation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use apollo_compiler::ExecutableDocument;
use apollo_compiler::Schema;
use criterion::*;
use std::fmt::Write;

fn bench_big_schema_many_fragments(c: &mut Criterion) {
const NUM_INTERFACES: usize = 200;
const NUM_OBJECTS: usize = 10_000;

let mut sdl = String::new();
for i in 0..NUM_INTERFACES {
_ = writeln!(&mut sdl, r#"interface Intf{i} {{ field: Int! }}"#);
}
for o in 0..NUM_OBJECTS {
let i = o % NUM_INTERFACES;
_ = writeln!(
&mut sdl,
r#"type Ty{o} implements Intf{i} {{ field: Int! }}"#
);
}

_ = writeln!(&mut sdl, "type Query {{");
for i in 0..NUM_INTERFACES {
_ = writeln!(&mut sdl, " intf{i}: Intf{i}");
}
_ = writeln!(&mut sdl, "}}");

let schema = Schema::parse_and_validate(sdl, "schema.graphql").unwrap();
let mut selection = String::new();
let mut fragments = String::new();
for i in 0..NUM_INTERFACES {
_ = writeln!(&mut selection, " intf{i} {{ ...frag{i} }}");
_ = writeln!(&mut fragments, "fragment frag{i} on Ty{i} {{ field }}");
}
let query = format!(
"query {{
{selection}}}
{fragments}"
);

c.bench_function("big_schema_many_fragments", move |b| {
b.iter(|| {
let doc =
ExecutableDocument::parse_and_validate(&schema, &query, "query.graphql").unwrap();
black_box(doc);
});
});
}

criterion_group!(fragments, bench_big_schema_many_fragments,);
criterion_main!(fragments);
10 changes: 5 additions & 5 deletions crates/apollo-compiler/src/executable/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::validation::fragment::validate_fragment_used;
use crate::validation::operation::validate_operation_definitions;
use crate::validation::selection::FieldsInSetCanMerge;
use crate::validation::DiagnosticList;
use crate::validation::ExecutableValidationContext;
use crate::validation::Valid;
use crate::ExecutableDocument;
use crate::Schema;
Expand Down Expand Up @@ -40,7 +41,8 @@ pub(crate) fn validate_with_or_without_schema(
schema: Option<&Schema>,
document: &ExecutableDocument,
) {
validate_operation_definitions(errors, schema, document);
let context = ExecutableValidationContext::new(schema);
validate_operation_definitions(errors, document, &context);
for def in document.fragments.values() {
validate_fragment_used(errors, document, def);
}
Expand All @@ -52,14 +54,12 @@ pub(crate) fn validate_field_set(
field_set: &FieldSet,
) {
let document = &ExecutableDocument::new(); // No fragment definitions
let context = ExecutableValidationContext::new(Some(schema));
crate::validation::selection::validate_selection_set(
diagnostics,
document,
Some((schema, &field_set.selection_set.ty)),
&field_set.selection_set,
crate::validation::operation::OperationValidationConfig {
schema: Some(schema),
variables: &[],
},
context.operation_context(&[]),
)
}
6 changes: 3 additions & 3 deletions crates/apollo-compiler/src/validation/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use crate::coordinate::{FieldArgumentCoordinate, TypeAttributeCoordinate};
use crate::validation::diagnostics::DiagnosticData;
use crate::{ast, executable, schema, ExecutableDocument, Node};

use super::operation::OperationValidationConfig;
use crate::ast::Name;
use crate::schema::Component;
use crate::validation::DiagnosticList;
use crate::validation::OperationValidationContext;
use indexmap::IndexMap;

pub(crate) fn validate_field(
Expand All @@ -14,13 +14,13 @@ pub(crate) fn validate_field(
// May be None if a parent selection was invalid
against_type: Option<(&crate::Schema, &ast::NamedType)>,
field: &Node<executable::Field>,
context: OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
// First do all the validation that we can without knowing the type of the field.

super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
field.directives.iter(),
ast::DirectiveLocation::Field,
context.variables,
Expand Down
31 changes: 17 additions & 14 deletions crates/apollo-compiler/src/validation/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::executable;
use crate::schema;
use crate::schema::Implementers;
use crate::validation::diagnostics::DiagnosticData;
use crate::validation::operation::OperationValidationConfig;
use crate::validation::DiagnosticList;
use crate::validation::OperationValidationContext;
use crate::validation::{CycleError, NodeLocation, RecursionGuard, RecursionStack};
use crate::ExecutableDocument;
use crate::Node;
Expand Down Expand Up @@ -52,6 +52,7 @@ fn validate_fragment_spread_type(
against_type: &NamedType,
type_condition: &NamedType,
selection: &executable::Selection,
context: OperationValidationContext<'_>,
) {
// Another diagnostic will be raised if the type condition was wrong.
// We reduce noise by silencing other issues with the fragment.
Expand All @@ -64,9 +65,9 @@ fn validate_fragment_spread_type(
return;
};

let implementers_map = schema.implementers_map();
let concrete_parent_types = get_possible_types(against_type_definition, &implementers_map);
let concrete_condition_types = get_possible_types(type_condition_definition, &implementers_map);
let implementers_map = context.implementers_map();
let concrete_parent_types = get_possible_types(against_type_definition, implementers_map);
let concrete_condition_types = get_possible_types(type_condition_definition, implementers_map);

let mut applicable_types = concrete_parent_types.intersection(&concrete_condition_types);
if applicable_types.next().is_none() {
Expand Down Expand Up @@ -107,18 +108,18 @@ pub(crate) fn validate_inline_fragment(
document: &ExecutableDocument,
against_type: Option<(&crate::Schema, &ast::NamedType)>,
inline: &Node<executable::InlineFragment>,
context: OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
inline.directives.iter(),
ast::DirectiveLocation::InlineFragment,
context.variables,
);

let previous = diagnostics.len();
if let Some(schema) = context.schema {
if let Some(schema) = context.schema() {
if let Some(t) = &inline.type_condition {
validate_fragment_type_condition(diagnostics, schema, None, t, inline.location())
}
Expand All @@ -138,12 +139,13 @@ pub(crate) fn validate_inline_fragment(
against_type,
type_condition,
&executable::Selection::InlineFragment(inline.clone()),
context,
);
}
super::selection::validate_selection_set(
diagnostics,
document,
if let (Some(schema), Some(ty)) = (&context.schema, &inline.type_condition) {
if let (Some(schema), Some(ty)) = (&context.schema(), &inline.type_condition) {
Some((schema, ty))
} else {
against_type
Expand All @@ -159,11 +161,11 @@ pub(crate) fn validate_fragment_spread(
document: &ExecutableDocument,
against_type: Option<(&crate::Schema, &NamedType)>,
spread: &Node<executable::FragmentSpread>,
context: OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
spread.directives.iter(),
ast::DirectiveLocation::FragmentSpread,
context.variables,
Expand All @@ -179,6 +181,7 @@ pub(crate) fn validate_fragment_spread(
against_type,
def.type_condition(),
&executable::Selection::FragmentSpread(spread.clone()),
context,
);
}
validate_fragment_definition(diagnostics, document, def, context);
Expand All @@ -198,18 +201,18 @@ pub(crate) fn validate_fragment_definition(
diagnostics: &mut DiagnosticList,
document: &ExecutableDocument,
fragment: &Node<executable::Fragment>,
context: OperationValidationConfig<'_>,
context: OperationValidationContext<'_>,
) {
super::directive::validate_directives(
diagnostics,
context.schema,
context.schema(),
fragment.directives.iter(),
ast::DirectiveLocation::FragmentDefinition,
context.variables,
);

let previous = diagnostics.len();
if let Some(schema) = context.schema {
if let Some(schema) = context.schema() {
validate_fragment_type_condition(
diagnostics,
schema,
Expand All @@ -228,7 +231,7 @@ pub(crate) fn validate_fragment_definition(
// If the type does not exist, do not attempt to validate the selections against it;
// it has either already raised an error, or we are validating an executable without
// a schema.
let type_condition = context.schema.and_then(|schema| {
let type_condition = context.schema().and_then(|schema| {
schema
.types
.contains_key(fragment.type_condition())
Expand Down
72 changes: 70 additions & 2 deletions crates/apollo-compiler/src/validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
use crate::coordinate::SchemaCoordinate;
#[cfg(doc)]
use crate::{ExecutableDocument, Schema};
use crate::ExecutableDocument;
use crate::Schema;

pub(crate) mod argument;
pub(crate) mod diagnostics;
Expand All @@ -25,13 +26,16 @@ pub(crate) mod variable;
use crate::ast::Name;
use crate::diagnostic::{CliReport, Diagnostic, ToCliReport};
use crate::executable::BuildError as ExecutableBuildError;
use crate::executable::VariableDefinition;
use crate::execution::{GraphQLError, Response};
use crate::schema::BuildError as SchemaBuildError;
use crate::schema::Implementers;
use crate::SourceMap;
use crate::{Node, NodeLocation};
use indexmap::IndexSet;
use std::collections::HashMap;
use std::fmt;
use std::sync::Arc;
use std::sync::{Arc, OnceLock};

pub(crate) use crate::node::FileId;

Expand Down Expand Up @@ -112,6 +116,70 @@ impl<T: fmt::Display> fmt::Display for Valid<T> {
}
}

/// Shared context with things that may be used throughout executable validation.
#[derive(Debug)]
pub(crate) struct ExecutableValidationContext<'a> {
/// When None, rules that require a schema to validate are disabled.
schema: Option<&'a Schema>,
/// `schema.implementers_map()` is expensive to compute. This caches it for reuse.
implementers_map: OnceLock<HashMap<Name, Implementers>>,
}

impl<'a> ExecutableValidationContext<'a> {
pub fn new(schema: Option<&'a Schema>) -> Self {
Self {
schema,
implementers_map: Default::default(),
}
}

/// Returns the schema to validate against, if any.
pub fn schema(&self) -> Option<&'a Schema> {
self.schema
}

/// Returns a cached reference to the implementers map.
pub fn implementers_map(&self) -> &HashMap<Name, Implementers> {
self.implementers_map.get_or_init(|| {
self.schema
.map(|schema| schema.implementers_map())
.unwrap_or_default()
})
}

/// Returns a context for operation validation.
pub fn operation_context<'o>(
&'o self,
variables: &'o [Node<VariableDefinition>],
) -> OperationValidationContext<'o> {
OperationValidationContext {
executable: self,
variables,
}
}
}

/// Shared context when validating things inside an operation.
#[derive(Debug, Clone, Copy)]
pub(crate) struct OperationValidationContext<'a> {
/// Parent context. Using a reference so the `OnceLock` is shared between all operation
/// contexts.
executable: &'a ExecutableValidationContext<'a>,
/// The variables defined for this operation.
pub variables: &'a [Node<VariableDefinition>],
}

impl<'a> OperationValidationContext<'a> {
pub fn schema(&self) -> Option<&'a Schema> {
self.executable.schema
}

/// Returns a cached reference to the implementers map.
pub fn implementers_map(&self) -> &HashMap<Name, Implementers> {
self.executable.implementers_map()
}
}

/// A conversion failed with some errors, but also resulted in a partial document.
///
/// The [`Debug`][fmt::Debug] trait is implemented by forwarding to [`Self::errors`] and
Expand Down
Loading