Skip to content

Commit

Permalink
Stop statically linking binaries when building with Nix. (#126)
Browse files Browse the repository at this point in the history
### What

It turns out that statically linking with musl increases our response
time under load from (in a very specific benchmark, on my computer)
~10ms to ~30ms per response. Disabling static linking fixes this issue,
bringing ndc-postgres in line with HGE v2.

This feels like a good enough reason not to statically link the code.

We ship the Docker image built with Nix, so this directly impacts
production. It also simplifies development (in theory) because `nix
build` and `cargo build` now have behavior that is more similar.

### How

I deleted the Nix code that turns on static linking.
  • Loading branch information
SamirTalwar authored Nov 1, 2023
1 parent 0225728 commit b5cf0ad
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 35 deletions.
29 changes: 5 additions & 24 deletions nix/cargo-build.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# This is a function that returns a derivation for the compiled Rust project.
# Supports cross-compiling, and statically-links builds compiled for Linux.
# Supports cross-compiling.
#
# The derivation includes some extra attributes with some of the resources that
# were used to produce the derivation:
Expand All @@ -15,7 +15,6 @@
# these special attributes will be given:
#
# - craneLib - see https://crane.dev/API.html#cranelibbuildpackage
# - staticallyLinked - boolean to indicate whether we are linking statically
#
crateExpression
, nixpkgs
Expand All @@ -37,28 +36,13 @@ let
buildPlatform = pkgs.stdenv.buildPlatform;
hostPlatform = pkgs.stdenv.hostPlatform;

# We can statically-link if the cross-compilation target is Linux, and the
# build system is 64-bit because these are the conditions where Nix has musl
# packages available.
staticallyLinked = hostPlatform.isLinux && buildPlatform.is64bit;

# `config` is a cpu-vendor-os-abi string like "aarch64-unknown-linux-gnu". To
# statically link we need to change the ABI from "gnu" to "musl".
#
# Ideally we would pass in a crossSystem argument of the form
# "aarch64-unknown-linux-musl" instead of doing this fixup. But that doesn't
# seem to work.
buildTarget =
if staticallyLinked then builtins.replaceStrings [ "gnu" ] [ "musl" ] hostPlatform.config
else hostPlatform.config;

# When possibly cross-compiling we get several versions of nixpkgs of the
# form, `pkgs.pkgs<where it runs><platform it produces outputs for>`. We use
# `pkgs.pkgsBuildHost` to get packages that run at build time (so run on the
# build platform), and that produce outputs for the host platform which is the
# cross-compilation target.
rustToolchain = (pkgs.pkgsBuildHost.rust-bin.fromRustupToolchainFile ../rust-toolchain.toml).override {
targets = [ buildTarget ];
targets = [ hostPlatform.config ];
};

lib = pkgs.pkgsBuildHost.lib;
Expand All @@ -69,24 +53,21 @@ let
envCase = triple: lib.strings.toUpper (builtins.replaceStrings [ "-" ] [ "_" ] triple);

buildArgs = {
CARGO_BUILD_TARGET = buildTarget;
"CARGO_TARGET_${envCase buildTarget}_LINKER" = "${pkgs.stdenv.cc.targetPrefix}cc";
CARGO_BUILD_TARGET = hostPlatform.config;
"CARGO_TARGET_${envCase hostPlatform.config}_LINKER" = "${pkgs.stdenv.cc.targetPrefix}cc";

# This environment variable may be necessary if any of your dependencies use
# a build-script which invokes the `cc` crate to build some other code. The
# `cc` crate should automatically pick up on our target-specific linker
# above, but this may be necessary if the build script needs to compile and
# run some extra code on the build system.
HOST_CC = "${pkgs.stdenv.cc.nativePrefix}cc";
}
// lib.optionalAttrs staticallyLinked {
CARGO_BUILD_RUSTFLAGS = "-C target-feature=+crt-static";
};

# Call the given crateExpression to build the crate - or rather to get
# a derivation that will build the crate.
crate = pkgs.callPackage crateExpression {
inherit craneLib staticallyLinked binary-name;
inherit craneLib binary-name;
};
in
# Override the derivation to add cross-compilation and static linking environment variables.
Expand Down
11 changes: 0 additions & 11 deletions nix/ndc-agent.nix
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
# https://crane.dev/API.html#cranelibbuildpackage
#
{ craneLib
, staticallyLinked
, lib
, openssl
, libiconv
Expand Down Expand Up @@ -52,16 +51,6 @@ let
pkg-config # required for non-static builds
protobuf # required by opentelemetry-proto, a dependency of axum-tracing-opentelemetry
];

} // lib.optionalAttrs staticallyLinked {
# Configure openssl-sys for static linking. The build script for the
# openssl-sys crate requires openssl lib and include locations to be
# specified explicitly for this case.
#
# `pkgsStatic` provides versions of nixpkgs that are compiled with musl
OPENSSL_STATIC = "1";
OPENSSL_LIB_DIR = "${pkgsStatic.openssl.out}/lib";
OPENSSL_INCLUDE_DIR = "${pkgsStatic.openssl.dev}/include";
};

cargoArtifacts = craneLib.buildDepsOnly buildArgs;
Expand Down

0 comments on commit b5cf0ad

Please sign in to comment.