Possible memory leak in amphora.components.get

[salomoneb]

Description

It looks like there's inefficient garbage collection happening when we call amphora.components.get, which may indicate a memory leak.

Background: our team created this script to batch update all instances of a component to the latest version. (You run it like node upgrade-components.js <component-name>.) I found that when I ran it on components with many instances (hundreds of thousands) that needed to be upgraded, it crashed with errors like this:

When I commented out the await amphora.components.get(component._ref, locals); call on line 56, it ran fine.

I tested the script on a component with 132,992 instances that needed upgrading. I ran the script twice: the first time without the get call, the second time with it. For each test, I called process.memoryUsage inside the for...await loop on line 52 and logged the output to a separate file. Here's the raw data:

• Without the get call - https://docs.google.com/spreadsheets/d/1oZ9n0DgyGUKmsSBlSB2sAD8S5vbbLGTSy33AaRum2j8/edit?usp=sharing
• With the get call - https://docs.google.com/spreadsheets/d/16RdsKOuL6WrJwIHZaxyn-9gEZhVvfPAGOUBIL2HSkmA/edit?usp=sharing

Here is the heap total size plotted for each test (values are in MB):

For the test without the get, the total heap size stays almost the same. However, for the test with the get, the total heap size expands from 79 MB at the start to 1 GB. In the graph, you can see how the heap size stays relatively flat before increasing and then plummeting (I assume this is garbage collection happening).

Steps to Reproduce

Try running the script in that gist on a component with hundreds of thousands of instances that need be upgraded and see if it can complete without running out of memory. The script did complete in both test cases on the component with 132k instances, but we saw failures on components with larger numbers.

Next Steps

We haven't experienced problems with this on production, but we also have a tool set up to auto-restart our server if it runs out of memory so it's possible that this has occurred and we just haven't noticed it.

If I were to debug this further, I would probably run the script with the node-inspector in Chrome and try to pull a more detailed memory profile. I'd also begin commenting things out in components.get, starting with return models.get (like, is this even a get problem or is one of the checks before the get call doing something?).