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

Maven transitive scanning is slow due to the new registry support #1447

Open
spencerschrock opened this issue Dec 16, 2024 · 7 comments
Open
Assignees

Comments

@spencerschrock
Copy link
Contributor

Scorecard's public scanning infrastructure has been having some runtime issues since upgrading to v1.9.1. I'm hoping downgrade in our deployment to confirm the issue goes away, but it lines up with when we merged the dependabot PR.

pprof profiling shows a lot of time in deps.dev/util/semver, specifically around the Maven dependencies.

resolve (* APIClient) MatchingVersions 31.57%
resolve (* APIClient) mavenRequirements 31%
semver (*Constraint) Match 24.33%

After chatting with the deps.dev team, it's know for deps.dev/util/semver to be a slow operation. I don't think its usage is new between v1.9.0 and v1.9.1, but perhaps it's used in a new way?

v1.9.0...v1.9.1

@cuixq cuixq self-assigned this Dec 17, 2024
@michaelkedar
Copy link
Member

Hmm that does line up with a change I made to the Maven resolver google/deps.dev@b3ba033

Though what benchmarks I did run on it found it should've been faster.

@spencerschrock
Copy link
Contributor Author

spencerschrock commented Dec 17, 2024

I profiled again before the v1.9.0 downgrade and after, and didn't see the maven code as a hotspot in either. So it's possible we were processing a lot of Java repositories at the time.

However after deploying the new revision to GKE (where the only change was the downgrade), our CPU resources dropped significantly (from ~5.0 CPU to ~1.0 CPU)

Our p99 latencies for the scan also went from occasional spikes of 4000 seconds back to ~50 seconds. I'll see if I can isolate a problematic repo

@cuixq
Copy link
Contributor

cuixq commented Dec 17, 2024

@spencerschrock thank you for your updates! I suspect #1286 might be the problematic change. I already have a draft PR which should reduce the number of Maven repositories we fetch.

@spencerschrock
Copy link
Contributor Author

spencerschrock commented Dec 17, 2024

Our p99 latencies for the scan also went from occasional spikes of 4000 seconds back to ~50 seconds. I'll see if I can isolate a problematic repo

One example is github.com/SpringCloud/spring-cloud-gray

which took 3-4 minutes to scan on v1.9.0 and on v1.9.1 I gave up after 15 minutes

Similarly github.com/SpringCloud/spring-cloud-dubbo

@spencerschrock
Copy link
Contributor Author

spencerschrock commented Dec 18, 2024

[@spencerschrock](https://github.com/spencerschrock) thank you for your updates! I suspect #1286 might be the problematic change. I already have a draft PR which should reduce the number of Maven repositories we fetch.

git bisect agrees that #1286 is the problematic change. I checked out your PR to experiment with and it helps a little but I still have a 3-4x in runtime compared to before #1286 when running with: osv-scanner scan -r /tmp/blah

@cuixq
Copy link
Contributor

cuixq commented Dec 18, 2024

It is a known issue that Maven resolution is slow due to the new registry support, and I am working on that.

I will change the title of this issue to reflect the root cause.

@cuixq cuixq changed the title expensive maven parsing on v1.9.1 Maven transitive scanning is slow due to the new registry support Dec 18, 2024
@michaelkedar
Copy link
Member

That change also added the PreFetch() to maven resolution, which is why MatchingVersions -> Match gets used so much.
I've made #1456 to remove that part of it, since it's expensive and not actually helpful.

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

No branches or pull requests

3 participants