codeql icon indicating copy to clipboard operation
codeql copied to clipboard

CodeQL run time increased from mins to hours

Open asreehari-splunk opened this issue 1 year ago • 10 comments

The code analysis run duration increased from mins to hours from 2.16.4. I've attached the runtime options as pdf for both versions below 2.16.4.pdf 2.16.3.pdf

It was consistently in the minute range before and has only increase since 2.16.4. The exact date seems to be March 12th, 2024 Latest version that is being used is "2.17.1"

asreehari-splunk avatar May 07 '24 20:05 asreehari-splunk

Hi @asreehari-splunk 👋

We shipped some updates to our Go support in 2.16.4 which mean that we better understand large Go repositories and analyse more code than we did in previous versions of CodeQL. For large repositories, that can lead to big increases in the time needed for the analysis because we perform a lot more work than before.

We shipped some performance improvements in CodeQL 2.17.0 that should have improved the time again. Can you confirm that you've tried 2.17.0 or later? If so, has that made any difference at all?

mbg avatar May 07 '24 22:05 mbg

Hi @mbg ...thank you for the clarification. yeah, looks like we are currently using 2.17.1 and seeing 1h+ run time. here is a screenshot

Screenshot 2024-05-07 at 4 37 10 PM

asreehari-splunk avatar May 07 '24 23:05 asreehari-splunk

one follow up question. Is there a metric I can look for like # of lines-of-code/files/packages etc that qualifies as large ? to my knowledge there hasn't been a big change in the repo

asreehari-splunk avatar May 08 '24 00:05 asreehari-splunk

@mbg These are two runs at almost the same time, the main difference is the switch from 2.16.3 to 2.16.4 . The source code should be mostly identical. It's the autobuilding phase that is taking an hour more. Was there any change in the autobuilding strategy or is there a some performance problem in the extractor? I also see that CodeQL is running the go tests, these are not needed for analysis so perhaps test running can be skipped to speed things up.

  • https://github.com/open-telemetry/opentelemetry-collector/actions/runs/8255666766
  • https://github.com/open-telemetry/opentelemetry-collector/actions/runs/8255818248

aibaars avatar May 08 '24 08:05 aibaars

Looking at those logs, it could be that @mbg is right about 2.16.4 scanning more code:

  • CodeQL scanned 155 out of 917 Go files in this invocation. Check the status page for overall coverage information: https://github.com/open-telemetry/opentelemetry-collector/security/code-scanning/tools/CodeQL/status/
  • CodeQL scanned 488 out of 917 Go files in this invocation. Check the status page for overall coverage information: https://github.com/open-telemetry/opentelemetry-collector/security/code-scanning/tools/CodeQL/status/

Spending 6 times longer on 3 times as many files sounds like there may still be a performance issue. It can also be that the test cases for the additional files simply take a long time. Could you try a run without running any tests (a test pull request with a quick-n-dirty tweak to the Makefile should work).

aibaars avatar May 08 '24 08:05 aibaars

Thanks @aibaars for having a look.

Like I said, we shipped big changes to the Go autobuilder in 2.16.4 which result in better support for repositories with multiple go.mod files. In CodeQL 2.16.3 and earlier, we didn't really support that well and our analysis of such repositories was just a best effort. As @aibaars notes for your repository, that meant we only extracted 155 out of 917 Go files then.

As of 2.16.4 and above, we do support repositories with multiple go.mod files properly and that often explains an increase in extraction/analysis time for such repositories, since we actually extract more code now. As @aibaars notes, that's now 488 out of 917 Go files.

I have had a look over your logs to determine why there is such a big increase in the time needed for this now and why this has not improved with 2.17.0 or above (when we shipped performance improvements to dependency extraction that improved the times for most users).

For mainly historical reasons, we run make if there is a Makefile in the repository before we begin extraction of the source code. Your Makefile in particular seems to build and test all of your code.

When we implemented the changes to the Go autobuilder in 2.16.4, we kept the part that invokes make before extraction to ensure that CodeQL would not suddenly break for repositories which relied on this behaviour. However, it seems that this now gets erroneously invoked for every go.mod file in your repository. I will look into getting this fixed ASAP.

In the meantime, to avoid this issue until it is fixed, you can either revert to 2.16.3 (but fewer Go sources files will get extracted) or switch to a custom build. The latter would involve replacing the autobuild step in your workflow with a step that invokes the right build commands for your repository (possibly just make).

mbg avatar May 08 '24 09:05 mbg

Thank you @aibaars and @mbg for the quick follow ups and really appreciate helping me understand the root cause. I will take this back and see what the team has to say. Do we need to change the label on this to something other than a question ?

asreehari-splunk avatar May 08 '24 17:05 asreehari-splunk

I have updated the labels, but we are also tracking this internally as well.

mbg avatar May 08 '24 18:05 mbg

@asreehari-splunk We merged https://github.com/github/codeql/pull/16704 yesterday which should fix this issue. I also tested it with your repo to confirm this. It will take a little before this trickles down into a CodeQL release, but once it has you should see your CodeQL workflow times improve.

