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

Feature/persistent term #200

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Feature/persistent term #200

wants to merge 10 commits into from

Conversation

jlouis
Copy link
Owner

@jlouis jlouis commented Dec 16, 2018

No description provided.

The schema in a GraphQL engine has the property that it is very rarely
updated if at all, but is read into every process using GraphQL calls.
To avoid copying the term into every process, we store this as a
persistent_term upon schema validation.

This avoids the copy into the process and shares the term for every
process in the system. The major gain is not the avoidance of ETS
locking, but avoiding putting the schema on the heap. For large
queries, this doesn't matter, but smaller queries which touch lots of
the schema, this should lead to a considerably leaner process.
All calls to this function can be served by graphql_schema:lookup/1
easily. This streamlines the code.
If we don't have persistent_term support, the call will fail. Back it
by the older ETS lookup. This will make the system run on pre OTP 21.2
machines as well as provide a forward path.
Fixes a dialyzer warning in the system.
Move specifications under the ifdef's so we can make the dialyzer
happy on older releases.
The reason we use persistent term is to avoid copying the schema more
than making things run fast. So use this construction to handle OTP
21.0 to OTP 21.1 versions. We can lift this restriction later on when
people are more likely to have upgraded to newer versions. And when
OTP 22.0 comes out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant