Support debug-only code

Currently in SwiftNIO we have a small amount of code that we want to insert into debug builds only. This code keeps track of allocation locations of futures and promises to help track down tricky concurrency bugs, and is extremely valuable in debugging, but adds a substantial performance overhead. For this purpose, we have been guarding the code with _isDebugAssertConfiguration.

In addition to this being an underscore-prefixed function, this produces the very unpleasant side-effect of compiler warnings when building in release mode, as the code in the block is unreachable. Now, this is absolutely the intended effect, so these warnings are nothing more than noise that we cannot disable.

Right now we plan to work around this issue by using a side-effect of assert, like so:

func debugOnly(_ body: () -> Void) {
    assert({ body(); return true }())
}

However, this reliance on assert side effects is clearly not good. Is there any appetite to provide something to do this more cleanly?

8 Likes

I don’t understand. Why can’t you just pass flags to the build and check using #if DEBUG or something similar. Even if all of your debug code lives in functions, I think the optimizer will optimize away empty functions in release mode.

Lots of reasons, but the main one is because my project is open source using SwiftPM. To pass such a flag in SwiftPM requires the following ugly invocation: swift test -Xswiftc "-D DEBUG. Additionally, it requires that all my users remember to pass such a flag, not only when building my framework, but also when building their own tools that link my framework.

The requirement to remember to do this in all dependent code basically means that no-one will ever do it, which reduces the utility dramatically.

2 Likes

Maybe because one may want to reuse the same flag that controls if asserts are compiled or not. Without adding an extra -D DEBUG option. This flag exists, this thread is about exposing it in one way or another.

2 Likes

I don’t think it’s a good idea to modify the language in order to work around spm’s many shortcomings. However, @gwendal.roue has clarified what you were really asking for here, and I’d agree that it’s useful. Perhaps you could be more specific about which conditions you want to check for? Asserts enabled, debug optimization set? Something as simple as a few built in conditions may work unless you want something more complex.#if DEBUG_OPTIMIZATION or #if ASSERTS_ENABLED could be the base feature, which users could wrap in their own functions if they want something cleaner.

2 Likes

Seconded: adding -D DEBUG to the build system works, but this ad hoc measure is mildly burdensome.

I wonder, though, if DEBUG should be orthogonal to the optimized build settings that control assert(…)? Hmm. The uses @lukasa describes in the original post similar in spirit to assertions: sanity checks that theoretically do not change behavior, but flag problems at the cost of performance. However, other uses of DEBUG might include things like:

  • additional logging,
  • pointing at a dev/sandbox server,
  • additional in-app UI for diagnostics, and
  • the ability to induce special conditions for testing.

All of these are things one might want enabled in an optimized build to track down an “only happens in optimized builds” bug.

This makes me like @Jon_Shier’s idea of having compilation conditions named around whether assertions are enabled, whether optimization is on, etc., and leaving the more general DEBUG flag app-specific, or at least an orthogonal concern.

1 Like

The question raised before is “what is debug”, and I think the answer we came up with is to use builds where asserts can fire. More here: https://gist.github.com/erica/d20639b409fe1b318c0e.

1 Like

I’d be quite happy with that as an option: it roughly matches the semantic I’m going for.

I think _isDebugAssertConfiguration is the right test to promote. If an assert will not normally fire, then it’s “debug mode”. Calling it “debug” directly, however, is problematic because it is promising something it is not delivering (there is no check for a debug scheme). I also think this is best as a build configuration test and not as a statement.

Existing build configuration tests include os() operating system, arch() target architecture, swift() language version, canImport() module availability, and targetEnvironment() test for simulator. Disabled asserts is not a target environment and none of the other existing tests apply. There are three assert configurations: debug assert, release, and fast. If the condition is built around assert, you could have: assertConfiguration(debug), assertConfiguration(fast), and assertConfiguration(release), which could get the job done.

Would you want a better name than that and would Swift users be okay with that kind of naming?

1 Like

I’m happy enough with that naming. Obviously I can’t speak for the wider Swift community.

This is an interesting proposal for us as well. I like the idea of having a compiler directive for it.

My main issue with the proposal is I’m not sure what an “assertConfiguration” is. Especially if one of the options is “fast”.

My dev-facing options in 99% of use cases stem from Xcode and SPM: both of those have (admittedly alterable) build configurations, namely Debug and Release. I’d propose that almost all Swift devs have at least some familiarity with these two concepts, so it’d make sense to use them here.

As mentioned elsewhere in this thread though, if the issue is SPM, maybe it’d make the most sense to give SPM a higher-level understanding of BUILD_CONFIGURATION (as Xcode has) instead. We could then allow compiler flags (among other things?) to be set depending on which configuration is active.