It will still be a little longer than they used to be, because we do extract more code than before as of the changes that we shipped back in 2.17.0, but it shouldn't be nearly as long as the workflows take to run at the moment.

Sorry this took a bit longer to fix than I anticipated - we discovered a chain of other things that needed to be fixed or improved first and some of us were on holidays recently as well.

I will leave this issue open for now and would appreciate an update once you notice a change. Based on current plans, I am expecting this to ship in CodeQL 2.18.0, which should be released in about a month. If you want to give it a go before then, you could use one of our nightly releases by pointing the tools property of the codeql-action/init action at a recent CLI bundle (e.g. this one should work).

mbg avatar Jun 12 '24 10:06 mbg

Thanks Michael. I was away for a couple of weeks. will review this in the coming days

On Wed, Jun 12, 2024, 4:29 PM Michael B. Gale @.***> wrote:

[ External sender. Exercise caution. ]

@asreehari-splunk https://github.com/asreehari-splunk We merged #16704 https://github.com/github/codeql/pull/16704 yesterday which should fix this issue. I also tested it with your repo to confirm this. It will take a little before this trickles down into a CodeQL release, but once it has you should see your CodeQL workflow times improve.

It will still be a little longer than they used to be, because we do extract more code than before as of the changes that we shipped back in 2.17.0, but it shouldn't be nearly as long as the workflows take to run at the moment.

Sorry this took a bit longer to fix than I anticipated - we discovered a chain of other things that needed to be fixed or improved first and some of us were on holidays recently as well.

I will leave this issue open for now and would appreciate an update once you notice a change. Based on current plans, I am expecting this to ship in CodeQL 2.18.0, which should be released in about a month. If you want to give it a go before then, you could use one of our nightly releases https://github.com/dsp-testing/codeql-cli-nightlies/releases by pointing the tools property of the codeql-action/init action at a recent CLI bundle (e.g. this one https://github.com/dsp-testing/codeql-cli-nightlies/releases/download/codeql-bundle-20240612/codeql-bundle.tar.gz should work).

— Reply to this email directly, view it on GitHub https://github.com/github/codeql/issues/16448#issuecomment-2162716307, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARX2CBO6LW4NOLK3TRV4C5DZHASYFAVCNFSM6AAAAABHLVHCBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSG4YTMMZQG4 . You are receiving this because you were mentioned.Message ID: @.***>

asreehari-splunk avatar Jul 01 '24 15:07 asreehari-splunk

@mbg ... finally got around to this. So i am not entirely sure if i am doing this right. Here is a PR i opened with the change you suggested https://github.com/open-telemetry/opentelemetry-collector/pull/10586/files .

Using CodeQL CLI sourced from https://github.com/dsp-testing/codeql-cli-nightlies/releases/download/codeql-bundle-20240612/codeql-bundle.tar.gz.
  Downloading CodeQL tools from https://github.com/dsp-testing/codeql-cli-nightlies/releases/download/codeql-bundle-20240612/codeql-bundle.tar.gz . This may take a while.
  /usr/bin/tar xz --warning=no-unknown-keyword --overwrite -C /home/runner/work/_temp/b1ddbda0-7fc1-4aaa-b676-4e4ab5b8b5b6 -f /home/runner/work/_temp/7f6f5a67-0928-40e8-9e2c-02feb425ed1d
  /opt/hostedtoolcache/CodeQL/0.0.0-20240612/x64/codeql/codeql version --format=json
  {
    "productName" : "CodeQL",
    "vendor" : "GitHub",
    "version" : "2.17.5+202406[11](https://github.com/open-telemetry/opentelemetry-collector/actions/runs/9879255129/job/27286113393?pr=10586#step:4:12)1339",
    "sha" : "6071e11690bb2c1ca9368fd2f92f12742737d75c",

I still see it pick up 2.17.5 and not 2.18+ . I don't know if i am missing something obvious here. any suggestions ?

asreehari-splunk avatar Jul 10 '24 18:07 asreehari-splunk

Hi @asreehari-splunk, sorry about that. It looks like the nightly release doesn't include the changes since it's based on the last 2.17 branch. However, CodeQL 2.18.0 is being released today, so as soon as that rolls out you should see that version in your workflow (without the extra tools input), including the fix for this.

mbg avatar Jul 11 '24 09:07 mbg

@mbg there have been like 3 pipelines so far but i am seeing significant improvement in the CodeQL step of the pipeline. Here is the last one before 2.18 https://github.com/open-telemetry/opentelemetry-collector/actions/runs/9904690698/job/27362635937 after/with 2.18 https://github.com/open-telemetry/opentelemetry-collector/actions/runs/9910238732/job/27380151186

Appreciate the time spent on this.

asreehari-splunk avatar Jul 12 '24 16:07 asreehari-splunk

That's great to hear, thanks for the update! That all looks good to me as well, so I will go ahead and close this issue now, but feel free to reopen or open another issue if you run into another problem.

mbg avatar Jul 15 '24 12:07 mbg