Skip to content

Commit

Permalink
postgresql: implement opt-in JIT support
Browse files Browse the repository at this point in the history
Closes NixOS#150801

Note: I decided against resuming directly on NixOS#150801 because the
conflict was too big (and resolving it seemed too error-prone to me).
Also the `this`-refactoring could be done in an easier manner, i.e. by
exposing JIT attributes with the correct configuration. More on that
below.

This patch creates variants of the `postgresql*`-packages with JIT[1]
support. Please note that a lot of the work was derived from previous
patches filed by other contributors, namely dasJ, andir and abbradar,
hence the co-authored-by tags below.

Effectively, the following things have changed:

* For JIT variants an LLVM-backed stdenv with clang is now used as
  suggested by dasJ[2]. We need LLVM and CLang[3] anyways to build the
  JIT-part, so no need to mix this up with GCC's stdenv. Also, using the
  `dev`-output of LLVM and clang's stdenv for building (and adding llvm
  libs as build-inputs) seems more cross friendly to me (which will
  become useful when cross-building for JIT-variants will actually be
  supported).

* Plugins inherit the build flags from the Makefiles in
  `$out/lib/pgxs/src` (e.g. `-Werror=unguarded-availability-new`). Since
  some of the flags are clang-specific (and stem from the use of the
  CLang stdenv) and don't work on gcc, the stdenv of `pkgs.postgresql`
  is passed to the plugins. I.e., plugins for non-JIT variants are built
  with a gcc stdenv on Linux and plugins for JIT variants with a clang
  stdenv.

  Since `plv8` hard-codes `gcc` as `$CC` in its Makefile[4], I marked it
  as broken for JIT-variants of postgresql only.

