Skip to content

Commit

Permalink
Extend multiple schemas in parallel
Browse files Browse the repository at this point in the history
Summary:
Instead of generating/validating schemas 1-by-1, let's generate them simultaneously. If we do it fast enough, we can replace WWW's parallelism with Rust's, which may in theory be faster.

I had to remove parallelism from `schema-validate` as it was making the whole thing ~7 seconds slower. Tried experimenting with limiting the concurrency and got the same results - it works best when validation is not parallelized at all.

 Removing parallelization in schema-validate which turns out to hurt performance.

Reviewed By: gary-z

Differential Revision: D47136978

fbshipit-source-id: e822e9b41dd02b4f00673e6f66aca66d3caa5d32
  • Loading branch information
Arseniy Panfilov authored and facebook-github-bot committed Jul 6, 2023
1 parent 3c82069 commit a062e17
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 34 deletions.
1 change: 0 additions & 1 deletion compiler/crates/schema-validate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ fnv = "1.0"
intern = { path = "../intern" }
lazy_static = "1.4"
log = { version = "0.4.17", features = ["kv_unstable", "kv_unstable_std"] }
rayon = "1.2"
regex = "1.6.0"
schema = { path = "../schema" }
schema-print = { path = "../schema-print" }
Expand Down
57 changes: 25 additions & 32 deletions compiler/crates/schema-validate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
mod errors;

use std::fmt::Write;
use std::sync::Mutex;
use std::time::Instant;

use common::DirectiveName;
Expand All @@ -22,7 +21,6 @@ use intern::string_key::StringKey;
use intern::Lookup;
use lazy_static::lazy_static;
use log::info;
use rayon::prelude::*;
use regex::Regex;
use schema::EnumID;
use schema::Field;
Expand Down Expand Up @@ -82,14 +80,14 @@ impl ValidationContextType {

pub struct ValidationContext<'schema> {
pub schema: &'schema SDLSchema,
pub errors: Mutex<FnvHashMap<ValidationContextType, Vec<SchemaValidationError>>>,
pub errors: FnvHashMap<ValidationContextType, Vec<SchemaValidationError>>,
}

