Conversation
|
This pull request is stale because it has been open for 60 days with no activity. |
09dbf1c to
49a431a
Compare
Contributor
Automated performance analysis summaryThis comment was automatically generated given there is performance data available. In summary:
You can check a comparison in detail via the grafana link Performance Improvements - Comparison between master and gd_10751_lto.Time Period from 30 days ago. (environment used: oss-standalone)
Performance Regressions and Issues - Comparison between master and gd_10751_lto.Time Period from 30 days ago. (environment used: oss-standalone)
Tests with No Significant Changes (25 tests)Tests with No Significant Changes
|
370e9bf to
606b383
Compare
5e08f3d to
10d61c0
Compare
faa9473 to
9a1c5a8
Compare
75ab323 to
308d9ee
Compare
9a1c5a8 to
37c76f3
Compare
e6ed0eb to
390f528
Compare
67af00f to
fde94ae
Compare
fde94ae to
4e80ee3
Compare
.github/workflows/task-test.yml
Outdated
Comment on lines
+441
to
+443
| LTO_PLATFORMS=("ubuntu:noble") | ||
| CONTAINER="${{ needs.get-config.outputs.container }}" | ||
| if [[ " ${LTO_PLATFORMS[@]} " =~ " ${CONTAINER} " ]]; then |
Collaborator
There was a problem hiding this comment.
Just want to confirm: is the intention to enable LTO in the merge queue? Because the current conditional won't enable it on PRs.
Collaborator
There was a problem hiding this comment.
(Since they don't run in containers)
Collaborator
Author
There was a problem hiding this comment.
Thanks I missed that.
I assumed the merge flow was a subset of the "Build on Platforms" one.
Adding a note about that then, we still need to update the build artifacts flow as well.
Collaborator
There was a problem hiding this comment.
Do we know how much longer the build takes when LTO is enabled in CI?
Collaborator
Author
There was a problem hiding this comment.
Does not seem to slow it down. From a quick unscientific test:
On x86_64:
- LTO: 45min 3s https://github.com/RediSearch/RediSearch/actions/runs/21139820826/job/60790770753
- no LTO: 45min 41s https://github.com/RediSearch/RediSearch/actions/runs/21138590696/job/60787573750
On aarch64:
fbf2f8c to
03cd54c
Compare
This allow users to pick the right clang version manually by defining CC/CXX/LD without changing the default clang on their system.
Alpine's grep do not support -P so use sed instead.
Fix building tests with LTO on arm64.
Save us from re-checking if LTO is enabled or not.
Use the "append or define" pattern.
We should be good now that we call `make rust-tests LTO`
Linux distributions may ship either clang, clang-21 or both as binaries.
eb40356 to
d2ea0bd
Compare
Collaborator
Author
|
@LukeMathWalker : as discussed, this PR now contains only |
LukeMathWalker
approved these changes
Jan 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Various improvements to
build.shneeded to integrate LTO into the CI.Mark if applicable
Note
Improves cross-language LTO handling and robustness in
build.sh.clang/clang++/lldvia$CC/$CXX/$LD, falling back to versionedclang-$RUSTC_LLVM_VERSION/lld-$RUSTC_LLVM_VERSION, else unversioned toolsCMAKE_C_COMPILER,CMAKE_CXX_COMPILER,-fuse-ld) and Rust (RUSTFLAGSwithlinker-plugin-lto,-C linker=...,-C link-arg=-fuse-ld=...)LTO?(=1)) andRUSTFLAGScomposition (compactRUST_DYN_CRThandling); removes redundant LTO-specificRUSTFLAGSexport blockWritten by Cursor Bugbot for commit d2ea0bd. This will update automatically on new commits. Configure here.