Skip to content

Merge cached validation #133

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.

Sign up for GitHub

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

Merged
wravery merged 19 commits into from
Dec 14, 2020
Merged

Merge cached validation #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f7b7ed8
Add a benchmark based on sample
wravery Dec 10, 2020
3956732
schemagen allow introspection to be disabled
barbieri Dec 10, 2020
91249d5
Skip validation for now without Introspection
wravery Dec 11, 2020
4bf0b78
Generate a compact schema
wravery Dec 11, 2020
88ed5c6
Remove uneeded forward declares
wravery Dec 11, 2020
8eaf8c2
Make a weak singleton GetSchema function
wravery Dec 11, 2020
3dd30f0
Not passing tests yet, but refactor is done
wravery Dec 11, 2020
bf1221c
Fixed the rest of the tests
wravery Dec 11, 2020
f9d584c
Split out graphqlservice_nointrospection
wravery Dec 12, 2020
5fb0f42
Export schema accessors for introspection in separate DLL
wravery Dec 12, 2020
b8fdb0f
Cherry-pick new unit test to pin behavior
barbieri Dec 10, 2020
5e1a4ff
Optimize response::Value to avoid memory copies
wravery Dec 12, 2020
64f2dae
Merge branch 'merge-cached-validation' of https://github.com/wravery/…
wravery Dec 12, 2020
2586c89
Fix order of template specialization defintions
wravery Dec 12, 2020
9184e91
optimize Request::findOperationDefinition()
wravery Dec 13, 2020
b1b734b
Only call validate from findOperationDefinition
wravery Dec 13, 2020
fde76cb
Store results in ResolverResult until final return
wravery Dec 13, 2020
d0c8ed4
Skip descriptions for --no-introspection
wravery Dec 13, 2020
b88ba8e
Map directly to types instead of names
wravery Dec 14, 2020
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
6 changes: 5 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Debug/
Release/
Testing/
Win32/
/include/graphqlservice/IntrospectionSchema.h
/include/graphqlservice/introspection/IntrospectionSchema.h
/IntrospectionSchema.cpp
*.filters
*.vcxproj
Expand All @@ -27,12 +27,16 @@ Makefile
.ninja_*
schemagen
settings.json
/samples/benchmark
/samples/benchmark_nointrospection
/samples/sample
/samples/sample_nointrospection
/src/cmake/
/test/argument_tests
/test/pegtl_tests
/test/response_tests
/test/today_tests
/test/nointrospection_tests
build/
install/
isenseconfig/
4 changes: 2 additions & 2 deletions doc/subscriptions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ tell which operation type it is without parsing the query and searching for
a specific operation name, so it's hard to tell whether you should call
`resolve` or `subscribe` when the request is received that way. To help with
that, there's a public `Request::findOperationDefinition` method which returns
the operation type as a `std::string` along with a pointer to the AST node for
the operation type as a `std::string_view` along with a pointer to the AST node for
the selected operation in the parsed query:
```cpp
std::pair<std::string, const peg::ast_node*> findOperationDefinition(const peg::ast_node& root, const std::string& operationName) const;
std::pair<std::string_view, const peg::ast_node*> findOperationDefinition(peg::ast& root, std::string_view operationName) const;
```
3 changes: 2 additions & 1 deletion include/SchemaGenerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ struct GeneratorOptions
const bool verbose = false;
const bool separateFiles = false;
const bool noStubs = false;
const bool noIntrospection = false;
};

// RAII object to help with emitting matching include guard begin and end statements
Expand Down Expand Up @@ -280,7 +281,7 @@ class PendingBlankLine
bool reset() noexcept;

private:
bool _pending = false;
bool _pending = true;
std::ostream& _outputFile;
};

Expand Down
91 changes: 46 additions & 45 deletions include/Validation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@
#ifndef VALIDATION_H
#define VALIDATION_H

#include "graphqlservice/GraphQLSchema.h"
#include "graphqlservice/GraphQLService.h"
#include "graphqlservice/IntrospectionSchema.h"

namespace graphql::service {

using ValidateType = response::Value;
using ValidateType = std::optional<std::reference_wrapper<const schema::BaseType>>;
using SharedType = std::shared_ptr<const schema::BaseType>;

SharedType getSharedType(const ValidateType& type) noexcept;
ValidateType getValidateType(const SharedType& type) noexcept;

struct ValidateArgument
{
Expand All @@ -20,15 +24,15 @@ struct ValidateArgument
ValidateType type;
};

using ValidateTypeFieldArguments = std::map<std::string, ValidateArgument>;
using ValidateTypeFieldArguments = std::map<std::string_view, ValidateArgument>;

struct ValidateTypeField
{
ValidateType returnType;
ValidateTypeFieldArguments arguments;
};

using ValidateDirectiveArguments = std::map<std::string, ValidateArgument>;
using ValidateDirectiveArguments = std::map<std::string_view, ValidateArgument>;

struct ValidateDirective
{
Expand All @@ -40,14 +44,14 @@ struct ValidateArgumentVariable
{
bool operator==(const ValidateArgumentVariable& other) const;

std::string name;
std::string_view name;
};

struct ValidateArgumentEnumValue
{
bool operator==(const ValidateArgumentEnumValue& other) const;

std::string value;
std::string_view value;
};

struct ValidateArgumentValue;
Expand All @@ -71,7 +75,7 @@ struct ValidateArgumentMap
{
bool operator==(const ValidateArgumentMap& other) const;

std::map<std::string, ValidateArgumentValuePtr> values;
std::map<std::string_view, ValidateArgumentValuePtr> values;
};

using ValidateArgumentVariant = std::variant<ValidateArgumentVariable, response::IntType,
Expand Down Expand Up @@ -118,29 +122,30 @@ class ValidateArgumentValueVisitor
std::vector<schema_error>& _errors;
};

using ValidateFieldArguments = std::map<std::string, ValidateArgumentValuePtr>;
using ValidateFieldArguments = std::map<std::string_view, ValidateArgumentValuePtr>;

struct ValidateField
{
ValidateField(std::string&& returnType, std::optional<std::string>&& objectType,
const std::string& fieldName, ValidateFieldArguments&& arguments);
ValidateField(ValidateType&& returnType, ValidateType&& objectType, std::string_view fieldName,
ValidateFieldArguments&& arguments);

bool operator==(const ValidateField& other) const;

std::string returnType;
std::optional<std::string> objectType;
std::string fieldName;
ValidateType returnType;
ValidateType objectType;
std::string_view fieldName;
ValidateFieldArguments arguments;
};

using ValidateTypeKinds = std::map<std::string, introspection::TypeKind>;
using ValidateTypes = std::map<std::string_view, ValidateType>;
Copy link
Contributor

@barbieri barbieri Dec 14, 2020

Choose a reason for hiding this comment

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

any reason to keep using map instead of unordered_map?

Copy link
Contributor Author

@wravery wravery Dec 15, 2020

Choose a reason for hiding this comment

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

I'm acutally thinking of replacing them all with sorted vectors: https://youtu.be/fHNmRkzxHWs?t=2818

Copy link
Contributor Author

@wravery wravery Dec 15, 2020

Choose a reason for hiding this comment

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

The key with vectors is to avoid reallocating on append by reserving or resizing to the exact size you need, and not inserting/removing other than at the end. But for most of these objects that should be doable, they're static for the lifetime of the service.

Copy link
Contributor

@barbieri barbieri Dec 16, 2020

Choose a reason for hiding this comment

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

if you're moving to sorted and looking into static... why not target array? There is a generator tool to help that, then we can try to get more stuff into .ro-data section of the binaries.

There is no need to get the vector helpers.

The binary search is not as fast as an unordered map, but I think it doesn't matter much for these use cases. If so we can look into integrating https://www.gnu.org/software/gperf/manual/gperf.html or an equivalent later.

Copy link
Contributor Author

@wravery wravery Dec 18, 2020

Choose a reason for hiding this comment

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

if you're moving to sorted and looking into static... why not target array? There is a generator tool to help that, then we can try to get more stuff into .ro-data section of the binaries.

Most of them still have a size which varies, so it couldn't be built into the graphqlservice library directly. The generated code would need to pass a span/range into graphqlservice to get dynamic sizing backed by std::array with a fixed size in the generated code.

C++20 adds span/range, so that would make sense whenever cppgraphqlgen moves to that language version (I'm tentatively planning on doing that for v4.0). There's also the gsl (C++ Core Guideline Support Library) which adds lookalike definitions of std::span<>, so that's another option that makes C++20 adoption easier down the road. But I'm reluctant to add another build dependency unless/until it's absolutely worth it.

The binary search is not as fast as an unordered map, but I think it doesn't matter much for these use cases. If so we can look into integrating https://www.gnu.org/software/gperf/manual/gperf.html or an equivalent later.

I'm not so worried about the quality of the std::hash implementation for std::string_view, and integers/pointers can just hash to their own value.

The multiple levels of indirection and poor data locality (cache misses) in std::unordered_map overwhelm the savings on lookup compared to a binary search, until you get to a very large number of items. Anecdotal benchmarks I've heard say you need at least 100 entries to make std::unordered_map break even, but really we should do some of our own benchmarking/profiling.

You mentioned you are dealing with large numbers of fields per object, so maybe you're more likely to hit that break even point than I expected. If you want to try comparing them for your scenario, take a look at fa772a1 where I already did this for the ResolverMap generated for Object. If you locally change where it declares the ResolverMap type as a std::vector<std::pair<>> back to std::unordered_map and change the lookup in SelectionVisitor::visitField back to a call to find, you can see if/what difference it makes.


// ValidateVariableTypeVisitor visits the AST and builds a ValidateType structure representing
// a variable type in an operation definition as if it came from an Introspection query.
class ValidateVariableTypeVisitor
{
public:
ValidateVariableTypeVisitor(const ValidateTypeKinds& typeKinds);
ValidateVariableTypeVisitor(
const std::shared_ptr<schema::Schema>& schema, const ValidateTypes& types);

void visit(const peg::ast_node& typeName);

Expand All @@ -152,7 +157,8 @@ class ValidateVariableTypeVisitor
void visitListType(const peg::ast_node& listType);
void visitNonNullType(const peg::ast_node& nonNullType);

const ValidateTypeKinds& _typeKinds;
const std::shared_ptr<schema::Schema>& _schema;
const ValidateTypes& _types;

bool _isInputType = false;
ValidateType _variableType;
Expand All @@ -163,38 +169,34 @@ class ValidateVariableTypeVisitor
class ValidateExecutableVisitor
{
public:
ValidateExecutableVisitor(const Request& service);
ValidateExecutableVisitor(const std::shared_ptr<schema::Schema>& schema);

void visit(const peg::ast_node& root);

std::vector<schema_error> getStructuredErrors();

private:
response::Value executeQuery(std::string_view query) const;

static ValidateTypeFieldArguments getArguments(response::ListType&& argumentsMember);
static ValidateTypeFieldArguments getArguments(
const std::vector<std::shared_ptr<const schema::InputValue>>& args);

using FieldTypes = std::map<std::string, ValidateTypeField>;
using TypeFields = std::map<std::string, FieldTypes>;
using FieldTypes = std::map<std::string_view, ValidateTypeField>;
using TypeFields = std::map<std::string_view, FieldTypes>;
using InputFieldTypes = ValidateTypeFieldArguments;
using InputTypeFields = std::map<std::string, InputFieldTypes>;
using EnumValues = std::map<std::string, std::set<std::string>>;
using InputTypeFields = std::map<std::string_view, InputFieldTypes>;
using EnumValues = std::map<std::string_view, std::set<std::string_view>>;

std::optional<introspection::TypeKind> getTypeKind(const std::string& name) const;
std::optional<introspection::TypeKind> getScopedTypeKind() const;
constexpr bool isScalarType(introspection::TypeKind kind);

bool matchesScopedType(const std::string& name) const;
bool matchesScopedType(std::string_view name) const;

TypeFields::const_iterator getScopedTypeFields();
InputTypeFields::const_iterator getInputTypeFields(const std::string& name);
InputTypeFields::const_iterator getInputTypeFields(std::string_view name);
static const ValidateType& getValidateFieldType(const FieldTypes::mapped_type& value);
static const ValidateType& getValidateFieldType(const InputFieldTypes::mapped_type& value);
template <class _FieldTypes>
static std::string getFieldType(const _FieldTypes& fields, const std::string& name);
static ValidateType getFieldType(const _FieldTypes& fields, std::string_view name);
template <class _FieldTypes>
static std::string getWrappedFieldType(const _FieldTypes& fields, const std::string& name);
static std::string getWrappedFieldType(const ValidateType& returnType);
static ValidateType getWrappedFieldType(const _FieldTypes& fields, std::string_view name);

void visitFragmentDefinition(const peg::ast_node& fragmentDefinition);
void visitOperationDefinition(const peg::ast_node& operationDefinition);
Expand All @@ -213,23 +215,22 @@ class ValidateExecutableVisitor
bool validateVariableType(bool isNonNull, const ValidateType& variableType,
const schema_location& position, const ValidateType& inputType);

const Request& _service;
const std::shared_ptr<schema::Schema> _schema;
std::vector<schema_error> _errors;

using OperationTypes = std::map<std::string_view, std::string>;
using Directives = std::map<std::string, ValidateDirective>;
using ExecutableNodes = std::map<std::string, const peg::ast_node&>;
using FragmentSet = std::unordered_set<std::string>;
using MatchingTypes = std::map<std::string, std::set<std::string>>;
using ScalarTypes = std::set<std::string>;
using VariableDefinitions = std::map<std::string, const peg::ast_node&>;
using VariableTypes = std::map<std::string, ValidateArgument>;
using Directives = std::map<std::string_view, ValidateDirective>;
using ExecutableNodes = std::map<std::string_view, const peg::ast_node&>;
using FragmentSet = std::unordered_set<std::string_view>;
using MatchingTypes = std::map<std::string_view, std::set<std::string_view>>;
using ScalarTypes = std::set<std::string_view>;
using VariableDefinitions = std::map<std::string_view, const peg::ast_node&>;
using VariableTypes = std::map<std::string_view, ValidateArgument>;
using OperationVariables = std::optional<VariableTypes>;
using VariableSet = std::set<std::string>;
using VariableSet = std::set<std::string_view>;

// These members store Introspection schema information which does not change between queries.
OperationTypes _operationTypes;
ValidateTypeKinds _typeKinds;
ValidateTypes _operationTypes;
ValidateTypes _types;
MatchingTypes _matchingTypes;
Directives _directives;
EnumValues _enumValues;
Expand All @@ -250,8 +251,8 @@ class ValidateExecutableVisitor
size_t _fieldCount = 0;
TypeFields _typeFields;
InputTypeFields _inputTypeFields;
std::string _scopedType;
std::map<std::string, ValidateField> _selectionFields;
ValidateType _scopedType;
std::map<std::string_view, ValidateField> _selectionFields;
};

} /* namespace graphql::service */
Expand Down
Loading