-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[tools] Use mold linker on Linux debug builds #22951
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
[tools] Use mold linker on Linux debug builds #22951
Conversation
d3a26fb
to
f66437a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tons of builds will fail without revised provisioning. The goal at this point is for the correct set of builds to fail.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
@drake-jenkins-bot linux-jammy-unprovisioned-clang-bazel-experimental-debug please. |
f66437a
to
f2c146e
Compare
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-jammy-unprovisioned-clang-bazel-experimental-debug please. |
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are now at an interesting spot. Pretty much the correct sets of builds pass and fail, and for the right reasons (provisioned debugs fail for lack of requested mold
linker). The outlier is unprovisioned debug on jammy; the old version of mold
in Jammy is not good enough to pass the install test. I will claim this maybe doesn't matter, and perhaps the plan should be to disable that test when config is (Jammy AND -compilation-mode=dbg).
The version of mold
in noble is (recently?) considerably newer, and handles install test just fine.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-jammy-unprovisioned-gcc-bazel-experimental-debug please. |
c688b02
to
b8d155a
Compare
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
dd016ff
to
999792f
Compare
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
1 similar comment
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r2
disabled some tests for jammy and dbg. Now the only build failing that should not is
https://drake-jenkins.csail.mit.edu/job/linux-jammy-unprovisioned-gcc-bazel-experimental-debug/27/
which is reported "no space left on device".
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
999792f
to
e5933ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest (still r2
according to reviewable) abandons jammy mold.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-(status: do not review) +@jwnimmer-tri for a first read.
Still not-merge, since this will need Noble CI provisioning updates.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
@drake-jenkins-bot linux-noble-unprovisioned-clang-bazel-experimental-debug please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 4 files at r2.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
setup/ubuntu/source_distribution/packages-noble-test-only.txt
line 3 at r2 (raw file):
curl libstdc++6-10-dbg mold
Users who build Drake from source via CMake still end up using -c dbg
because we plumb the CMake "Debug" option through to -c dbg
in our CMake-Bazel wrapper. Thus, those users will now get an error message about not having mold, since they don't enable "test only" dependencies.
Do we want downstream project consuming Drake as an external to use mold, or should it only be used for Drake Developers? That will decide how we go about re-aligning things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
setup/ubuntu/source_distribution/packages-noble-test-only.txt
line 3 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Users who build Drake from source via CMake still end up using
-c dbg
because we plumb the CMake "Debug" option through to-c dbg
in our CMake-Bazel wrapper. Thus, those users will now get an error message about not having mold, since they don't enable "test only" dependencies.Do we want downstream project consuming Drake as an external to use mold, or should it only be used for Drake Developers? That will decide how we go about re-aligning things.
It's a good question. Right now we don't have an alternate story for getting gdb line numbers to work, but maybe we don't care about that w.r.t. downstream projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @rpoyner-tri)
setup/ubuntu/source_distribution/packages-noble-test-only.txt
line 3 at r2 (raw file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
It's a good question. Right now we don't have an alternate story for getting gdb line numbers to work, but maybe we don't care about that w.r.t. downstream projects.
Having a broken build is worse that missing line numbers. Since it's probably somewhat difficult to engineer this to work correctly when Drake is used as a CMake external, at the very least mold should be Bazel-only, and probably even Developer-only.
@drake-jenkins-bot linux-noble-clang-bazel-experimental-everything-release please |
@drake-jenkins-bot linux-noble-clang-bazel-experimental-address-sanitizer please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dismissed @rpoyner-tri from a discussion.
Reviewable status: labeled "do not merge" (waiting on @rpoyner-tri)
@drake-jenkins-bot linux-noble-gcc-bazel-experimental-debug please The The valgrind failure was just a timeout while running the test, we can ignore that for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, labeled "do not merge" (waiting on @rpoyner-tri)
a discussion (no related file):
Per f2f, the linux-noble-gcc-bazel-experimental-debug
failure is probably real, and needs further attention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: 1 unresolved discussion, labeled "do not merge"
3d04b14
to
97b6aaa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: 1 unresolved discussion, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits)
d50cf1e
to
c143691
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-(status: do not merge)
Reviewable status: labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits)
c143691
to
eec4257
Compare
@drake-jenkins-bot linux-noble-gcc-bazel-experimental-debug please |
79c971b
to
e59346f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits)
a discussion (no related file):
Hand-tuning test timeouts sorta worked, but what I now suspect is going on is that mold and bazel are blindly spamming out threads and processes in an unhealthy way. Let's try thread-limiting mold instead.
e59346f
to
71a68a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! all discussions resolved, LGTM from assignees sammy-tri(platform),jwnimmer-tri(platform)
a discussion (no related file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
Hand-tuning test timeouts sorta worked, but what I now suspect is going on is that mold and bazel are blindly spamming out threads and processes in an unhealthy way. Let's try thread-limiting mold instead.
All good that time. If the mold debug build causes further trouble, we can dail the --thread-count=2
down to 1.
This reverts commit 22cbc53.
-(release notes: fix) +(release notes: none) because this was reverted. |
Closes: #21836.
This change is