* Added a test-matrix to confirm that JIT works fine on each
  `pkgs.postgresql_*_jit` (thanks Andi for the original test in
  NixOS#124804!).

* For each postgresql version, a new attribute
  `postgresql_<version>_jit` (and a corresponding
  `postgresqlPackages<version>JitPackages`) are now exposed for better
  discoverability and prebuilt artifacts in the binary cache.

* In NixOS#150801 the `this`-argument was replaced by an internal recursion.
  I decided against this approach because it'd blow up the diff even
  more which makes the readability way harder and also harder to revert
  this if necessary.

  Instead, it is made sure that `this` always points to the correct
  variant of `postgresql` and re-using that in an additional
  `.override {}`-expression is trivial because the JIT-variant is
  exposed in `all-packages.nix`.

* I think the changes are sufficiently big to actually add myself as
  maintainer here.

* Added `libxcrypt` to `buildInputs` for versions <v13. While
  building things with an LLVM stdenv, these versions complained that
  the extern `crypt()` symbol can't be found. Not sure what this is
  exactly about, but since we want to switch to libxcrypt for `crypt()`
  usage anyways[5] I decided to add it. For >=13 it's not relevant
  anymore anyways[6].

* JIT support doesn't work with cross-compilation. It is attempted to
  build LLVM-bytecode (`%.bc` is the corresponding `make(1)`-rule) for
  each sub-directory in `backend/` for the JIT apparently, but with a
  $(CLANG) that can produce binaries for the build, not the host-platform.

  I managed to get a cross-build with JIT support working with
  `depsBuildBuild = [ llvmPackages.clang ] ++ buildInputs`, but
  considering that the resulting LLVM IR isn't platform-independent this
  doesn't give you much. In fact, I tried to test the result in a VM-test,
  but as soon as JIT was used to optimize a query, postgres would
  coredump with `Illegal instruction`.

A common concern of the original approach - with llvm as build input -
was the massive increase of closure size. With the new approach of using
the LLVM stdenv directly and patching out references to the clang drv in
`$out` the effective closure size changes are:

    $ nix path-info -Sh $(nix-build -A postgresql_14)
    /nix/store/kssxxqycwa3c7kmwmykwxqvspxxa6r1w-postgresql-14.7	306.4M
    $ nix path-info -Sh $(nix-build -A postgresql_14_jit)
    /nix/store/xc7qmgqrn4h5yr4vmdwy56gs4bmja9ym-postgresql-14.7	689.2M

Most of the increase in closure-size stems from the `lib`-output of
LLVM

    $ nix path-info -Sh /nix/store/5r97sbs5j6mw7qnbg8nhnq1gad9973ap-llvm-11.1.0-lib
    /nix/store/5r97sbs5j6mw7qnbg8nhnq1gad9973ap-llvm-11.1.0-lib	349.8M

which is why this shouldn't be enabled by default.

While this is quite much because of LLVM, it's still a massive
improvement over the simple approach of adding llvm/clang as
build-inputs and building with `--with-llvm`:

    $ nix path-info -Sh $(nix-build -E '
	with import ./. {};
	postgresql.overrideAttrs ({ configureFlags ? [], buildInputs ? [], ... }: {
	  configureFlags = configureFlags ++ [ "--with-llvm" ];
	  buildInputs = buildInputs ++ [ llvm clang ];
	})' -j0)
    /nix/store/i3bd2r21c6c3428xb4gavjnplfqxn27p-postgresql-14.7	  1.6G

Co-authored-by: Andreas Rammhold <[email protected]>
Co-authored-by: Janne Heß <[email protected]>
Co-authored-by: Nikolay Amiantov <[email protected]>

[1] https://www.postgresql.org/docs/current/jit-reason.html
[2] NixOS#124804 (comment)
    & NixOS#150801 (comment)
[3] This fails with the following error otherwise:
    ```
    configure: error: clang not found, but required when compiling --with-llvm, specify with CLANG=
    ```
[4] https://github.com/plv8/plv8/blob/v3.1.5/Makefile#L14
[5] NixOS#181764
[6] postgres/postgres@c45643d
  • Loading branch information
Ma27 committed Mar 29, 2023
1 parent 46c5c6e commit 2282fa7
Show file tree
Hide file tree
Showing 8 changed files with 227 additions and 64 deletions.
11 changes: 11 additions & 0 deletions nixos/doc/manual/release-notes/rl-2305.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,17 @@ In addition to numerous new and upgraded packages, this release has the followin

- `services.chronyd` is now started with additional systemd sandbox/hardening options for better security.

- PostgreSQL has opt-in support for [JIT compilation](https://www.postgresql.org/docs/current/jit-reason.html). It can be enabled like this:
```nix
{
services.postgresql = {
enable = true;
package = pkgs.postgresql_jit;
settings."jit" = "on";
};
}
```

- `services.dhcpcd` service now don't solicit or accept IPv6 Router Advertisements on interfaces that use static IPv6 addresses.

- The module `services.headscale` was refactored to be compliant with [RFC 0042](https://github.com/NixOS/rfcs/blob/master/rfcs/0042-config-option.md). To be precise, this means that the following things have changed:
Expand Down
1 change: 1 addition & 0 deletions nixos/tests/all-tests.nix
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ in {
postfixadmin = handleTest ./postfixadmin.nix {};
postgis = handleTest ./postgis.nix {};
postgresql = handleTest ./postgresql.nix {};
postgresql-jit = handleTest ./postgresql-jit.nix {};
postgresql-wal-receiver = handleTest ./postgresql-wal-receiver.nix {};
powerdns = handleTest ./powerdns.nix {};
powerdns-admin = handleTest ./powerdns-admin.nix {};
Expand Down
50 changes: 50 additions & 0 deletions nixos/tests/postgresql-jit.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{ system ? builtins.currentSystem
, config ? {}
, pkgs ? import ../.. { inherit system config; }
}:

with import ../lib/testing-python.nix { inherit system pkgs; };

let
inherit (pkgs) lib;
packages = lib.filter
(lib.hasSuffix "_jit")
(builtins.attrNames (import ../../pkgs/servers/sql/postgresql pkgs));

mkJitTest = packageName: makeTest {
name = "${packageName}";
meta.maintainers = with lib.maintainers; [ ma27 ];
nodes.machine = { pkgs, lib, ... }: {
services.postgresql = {
enable = true;
package = pkgs.${packageName};
settings.jit = "on";
initialScript = pkgs.writeText "init.sql" ''
create table demo (id int);
insert into demo (id) select generate_series(1, 5);
'';
};
};
testScript = ''
machine.start()
machine.wait_for_unit("postgresql.service")
with subtest("JIT is enabled"):
machine.succeed("sudo -u postgres psql <<<'show jit;' | grep 'on'")
with subtest("Test JIT works fine"):
output = machine.succeed(
"cat ${pkgs.writeText "test.sql" ''
set jit_above_cost = 1;
EXPLAIN ANALYZE SELECT CONCAT('jit result = ', SUM(id)) FROM demo;
SELECT CONCAT('jit result = ', SUM(id)) from demo;
''} | sudo -u postgres psql"
)
assert "JIT:" in output
assert "jit result = 15" in output
machine.shutdown()
'';
};
in
lib.genAttrs packages mkJitTest
2 changes: 1 addition & 1 deletion nixos/tests/postgresql-wal-receiver.nix
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,4 @@ let
};

# Maps the generic function over all attributes of PostgreSQL packages
in builtins.listToAttrs (map makePostgresqlWalReceiverTest (builtins.attrNames (import ../../pkgs/servers/sql/postgresql { })))
in builtins.listToAttrs (map makePostgresqlWalReceiverTest (builtins.attrNames (import ../../pkgs/servers/sql/postgresql pkgs)))
Loading

0 comments on commit 2282fa7

Please sign in to comment.