Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix jemalloc linking in CI #394

Merged
merged 14 commits into from
May 24, 2024
Merged

fix jemalloc linking in CI #394

merged 14 commits into from
May 24, 2024

Commits on May 24, 2024

  1. only link to one jemalloc build 

    Without setting JEMALLOC_OVERRIDE, we end up linking to two different
jemalloc builds. Once dynamically, as a transitive dependency through
rocksdb, and a second time to the static jemalloc that tikv-jemalloc-sys
builds.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    bec507d View commit details
    Browse the repository at this point in the history

  2. do default-feature unification in nix 

    Some of the features affect nix dependencies, so we need to have a
full feature list available when constructing the nix derivation. This
incidentally fixes the bug where we weren't enabling jemalloc on rocksdb
in CI/devshells, because jemalloc is now a default feature. It does not
fix the more general class of that issue, where CI is performing an
`--all-features` build in a nix devshell built for default-features.

I am now passing `--no-default-features` to cargo, and having it use our
unified feature list rather than duplicating the unification inside cargo.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    857ac42 View commit details
    Browse the repository at this point in the history

  3. remove liburing from devshell 

    This doesn't seem to be necessary to build, and the derivation is broken
in pkgsStatic.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    7138eeb View commit details
    Browse the repository at this point in the history

  4. factor devshell out into a helper function 

    We're planning to add a second devshell with `all-features` for CI.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    5479ed4 View commit details
    Browse the repository at this point in the history

  5. switch default devshell to static linking 

    Dynamically-linked jemalloc doesn't work due to link-order issues, and we
want CI to be testing a static binary anyway since that's what we're
publishing in releases.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    cc71f64 View commit details
    Browse the repository at this point in the history

  6. enable all-features in nix for CI builds 

    CI is running `cargo build --all-features`, so we should be passing all
the features to nix as well.

The only thing this currently affects is the jemalloc_prof feature, but if
we add any non-default features that affect nix in the future they should
also be handled correctly now.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    1e7be34 View commit details
    Browse the repository at this point in the history

  7. add a 'no-features' devshell for local testing

    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    cf91db4 View commit details
    Browse the repository at this point in the history

  8. set show-trace for nix in CI

    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    6e6f67a View commit details
    Browse the repository at this point in the history

  9. don't use prefixed jemalloc with rocksdb 

    This is causing build failures on Mac:

> In file included from /tmp/nix-build-rocksdb-static-aarch64-apple-darwin-9.1.1.drv-0/source/memory/memory_allocator.cc:8:
> In file included from /tmp/nix-build-rocksdb-static-aarch64-apple-darwin-9.1.1.drv-0/source/memory/jemalloc_nodump_allocator.h:11:
> /tmp/nix-build-rocksdb-static-aarch64-apple-darwin-9.1.1.drv-0/source/port/jemalloc_helper.h:63:36: warning: unknown attribute '_rjem_malloc' ignored [-Wunknown-attributes]
> mallocx(size_t, int) JEMALLOC_ATTR(malloc) JEMALLOC_ALLOC_SIZE(1)
>                                    ^~~~~~
> /nix/store/3bix0kzy670dyhhizri3dwb1qfj3sdpa-jemalloc-static-aarch64-apple-darwin-5.3.0/include/jemalloc/jemalloc.h:412:18: note: expanded from macro 'malloc'
> #  define malloc je_malloc
>                  ^~~~~~~~~
> /nix/store/3bix0kzy670dyhhizri3dwb1qfj3sdpa-jemalloc-static-aarch64-apple-darwin-5.3.0/include/jemalloc/jemalloc.h:75:21: note: expanded from macro 'je_malloc'
> #  define je_malloc _rjem_malloc
>                     ^~~~~~~~~~~~
> /nix/store/3bix0kzy670dyhhizri3dwb1qfj3sdpa-jemalloc-static-aarch64-apple-darwin-5.3.0/include/jemalloc/jemalloc.h:183:43: note: expanded from macro 'JEMALLOC_ATTR'
> #  define JEMALLOC_ATTR(s) __attribute__((s))

Full build log at <https://girlboss.ceo/~strawberry/pb/ygJ3>. This is
likely fixable with patches to rocksdb, but not worth it since darwin is
only a dev platform.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    1224e19 View commit details
    Browse the repository at this point in the history

  10. mark static rocksdb broken on darwin

    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    eec0a0a View commit details
    Browse the repository at this point in the history

  11. mark dynamic jemalloc builds as broken on linux

    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    b4c1c0d View commit details
    Browse the repository at this point in the history

  12. add a dynamically-linked devshell 

    This is broken on linux, but can be used by darwin users for development,
since static/jemalloc/darwin is broken.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    6a1cbe7 View commit details
    Browse the repository at this point in the history

  13. use a statically-linked binary for complement 

    Dynamically-linked jemalloc is broken.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    9e9ff64 View commit details
    Browse the repository at this point in the history

  14. set C/LDFLAGS for complement dependencies directly 

    Previously we were relying on NIX_CFLAGS_COMPILE, but this is not being
set in static devshells. A cleaner solution for complement would likely
be to build the tests in their own nix derivation instead of building
them in the devshell, but this change unblocks CI for now.
    @Benjamin-L
    Benjamin-L committed May 24, 2024
    Configuration menu
    Copy the full SHA
    5c4cc43 View commit details
    Browse the repository at this point in the history