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

Return early in GraphQL fill for primitive partial #2701

Merged
merged 1 commit into from Dec 12, 2023

Conversation

Flufd
Copy link
Contributor

@Flufd Flufd commented Dec 7, 2023

Description

Optimises the GraphQL filler by returning early in the createValue function if we have a concrete partial value.


I was profiling some slower tests in admin and found that a lot of the time was spent calling faker's seed method. This is done with a stable seed for the particular branch/leaf node on the GraphQL document so that the random values for each node are stable. The re-seeding of the faker instance is slow, because it needs to reinitialise the mersenne twister that is used for RNG.

For large lists of known primitive values, such as currency or country codes, or other arrays of enums, which in the admin might be 200+ elements long for country codes, this becomes a performance issue. We are reseeding the faker instance for each element, even when not consuming it.


Another optimisation and some thoughts on random data in general:
#2700

@Flufd Flufd marked this pull request as ready for review December 7, 2023 15:47
@Flufd Flufd requested a review from a team as a code owner December 7, 2023 15:47
@Flufd
Copy link
Contributor Author

Flufd commented Dec 7, 2023

/snapit

@Flufd Flufd force-pushed the graphql-fixtures/optimise-fill branch from 3b82927 to 656eade Compare December 7, 2023 16:00
@Flufd
Copy link
Contributor Author

Flufd commented Dec 7, 2023

/snapit

Copy link
Contributor

github-actions bot commented Dec 7, 2023

🫰✨ Thanks @Flufd! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

yarn add graphql-fixtures@4.0.1

})),
});

expect(spy.mock.calls.length).toBeLessThan(100);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this change, the seed is reset 8000+ times, with the change it is ~60

@Flufd Flufd force-pushed the graphql-fixtures/optimise-fill branch from 656eade to 75b01ee Compare December 7, 2023 21:02
@Flufd Flufd force-pushed the graphql-fixtures/optimise-fill branch from 75b01ee to 43ec02d Compare December 7, 2023 21:18
@Flufd Flufd merged commit 081fd65 into main Dec 12, 2023
9 checks passed
@Flufd Flufd deleted the graphql-fixtures/optimise-fill branch December 12, 2023 16:17
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.

None yet

3 participants