PSA: There is a parallel pitch thread discussing conditional compilation: Unified way for target platform condition, which includes configuration.

FWIW, this will solve the SwiftPM issue.

1 Like

Last night @johnno1962 got a prototype working. Although there are three available modes (debug, release, fast), I think the only one of sufficient value or interest is checking for -Onone, aka #if assertConfiguration(debug).

Today I managed to get a cleaner/slimmer prototype working, again with huge assist from @johnno1962. This remains a proof of concept.

The updated proposal is here: https://gist.github.com/erica/d20639b409fe1b318c0e

The big difference is that the test has changed to testing for optimization instead of testing for “debug” conditions. This was done out of practical necessity. There are several times the target conditions are set (and each time reset) before the command line arguments are parsed. This encouraged me to go with a “trap” that once triggered optimization(enabled).

Here are my prototype diffs: https://github.com/apple/swift/compare/master...erica:optcheck

Here is what the tests look like running on a short sample file using a variety of optimization flags:

% ./swift testAssertConfigurations.swift 
Starting tests
asserts will fire. unoptimized/debug
asserts will fire. unoptimized/debug
Ending tests
% ./swift -O testAssertConfigurations.swift 
Starting tests
asserts will not fire. optimized
asserts will not fire. optimized
Ending tests
% ./swift -Onone testAssertConfigurations.swift 
Starting tests
asserts will fire. unoptimized/debug
asserts will fire. unoptimized/debug
Ending tests
% ./swift -Ounchecked testAssertConfigurations.swift 
Starting tests
asserts will not fire. optimized
asserts will not fire. optimized
Ending tests
% cat testAssertConfigurations.swift 
// Test compilation with  -O, -Onone, and -Ounchecked

print("Starting tests")

#if optimization(enabled)
    print("asserts will not fire. optimized")
#else
    print("asserts will fire. unoptimized/debug")
#endif

#if !optimization(enabled)
    print("asserts will fire. unoptimized/debug")
#else
    print("asserts will not fire. optimized")
#endif

print("Ending tests")

If you have suggestions or feedback, now would be a very good time to offer them. Thank you in advance.

How do we account for Joe Groff’s point raised in the original pitch?

Joe Groff writes, “We specifically avoided making debug/release an #if
condition because we considered #if to be the wrong point at which to start
conditionalizing code generation for assertions. Though the final
executable image’s behavior is unavoidably dependent on whether asserts are
enabled, we didn’t want the SIL for inlineable code to be, since that would
mean libraries with inlineable code would need to ship three times the
amount of serialized SIL to support the right behavior in -Onone, -O, and
-Ounchecked builds.”

Thank you for bringing that up. Some points:

  • Joe’s statement, which I had in the earlier draft for reference, will be brought back and it will be joined by talking about promoting the three existing in-language checks from private to public. (The downside I see is that conditional code remains in the build unlike conditional compilation.)
  • Before I get there, there’s work to be done on deciding exactly what is being proposed and evaluating whether @johnno1962 and I want to invest the significant time and emotional effort needed to pushing this towards full proposal status.
  • This is a draft, not a final proposal or final code. I haven’t gotten close to the point where it is ready to be a pull release. I’m exploring the space by participating in this Swift Evolution thread.Feedback that’s most valuable right now will focus on whether this is the right direction and the right approach for Swift. I will work on updating supporting (and dissenting) arguments after we figure out what it is that will be proposed.
  • I do not believe it is a foregone conclusion that the core team will reject a test that effectively checks for optimization conditions, leading to a way to insert debug-only code, which is how I think Joe’s statement can be misread.

And here is a revised version of the proposal with a good deal of fleshing out. There’s still more to do and everyone’s feedback is appreciated.

Hello ! I enjoyed the reading. May I ask a question? Why a runtime test? Couldn’t the given example (switching between two urls) be done with compile-time checks as well?

It could indeed. However, a run-time check is much shorter an on-point. Compare:

#if optimization(enabled)
    // release URL asset
    let urlString = releaseURLString
#else
    // debug URL asset
    let urlString = debugURLString
#endif
let url: URL = URL(string: urlString)!

/// or

#if optimization(enabled)
    let url: URL = URL(string: releaseURLString)!
#else
    let url: URL = URL(string: debugURLString)!
#endif

versus

let urlString = isOptimized() ? releaseURLString : debugURLString
let url: URL = URL(string: urlString)!

// or even

let url: URL = URL(string: isOptimized() ? releaseURLString : debugURLString)!

The runtime version is short and to the point. You’d definitely want to use compile-time if the debug URL were sensitive.