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

Revive LGTM C++ analysis #58246

Merged
merged 1 commit into from
Jun 11, 2022
Merged

Revive LGTM C++ analysis #58246

merged 1 commit into from
Jun 11, 2022

Conversation

BrettDong
Copy link
Member

@BrettDong BrettDong commented Jun 7, 2022

Summary

None

Purpose of change

LGTM C++ analysis has been timing out for months, resulting in a red X mark in all C++ pull requests. This does not look good.

Describe the solution

After doing a few test builds with different configurations, I found that excluding the tests from the analysis ensures it can complete in around two hours and a half. Including the tests adds 1.5 hours of build time and analysis time, and brings the total time to 4 hours and timeouts. So this pull request instructs LGTM to only analyze the main game and exclude the tests.

Describe alternatives you've considered

  1. Use CMake and Ninja to spawn parallel compilation jobs as many as available logical CPUs, but in a real test it exhausts memory and thrashes the system
  2. Use Clang compiler, which is expected to be slightly faster than GCC in compile speeds, but I ran into the problem that CodeQL database is not properly generated on LGTM.com build environment and no useful analysis is performed

Testing

LGTM C++ analysis succeeded, finishing within two hours: https://lgtm.com/logs/f4c70305b114ec88fac7a40b4b6277bb6192150e/lang:cpp

Additional context

LGTM C++ analysis may still fail on this pull request, because building the master branch still times out. But building the merge commit should succeed.

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jun 7, 2022
@BrettDong BrettDong changed the title Specify build command in LGTM.com analysis Revive LGTM C++ analysis Jun 8, 2022
@BrettDong BrettDong marked this pull request as ready for review June 8, 2022 01:37
@BrettDong BrettDong added the Code: Tests Measurement, self-control, statistics, balancing. label Jun 9, 2022
@dseguin dseguin merged commit b3ff1e3 into master Jun 11, 2022
@BrettDong BrettDong deleted the lgtm branch June 11, 2022 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants