-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix: refactor build tree algorithm #59
Conversation
3e67526
to
c2c7bcc
Compare
:wow: 👏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments. Thank you for doing this it's awesome!
c2c7bcc
to
2c271a4
Compare
can you also rename to commit to |
a0b33bf
to
e325685
Compare
let depResolvedName = ''; | ||
let originalDepKey = ''; | ||
function isFreqDep(packageName: string): boolean { | ||
return packageName in freqDeps; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This syntax actually works?! TIL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orsagie this works :) but you need to be really careful :) cos in freqDeps
is an object this will also return true:
'toString' in freqDeps
'__proto__' in freqDeps
'constructor' in freqDeps
etc
executing snyk test took very long time due to the parsing algorithm. this pr is a poc that introduce a different approach for constructing dep tree using bfs algorithm. with this technique processing time decreased by x25
e325685
to
850f018
Compare
🎉 This PR is included in version 1.11.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
running
snyk test
command in order to scan local NuGet dependencies, took approx ~2.5 minutes in the best case for a user. after digging a bit, It was easy to notice that the dep tree parsing algorithm was the root cause for this issue since it used recursive function inside a loop, which has high time complexity. this PR introduces a different approach for constructing a dep tree using the BFS algorithm. with this technique, processing time decreased by x25.How should this be manually tested?
snyk-dev test --file="some.nugetdeps.json"
Any background context you want to provide?
zendesk user complain
Screenshots
Before (139120ms):
After (5478ms):