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

single packages.Load for NameForPackage #983

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

vikstrous
Copy link
Contributor

@vikstrous vikstrous commented Jan 9, 2020

I simplified this PR to make the minimum necessary changes to get the job done. This PR supersedes #944
Unfortunately, I had to use global state because the existing code was using global state. I'm willing to refactor this in a follow up, ending up with something similar to #944 if that makes more sense.

I didn't add any new tests because exists tests already cover this change.

Non-scientific test comparing master vs this PR on our proprietary codebase:

Before:

40.92user 23.75system 0:27.99elapsed 231%CPU (0avgtext+0avgdata 197584maxresident)k
0inputs+0outputs (1major+430432minor)pagefaults 0swaps

After:

21.46user 8.37system 0:13.77elapsed 216%CPU (0avgtext+0avgdata 201152maxresident)k
0inputs+0outputs (1major+160637minor)pagefaults 0swaps

This is done on a pretty weak machine with somewhat slow IO. I used the second results for each after running the tests twice.

I have:

  • n/a Added tests covering the bug / feature (see testing)
  • n/a Updated any relevant documentation (see docs)

@vikstrous vikstrous force-pushed the name-for-package-global branch from 7c3b7d6 to 2305621 Compare January 9, 2020 05:17
@vikstrous vikstrous marked this pull request as ready for review January 9, 2020 05:23
@vikstrous vikstrous force-pushed the name-for-package-global branch from 2305621 to 31341b1 Compare January 9, 2020 05:23
@vektah
Copy link
Collaborator

vektah commented Jan 15, 2020

Thanks, this looks a bit cleaner. Shame about the global but we can iterate. I think the other PR had issues with multiple levels of caching.

@vektah vektah merged commit c6b3e2a into 99designs:master Jan 15, 2020
cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
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.

2 participants