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

Lambda lazy load #2278

Closed
wants to merge 77 commits into from
Closed

Lambda lazy load #2278

wants to merge 77 commits into from

Conversation

kyeotic
Copy link
Contributor

@kyeotic kyeotic commented Feb 6, 2019

TODO:

  • Update CHANGELOG.md with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests ("significant" is a bit subjective, please let me know if these changes need tests, I'd be happy to write them if so)
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

This is to address loading time issues raised in #2162

There are three changes of note.

  1. In lambda-server-core I moved the subscriptions-transport-ws and apollo-engine-reporting into lazy-loaded requires and changed the top-level imports to just import the type. Since lambda's cannot take advantage of these packages, loading them is pure cost. I didn't see any other type-only imports to style after, so I followed the handbook convention of underscore-prefix-capitalized.
  2. I had the util.promisify polyfill check the node version before loading itself. NOTE: there are already tests in node 6 and node 8 that exercise this code
  3. this is a hack I added an ENV VAR check into runtimeSupportsUploads() that looks for AWS_EXECUTION_ENV, which the lambda runtime sets. This checks stop the import of apollo-upload from occuring. The library is unfortunately a top-level import, so moving it anywhere that apollo-server-lambda could have controlled directly without changing the contract was impossible. A better solution probably exists that does not hack its way through process.env. The best solution would be removing apollo-upload from the core entirely, since it surely does not belong there.

These changes combine to reduce the cold-start time of a 512mb AWS Lambda from ~450ms to ~250ms.

Before
image

After
image

(Yes, the flame graph has a rendering bug, but the numbers are accurate)

512 is a pretty beefy lambda for node, 256 is more common. These effect will be amplified the lower memory you go, because AWS matches the CPU to the allocated memory. If it is necessary, I can re-run these performance profiling on smaller lambdas.

addityasingh and others added 30 commits October 3, 2018 01:59
This specific port per integration is pretty brittle to begin with, but it
does work.  Currently, the fact that it works is facilitated by the fact
that most people don't use 5555 (Hapi) and 6666 (Express) for anything.

That said, the ever-popular Gatsby uses 8888 by default, so let's use 9999!
abernix and others added 25 commits February 13, 2019 15:34
Without this fix to `package-lock.json`, which it's not immediately clear to
me what was wrong (possibly a transitive error) there were type errors
manifesting on the `master` branch.
I've added these on Mac, and I believe that Mac will no longer _remove_ them
after [email protected], so hopefully fix some `package-lock.json` flapping!
When debugging the fix in b84770c, I
removed the `package-lock.json`, ran `npm install`, then individually
reviewed the fixes to better understand what happened.

I noted a few other differences in the `package-lock.json`, mostly due to
changes in `fsevents` (which internally switched to `bundledDependencies`),
and changes in `@apollographql/apollo-tools`.

I've reviewed these as well, and they all look reasonable, but wanted to put
them in a different commit for later bisecting purposes (hopefully a purpose
I won't have!).
There was a breaking change to `core-js` in `3.0.0-beta.12` which seems to
warrant this extra protection.
These missing configurations, which weren't identified in the original
implementation of `apollo-server-azure-functions` in #1926, are responsible
for the failures which have surfaced in the #2228, which updates Jest to v24.x.
The purpose of this PR is to remove the `apollo-graphql` package from this repository, to be added to `apollo-tooling` instead.

https://github.com/apollographql/apollo-tooling

Ref: apollographql/apollo-tooling#1018
This will allow Lerna to release the first version as
`[email protected]`.
hacky lambda process.env fix"

fix linting issues

update changelog

fix polyfill check

use AWS_EXECUTION_ENV

fix styling

fix version check

Update CHANGELOG.md
@kyeotic
Copy link
Contributor Author

kyeotic commented Feb 14, 2019

I screwed up a rebease and cant seem to unscrew it, so I am closing this and opening #2324

@kyeotic kyeotic closed this Feb 14, 2019
@kyeotic kyeotic deleted the lambda_lazy_load branch February 14, 2019 16:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants