src.opts.mk: Default LLVM_ASSERTIONS off

[kbowling]

Summary

This noticeably slows down the compiler execution.

It seems like it should be toggled on in the import branches for test and exp-run and default off otherwise.

[jrtc27]

Or, like WITNESS or MALLOC_PRODUCTION, on in HEAD and off in stable branches?

[kbowling]

In D50388#1149945, @jrtc27 wrote:

Or, like WITNESS or MALLOC_PRODUCTION, on in HEAD and off in stable branches?

My thinking here is few of us are LLVM developers, and it is not clear this is a benefit to apply broadly. I am less certain on where MALLOC_PRODUCTION should fall, if the thought is it catches src errors than that is a favor for keeping it on in HEAD. WITNESS is directly relevant to HEAD development and the pkgbase now makes it easy to opt out.

[kbowling]

I'm planning to commit this soon unless a serious objection is raised.

[emaste]

I have no objection to this general idea but defer to @dim. I do think that there's value in having LLVM_ASSERTIONS enabled in head and disabled in stable branches in the coverage it provides in the window before the compiler makes it to the stable branch.

Oh, LLVM_ASSERTIONS support arrived in 147d7b567f03f7e327b9bd2fa5bec6077022aeeb with the note:

For head/, this will remain eternally default-on to maintain the status quo.
For stable/ branches, it should be flipped to default-off to maintain the
status quo.

Adding @kevans to reviewers list

[kevans]