impl<'schema> ValidationContext<'schema> {
pub fn new(schema: &'schema SDLSchema) -> Self {
Self {
schema,
errors: Mutex::new(FnvHashMap::default()),
errors: FnvHashMap::default(),
}
}

Expand All @@ -99,10 +97,7 @@ impl<'schema> ValidationContext<'schema> {
self.validate_directives();
self.validate_types();
info!("Validated Schema in {}ms", now.elapsed().as_millis());
info!(
"Found {} validation errors",
self.errors.lock().unwrap().len()
)
info!("Found {} validation errors", self.errors.len())
}

fn validate_root_types(&mut self) {
Expand All @@ -111,7 +106,7 @@ impl<'schema> ValidationContext<'schema> {
self.validate_root_type(self.schema.mutation_type(), *MUTATION);
}

fn validate_root_type(&self, root_type: Option<Type>, type_name: StringKey) {
fn validate_root_type(&mut self, root_type: Option<Type>, type_name: StringKey) {
if let Some(type_) = root_type {
if !type_.is_object() {
self.report_error(
Expand Down Expand Up @@ -146,11 +141,11 @@ impl<'schema> ValidationContext<'schema> {
}

fn validate_types(&mut self) {
let types = self.schema.get_type_map().collect::<Vec<_>>();
types.par_iter().for_each(|(type_name, type_)| {
let types = self.schema.get_type_map();
for (type_name, type_) in types {
// Ensure it is named correctly (excluding introspection types).
if !is_introspection_type(type_, **type_name) {
self.validate_name(**type_name, ValidationContextType::TypeNode(**type_name));
if !is_introspection_type(type_, *type_name) {
self.validate_name(*type_name, ValidationContextType::TypeNode(*type_name));
}
match type_ {
Type::Enum(id) => {
Expand All @@ -164,7 +159,7 @@ impl<'schema> ValidationContext<'schema> {
Type::Interface(id) => {
let interface = self.schema.interface(*id);
// Ensure fields are valid
self.validate_fields(**type_name, &interface.fields);
self.validate_fields(*type_name, &interface.fields);

// Validate cyclic references
if !self.validate_cyclic_implements_reference(interface) {
Expand All @@ -175,7 +170,7 @@ impl<'schema> ValidationContext<'schema> {
Type::Object(id) => {
let object = self.schema.object(*id);
// Ensure fields are valid
self.validate_fields(**type_name, &object.fields);
self.validate_fields(*type_name, &object.fields);

// Ensure objects implement the interfaces they claim to.
self.validate_type_with_interfaces(object);
Expand All @@ -186,10 +181,10 @@ impl<'schema> ValidationContext<'schema> {
}
Type::Scalar(_id) => {}
};
});
}
}

fn validate_fields(&self, type_name: StringKey, fields: &[FieldID]) {
fn validate_fields(&mut self, type_name: StringKey, fields: &[FieldID]) {
let context = ValidationContextType::TypeNode(type_name);
// Must define one or more fields.
if fields.is_empty() {
Expand Down Expand Up @@ -255,7 +250,7 @@ impl<'schema> ValidationContext<'schema> {
}
}

fn validate_union_members(&self, id: UnionID) {
fn validate_union_members(&mut self, id: UnionID) {
let union = self.schema.union(id);
let context = ValidationContextType::TypeNode(union.name.item);
if union.members.is_empty() {
Expand All @@ -276,7 +271,7 @@ impl<'schema> ValidationContext<'schema> {
}
}

fn validate_enum_type(&self, id: EnumID) {
fn validate_enum_type(&mut self, id: EnumID) {
let enum_ = self.schema.enum_(id);
let context = ValidationContextType::TypeNode(enum_.name.item.0);
if enum_.values.is_empty() {
Expand All @@ -296,7 +291,7 @@ impl<'schema> ValidationContext<'schema> {
}
}

fn validate_input_object_fields(&self, id: InputObjectID) {
fn validate_input_object_fields(&mut self, id: InputObjectID) {
let input_object = self.schema.input_object(id);
let context = ValidationContextType::TypeNode(input_object.name.item.0);
if input_object.fields.is_empty() {
Expand All @@ -323,7 +318,7 @@ impl<'schema> ValidationContext<'schema> {
}
}

fn validate_type_with_interfaces<T: TypeWithFields + Named>(&self, type_: &T) {
fn validate_type_with_interfaces<T: TypeWithFields + Named>(&mut self, type_: &T) {
let typename = type_.name().lookup().intern();
let mut interface_names = FnvHashSet::default();
for interface_id in type_.interfaces().iter() {
Expand All @@ -344,7 +339,7 @@ impl<'schema> ValidationContext<'schema> {
}

fn validate_type_implements_interface<T: TypeWithFields + Named>(
&self,
&mut self,
type_: &T,
interface: &Interface,
) {
Expand Down Expand Up @@ -447,7 +442,7 @@ impl<'schema> ValidationContext<'schema> {
}
}

fn validate_cyclic_implements_reference(&self, interface: &Interface) -> bool {
fn validate_cyclic_implements_reference(&mut self, interface: &Interface) -> bool {
for id in interface.interfaces() {
let mut path = Vec::new();
let mut visited = FnvHashSet::default();
Expand Down Expand Up @@ -500,7 +495,7 @@ impl<'schema> ValidationContext<'schema> {
false
}

fn validate_name(&self, name: StringKey, context: ValidationContextType) {
fn validate_name(&mut self, name: StringKey, context: ValidationContextType) {
let name = name.lookup();
let mut chars = name.chars();
if name.len() > 1 && chars.next() == Some('_') && chars.next() == Some('_') {
Expand All @@ -517,31 +512,28 @@ impl<'schema> ValidationContext<'schema> {
}
}

fn field_map(&self, fields: &[FieldID]) -> FnvHashMap<StringKey, Field> {
fn field_map(&mut self, fields: &[FieldID]) -> FnvHashMap<StringKey, Field> {
fields
.iter()
.map(|id| self.schema.field(*id))
.map(|field| (field.name.item, field.clone()))
.collect::<FnvHashMap<_, _>>()
}

fn report_error(&self, error: SchemaValidationError, context: ValidationContextType) {
fn report_error(&mut self, error: SchemaValidationError, context: ValidationContextType) {
self.errors
.lock()
.unwrap()
.entry(context)
.or_insert_with(Vec::new)
.push(error);
}

fn add_error(&self, error: SchemaValidationError) {
fn add_error(&mut self, error: SchemaValidationError) {
self.report_error(error, ValidationContextType::None);
}

pub fn print_errors(&self) -> String {
let mut builder: String = String::new();
let errors = self.errors.lock().unwrap();
let mut contexts: Vec<_> = errors.keys().collect();
let mut contexts: Vec<_> = self.errors.keys().collect();
contexts.sort_by_key(|context| context.type_name());
for context in contexts {
match context {
Expand All @@ -567,7 +559,8 @@ impl<'schema> ValidationContext<'schema> {
)
.unwrap(),
}
let mut error_strings = errors
let mut error_strings = self
.errors
.get(context)
.unwrap()
.iter()
Expand Down
2 changes: 1 addition & 1 deletion compiler/crates/schema-validate/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub fn main() {
match build_schema_from_file(&opt.schema_path) {
Ok(schema) => {
let validation_context = validate(&schema);
if !validation_context.errors.lock().unwrap().is_empty() {
if !validation_context.errors.is_empty() {
eprintln!(
"Schema failed validation with below errors:\n{}",
validation_context.print_errors()
Expand Down

0 comments on commit a062e17

Please sign in to comment.