From a062e17047d6114e0768d5ef7d00b61aaf69fc77 Mon Sep 17 00:00:00 2001 From: Arseniy Panfilov Date: Thu, 6 Jul 2023 11:21:14 -0700 Subject: [PATCH] Extend multiple schemas in parallel 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 --- compiler/crates/schema-validate/Cargo.toml | 1 - compiler/crates/schema-validate/src/lib.rs | 57 +++++++++------------ compiler/crates/schema-validate/src/main.rs | 2 +- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/compiler/crates/schema-validate/Cargo.toml b/compiler/crates/schema-validate/Cargo.toml index 0c46cf97d310f..56bc7300fad36 100644 --- a/compiler/crates/schema-validate/Cargo.toml +++ b/compiler/crates/schema-validate/Cargo.toml @@ -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" } diff --git a/compiler/crates/schema-validate/src/lib.rs b/compiler/crates/schema-validate/src/lib.rs index 540473cf2c01f..98f452f379a9b 100644 --- a/compiler/crates/schema-validate/src/lib.rs +++ b/compiler/crates/schema-validate/src/lib.rs @@ -8,7 +8,6 @@ mod errors; use std::fmt::Write; -use std::sync::Mutex; use std::time::Instant; use common::DirectiveName; @@ -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; @@ -82,14 +80,14 @@ impl ValidationContextType { pub struct ValidationContext<'schema> { pub schema: &'schema SDLSchema, - pub errors: Mutex>>, + pub errors: FnvHashMap>, } impl<'schema> ValidationContext<'schema> { pub fn new(schema: &'schema SDLSchema) -> Self { Self { schema, - errors: Mutex::new(FnvHashMap::default()), + errors: FnvHashMap::default(), } } @@ -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) { @@ -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_name: StringKey) { + fn validate_root_type(&mut self, root_type: Option, type_name: StringKey) { if let Some(type_) = root_type { if !type_.is_object() { self.report_error( @@ -146,11 +141,11 @@ impl<'schema> ValidationContext<'schema> { } fn validate_types(&mut self) { - let types = self.schema.get_type_map().collect::>(); - 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) => { @@ -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) { @@ -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); @@ -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() { @@ -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() { @@ -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() { @@ -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() { @@ -323,7 +318,7 @@ impl<'schema> ValidationContext<'schema> { } } - fn validate_type_with_interfaces(&self, type_: &T) { + fn validate_type_with_interfaces(&mut self, type_: &T) { let typename = type_.name().lookup().intern(); let mut interface_names = FnvHashSet::default(); for interface_id in type_.interfaces().iter() { @@ -344,7 +339,7 @@ impl<'schema> ValidationContext<'schema> { } fn validate_type_implements_interface( - &self, + &mut self, type_: &T, interface: &Interface, ) { @@ -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(); @@ -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('_') { @@ -517,7 +512,7 @@ impl<'schema> ValidationContext<'schema> { } } - fn field_map(&self, fields: &[FieldID]) -> FnvHashMap { + fn field_map(&mut self, fields: &[FieldID]) -> FnvHashMap { fields .iter() .map(|id| self.schema.field(*id)) @@ -525,23 +520,20 @@ impl<'schema> ValidationContext<'schema> { .collect::>() } - 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 { @@ -567,7 +559,8 @@ impl<'schema> ValidationContext<'schema> { ) .unwrap(), } - let mut error_strings = errors + let mut error_strings = self + .errors .get(context) .unwrap() .iter() diff --git a/compiler/crates/schema-validate/src/main.rs b/compiler/crates/schema-validate/src/main.rs index f2ea82703ea24..2e38eb5db42ec 100644 --- a/compiler/crates/schema-validate/src/main.rs +++ b/compiler/crates/schema-validate/src/main.rs @@ -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()