I don't really object to it defaulting off, but I think I'd want the cluster ports builders to keep it on (so we'll need to poke pkgmgr@). We have discovered llvm bugs in the past from random ports tripping assertions, and there's some value in still being able to discover that via exp-runs and pkg-fallout.

[dim]

The idea behind this being on in -CURRENT is to catch more errors. The same really holds for WITNESS and other assertions, but you could argue that those are more for the core of the system, while llvm-project is contributed software. Note that we caught many assertions since they are enabled by default, so they definitely have value.

In D50388#1155400, @kevans wrote:

I don't really object to it defaulting off, but I think I'd want the cluster ports builders to keep it on (so we'll need to poke pkgmgr@). We have discovered llvm bugs in the past from random ports tripping assertions, and there's some value in still being able to discover that via exp-runs and pkg-fallout.

For fixing bugs in the toolchain, that is indeed extremely useful. When assertions are off, sometimes the compiler or linker will simply segfault, but in many other cases there might be bad code silently emitted, or some other nastiness.

That said, if people are really so annoyed by this, it may make sense to turn them off by default. But you could then also say that for MALLOC_PRODUCTION, for example. Why make everybody's memory allocations slow, while you really only intend to debug the core system? :)

[kevans]

In D50388#1155608, @dim wrote:

In D50388#1155400, @kevans wrote:

I don't really object to it defaulting off, but I think I'd want the cluster ports builders to keep it on (so we'll need to poke pkgmgr@). We have discovered llvm bugs in the past from random ports tripping assertions, and there's some value in still being able to discover that via exp-runs and pkg-fallout.

For fixing bugs in the toolchain, that is indeed extremely useful. When assertions are off, sometimes the compiler or linker will simply segfault, but in many other cases there might be bad code silently emitted, or some other nastiness.

That said, if people are really so annoyed by this, it may make sense to turn them off by default. But you could then also say that for MALLOC_PRODUCTION, for example. Why make everybody's memory allocations slow, while you really only intend to debug the core system? :)

I'd argue a little harder for MALLOC_PRODUCTION staying as-is since that's more broadly applicable. Though, we have src.conf knobs for both of these exactly so that folks can just flip them easily for their own use-cases without having to constantly argue defaults, and the current defaults reflect generally what we do with debug features in the kernel (as you noted).

[dim]

Okay, let's change the default then. It would be nice if you can put a reminder in the commit message that says you have to turn on LLVM_ASSERTIONS whenever you want to report an issue with the toolchain itself.

[kbowling]

@kevans were you able to get any reaction from portmgr@?

[kevans]

In D50388#1177748, @kbowling wrote:

@kevans were you able to get any reaction from portmgr@?

No, sorry, I haven't had a chance to draft anything yet; I thought I had dropped a 'go ahead and do this, I will follow up on the cluster side' comment, but clearly not- sorry.

[kbowling]

In D50388#1177749, @kevans wrote:

In D50388#1177748, @kbowling wrote:

@kevans were you able to get any reaction from portmgr@?

No, sorry, I haven't had a chance to draft anything yet; I thought I had dropped a 'go ahead and do this, I will follow up on the cluster side' comment, but clearly not- sorry.

Np, I'll write an UPDATING message with @dim's note above and send it.

[markmi_dsl-only.net]

I'd argue a little harder for MALLOC_PRODUCTION staying as-is since that's more broadly applicable. Though, we have src.conf knobs for both of these exactly so that folks can just flip them easily for their own use-cases without having to constantly argue defaults, and the current defaults reflect generally what we do with debug features in the kernel (as you noted).

I will note that as of pkg 2.1+ being involved, including 2.2.1, main-*, such as main-amd64 on beefy18 and main-arm64 on ampere2, take significantly longer than the others to finish bulk -Ca like builds on those machines. (This is in comparison to pkg 2.0.6 .) Think Pkgs_built_so_far/Elapsed_time_so_far decreasing rather systematically. The main-arm64 and main-armv7 cycle on ampere2 now takes about a month, so each is being updated only about 12 times a year for the rate, as stands. Not exactly good for keeping the security status up to date.

Also, it is worse than that long term: the more builds that have completed already, the longer the total for the stages build-depends, lib-depends, and run-depends when a later package has dependencies. With 10s of thousands of smaller-to-build packages, that trend is not viable long term. It is not just or primarily the big-build jobs that are taking more overall time: it is the increasing total across the vast number of smaller-to-build packages. (bapt has an idea for eventually using a different technique.)

In my view, having distinct builders what is to be distributed vs. doing builds for testing would help, such as using beefy17 as a second main-amd64 once it stops handling main-i386 might be a useful example. (ampere2 is not so easy to suggest an example for without getting more aarch64 system(s).)

Anyway, I think there are definite tradeoffs with using WITHOUT_MALLOC_PRODUCTION for production on the scale of building the likes of 36,000 packages for distribution.

[jrtc27]

In D50388#1178050, @markmi_dsl-only.net wrote:

I'd argue a little harder for MALLOC_PRODUCTION staying as-is since that's more broadly applicable. Though, we have src.conf knobs for both of these exactly so that folks can just flip them easily for their own use-cases without having to constantly argue defaults, and the current defaults reflect generally what we do with debug features in the kernel (as you noted).

I will note that as of pkg 2.1+ being involved, including 2.2.1, main-*, such as main-amd64 on beefy18 and main-arm64 on ampere2, take significantly longer than the others to finish bulk -Ca like builds on those machines. (This is in comparison to pkg 2.0.6 .) Think Pkgs_built_so_far/Elapsed_time_so_far decreasing rather systematically. The main-arm64 and main-armv7 cycle on ampere2 now takes about a month, so each is being updated only about 12 times a year for the rate, as stands. Not exactly good for keeping the security status up to date.

Also, it is worse than that long term: the more builds that have completed already, the longer the total for the stages build-depends, lib-depends, and run-depends when a later package has dependencies. With 10s of thousands of smaller-to-build packages, that trend is not viable long term. It is not just or primarily the big-build jobs that are taking more overall time: it is the increasing total across the vast number of smaller-to-build packages. (bapt has an idea for eventually using a different technique.)

In my view, having distinct builders what is to be distributed vs. doing builds for testing would help, such as using beefy17 as a second main-amd64 once it stops handling main-i386 might be a useful example. (ampere2 is not so easy to suggest an example for without getting more aarch64 systems.)

Anyway, I think there are definite tradeoffs with using WITHOUT_MALLOC_PRODUCTION for production on the scale of building the likes of 36,000 packages for distribution.

This is not the place for this discussion. Please don't pollute this review with it.