Skip to content

perf: use a map to track which advisories should be checked for which packages#216

Merged
G-Rath merged 4 commits intomainfrom
use-a-map
Sep 14, 2023
Merged

perf: use a map to track which advisories should be checked for which packages#216
G-Rath merged 4 commits intomainfrom
use-a-map

Conversation

@G-Rath
Copy link
Owner

@G-Rath G-Rath commented Sep 13, 2023

This greatly improves the performance of determining vulnerabilities since now only advisories that actually could match a package are checked, rather than looping through all of them for all packages which scales much better as the number of advisories in a database grows.

You can easily see the difference in speed by just running make regenerate-e2e-fixtures - before this change it typically takes 30 seconds whereas after it takes only 10.

This also means that the detector no longer loads advisories without any affected packages, which really shouldn't change anything but lets us easily stop using the Vulnerabilities function reducing the size by a couple of KB - I've kept the function because its still used in tests and doesn't have to be removed so this is less breaking.

@G-Rath G-Rath merged commit 2e92110 into main Sep 14, 2023
@G-Rath G-Rath deleted the use-a-map branch September 14, 2023 02:01
another-rex pushed a commit to google/osv-scanner that referenced this pull request Sep 5, 2025
…ages are affected (#2219)

Currently when doing local matching we remove withdrawn advisories
before any further checks by building a dedicated new slice, which
results in huge memory usage - see [this
comment](#2217 (comment))
for some profiles, but e.g. scanning our own `go.mod` takes up 1.5gb of
memory.

I originally implemented it like this back in `osv-detector` as I found
it interesting to include withdrawn advisories to review data quality
and have their count in the output, but it's not worth this. I have
actually since ended up optimizing this out of the detector anyway via
G-Rath/osv-detector#216 which switched to using
a map for vulnerabilities meaning I calculated the count differently,
but for now this just brings over the change for fixing the memory
issue.

Resolves #2217

---------

Co-authored-by: Xueqin Cui <72771658+cuixq@users.noreply.github.com>
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.

1 participant