[native_toolchain_c] Optimization level

[dcharkes]

Using the CBuilder in a build hook does currently not set an optimization flags for the C compiler. This means that by default C/C++ code gets compiled unoptimized, which is a rather bad gotcha in release builds.

By comparison, the plugin_ffi template compiles C/C++ code optimized in Flutter release builds and unoptimized in development runs (which also turns out to be a gotcha as illustrated by @SaltySpaghetti at #ftcon24eu).

Wishlist:

• A param for CBuilder for optimization level, enum-style or opaque style that is automatically mapped to the different compilers. This is more discoverable than compiler flags.
• The param should be nullable or have an explicitly OptimizationLevel.unspecified to be able to pass the optimization level in compiler-specific compiler flags with CBuilder.flags.
• If implicitly unspecified CBuilder constructor, the opt-level should probably be -o3 for BuildMode.release and -o1 for BuildMode.debug. (Dart standalone is always BuildMode.release at the moment, Flutter passes it's --release and --debug.)

Any other suggestions welcome.

Test requirements:

• I don't know if we can inspect a dylib to see with what opt level it was produced. If that's not possible, then at least we can check that the binaries with different opt-levels are not equal in tests.

@knopp @csnewman Maybe we should introduce something similar for the Rust and Go builders.

[dcharkes]

Note from @mkustermann: It should probably be -o3 optimized by default always. Rationale: with transitive dependencies you want all those transitive dependencies to be -o3. Only when developing the package itself you might want -o1.

[knopp]

I don't mind optimizing by default, but maybe we should have at least an option to do -Os. In my experience the performance difference is in most cases negligible and the binary size, especially on mobile, matters.

As for Rust, the flags for profiles (debug, release) are configured inside Cargo.toml, the builder should not interfere with that other than choosing the profile. Right now it builds debug build for debug configurations, which we might want to change to build release always.

[dcharkes]

maybe we should have at least an option to do -Os. In my experience the performance difference is in most cases negligible and the binary size, especially on mobile, matters.

Great idea. Yeah so an enum with descriptive messages makes sense. (We don't really have to share them across the different native_toolchain packages, but it would be good if they're called the same.)

Right now it builds debug build for debug configurations, which we might want to change to build release always.

Yeah, otherwise we get the same gotcha as with the C++ program in flutter run (which is debug by default).

[mkustermann]

Whatever compiler flags dart build and flutter build may pass down to individual hook/build.dart scripts. The hook scripts themselves will use a helper package e.g. package:native_toolchain_c.

Now dart build and flutter build may have a preference, quite possibly -Os (in our Dart AOT compiler we also favor size over speed). But the individual hook/build.dart script (that uses e.g. package:native_toolchain_c) still has the last say and can decide that for that particular native library it prefers to use -O3 over -Os.

The API in e.g. package:native_toolchain_c can either allow arbitrary extra flags to be passed to it, or (if it wants to abstract entirely over clang/gcc/msvc/... differences) - via an OptimizationLevel.<> enum.

We discussed offline also that hook/build.dart should know whether we compile for development mode or production mode. So again, the package's hook/build.dart could decide to deviate from the default flags that dart build / flutter build pass and do something else in debug mode.

[knopp]

Great idea. Yeah so an enum with descriptive messages makes sense. (We don't really have to share them across the different native_toolchain packages, but it would be good if they're called the same.)

For the Rust builder it doesn't make sense to have an OptimizationLevel option, since that is specified per profile inside Cargo.toml. So I'd vote for placing that into native_toolchain_c.

I agree with @mkustermann that the build.dart should have the last word and know the current profile we're building for - I think that's already the case.

I think the question here is - should the native_toolchain_c have a option for convenienty specify optimization level (I'd think so), and should the default be -O3 or -Os regardless of whether we build for debug or release.

I think especially on Windows if we build the native library with MSVC the debug builds tend to perform much worse than clang and the likelyhood of end-user needed to debug a native assets are quite low.

[dcharkes]

Whatever compiler flags dart build and flutter build may pass down to individual hook/build.dart scripts.

The BuildConfig and LinkConfig don't have an arbitrary compilerFlags field. Maybe it should? Currently, there's only BuildMode.release/debug.

The hook scripts themselves will use a helper package e.g. package:native_toolchain_c.

Now dart build and flutter build may have a preference, quite possibly -Os (in our Dart AOT compiler we also favor size over speed). But the individual hook/build.dart script (that uses e.g. package:native_toolchain_c) still has the last say and can decide that for that particular native library it prefers to use -O3 over -Os.

So maybe we should add OptimizationLevel to the BuildConfig/LinkConfig to be consumed by the builder packages then.

Then the XBuilder.optimizationLevel should be nullable, and if it's provided it overrides the XConfig.optimizationLevel.

The API in e.g. package:native_toolchain_c can either allow arbitrary extra flags to be passed to it, or (if it wants to abstract entirely over clang/gcc/msvc/... differences) - via an OptimizationLevel.<> enum.

We discussed offline also that hook/build.dart should know whether we compile for development mode or production mode. So again, the package's hook/build.dart could decide to deviate from the default flags that dart build / flutter build pass and do something else in debug mode.

👍

[knopp]

Technically we can override cargo profile optimisation levels specified in Cargo.toml through environment variables (i.e. CARGO_PROFILE_DEV_OPT_LEVEL), but for me as a package author that'd be quite unexpected if the builder did that.

[dcharkes]

Yeah, so for rust it makes maybe more sense to use profiles. Profiles are user-defined. So that means the RustBuilder probably needs a String profile param.

That then also means that it would need a (HookConfig.optimizationLevel) => profile callback or map if we want Dart/Flutter to be able to chose between -Os and -O3.

[mkustermann]

The BuildConfig and LinkConfig don't have an arbitrary compilerFlags field. Maybe it should? Currently, there's only BuildMode.release/debug.

If we (in the future) want the ability to statically link the compiled dart app together with native code from packages, we'd need to ensure the code is compiled in a compatible way (same toolchain, sysroot, ... ?). If one then wants to allow tree shaking the result, one has to pass special flags to compiler & linker (e.g. by passing -ffunction-section or -flto to both compiler & linker).

We may want to support that use case via a different asset kind (which we can introduce in the future) but we should ensure the system is flexible enough to handle that (i.e. a way to pass this compiler-related information down to hook). Of course not every hook/build.dart has to use this mechanism - we'll always support dynamic libraries.

[dcharkes]

I think for now we can just add a class to package:native_toolchain_c:

/// Optimization level for code compilation.
final class OptimizationLevel {
  /// The optimization level.
  final String _level;

  const OptimizationLevel._(this._level);

  /// No optimization; prioritize fast compilation.
  static const OptimizationLevel O0 = OptimizationLevel._('O0');

  /// Basic optimizations; balance compilation speed and code size.
  static const OptimizationLevel O1 = OptimizationLevel._('O1');

  /// More aggressive optimizations; prioritize code size reduction.
  static const OptimizationLevel O2 = OptimizationLevel._('O2');

  /// The most aggressive optimizations; prioritize runtime performance.
  static const OptimizationLevel O3 = OptimizationLevel._('O3');

  /// Optimize for code size, even if it impacts runtime performance.
  static const OptimizationLevel Os = OptimizationLevel._('Os');

  /// Optimize aggressively for code size, potentially at the cost of
  /// compilation time and debugging capabilities.
  static const OptimizationLevel Oz = OptimizationLevel._('Oz');

  /// Unspecified optimization level; the default or compiler-chosen level.
  static const OptimizationLevel unspecified = OptimizationLevel._('unspecified');

  /// Returns the string representation of the optimization level.
  @override
  String toString() => _level;
}

Add it as optional parameter to CBuilder constructors defaulting to O3. And pass it into the compiler process invocation commandline arguments for both clang-like and Windows compiler (note MSVC uses different flags). And then also add some tests.

We can add Dart/Flutter passing in a preferred optimization level in the BuildConfig later.

Then we at least (1) by default compile fast, and (2) provide a convenient way for developers to change the optimization level when working on their own package hook.

[knopp]

This looks good to me, even though I'd probably prefer -Os to -O3 as default. Though not a hill I'd die on. I think it's a better balance to have as a default value and package author can always chose O3 if the package is performance sensitive.

[dcharkes]

This looks good to me, even though I'd probably prefer -Os to -O3 as default. Though not a hill I'd die on. I think it's a better balance to have as a default value and package author can always chose O3 if the package is performance sensitive.

Maybe we should not have a const value in the parameter-list and default to -Os for mobile and -O3 for desktop OSes.

[SaltySpaghetti]

Sorry if I step in, but I agree with @knopp since imho the optimization shouldn't be platform dependent but it changes depending on the purpose of your code

[dcharkes]

I don't have a super strong opinion about the default. Users can always specify it in the CBuilder constructor. Happy to receive a patch with either one as default :)

[dcharkes]

I think we should do #1267 (comment), preferably before FlutterconUSA. Otherwise, the next group of enthusiasts is going to be wondering why their native code is slow.

Happy to receive a PR, otherwise I'll do it.