feat: add memoization to when converting a graph into a tree

[admons]

• Ready for review
• Follows CONTRIBUTING rules
• Reviewed by Snyk internal team

What does this PR do?

It adds memoization in the graphToDepTree
This will result in reused objects in a tree instead, for example the following graph

A --> B --> D
  \-> C --> B

Instead of creating B --> D twice (as today), it will reuse the B subtree

[FauxFaux]

Does this just allocate forever?

[admons]

Does this just allocate forever?

No, it's a scoped function, GC will clean memoizationMap.

function a(b = new Map()) { return b; }
assert.notEqual(a(), a()); // true

[robcresswell]

This looks cool @admons! Any idea on the impact?

[admons]

@robcresswell thanks :)

1. This will create trees that today would get stuck and won't be generated (i.e. root -> 100,000 children -> all pointing to a specific node "B" -> 50,000 children)
   For those trees, the problem will be on the serialisation phase (if needed)
2. In case that we have a code (in another library/service) that mutate a specific node in a tree, if this node is reused, there will be side affects

[robcresswell]

@admons Ah sorry I should've been more specific. I got the overall idea; I meant rather if you'd been able to benchmark anything. Just out of curiosity than anything else 🤓

[admons]

@robcresswell :)
I have a DepGraph of the cockroachDB and its an enormous graph see here
More than 15 minutes of 100% cpu to convert this depgraph still wasn't enought
After adding this change, it took 88ms

[robcresswell]

Niiice, thats what I wanted to hear 🤓

[darscan]

🎉

[snyksec]

🎉 This PR is included in version 1.21.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