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.

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

spidermonkey_52: don't use jemalloc w/musl #46450

Merged
merged 1 commit into from Sep 10, 2018

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Sep 9, 2018

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: spidermonkey_52

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: spidermonkey_52

Partial log (click to expand)

/nix/store/4rv37m88jmjiqpr9d4r2s0r2c4cxmcpm-spidermonkey-52.9.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: spidermonkey_52

Partial log (click to expand)

/nix/store/fi6hdr8n5gqfjlr627dxrqxmbkr49llv-spidermonkey-52.9.0

@orivej
Copy link
Contributor

orivej commented Sep 10, 2018

Why do you want to disable jemalloc?

@dtzWill
Copy link
Member Author

dtzWill commented Sep 10, 2018

Not so much don't want as it doesn't work so disable it instead of failing to build :).

Historically musl wasn't compatible with malloc replacements, with 1.1.20 (days old) being first with support.

The build failure didn't look too bad but since I'm not sure it would work anyway seemed best to disable it.

@Mic92 Mic92 merged commit f38e8c0 into NixOS:master Sep 10, 2018
@Mic92
Copy link
Member

Mic92 commented Sep 10, 2018

Should this be backported?

@orivej
Copy link
Contributor

orivej commented Sep 10, 2018

@dtzWill Since 1.1.20 is already merged, would you revert this if it is not necessary? (Actually older musls can be used with external allocators as long as they are built without the internal one.)

@dtzWill
Copy link
Member Author

dtzWill commented Sep 10, 2018

@dtzWill Since 1.1.20 is already merged, would you revert this if it is not necessary? (Actually older musls can be used with external allocators as long as they are built without the internal one.)

Of course! Looking into it briefly I'm not optimistic about jemalloc-- having trouble getting the test-suite (for the jemalloc package itself, not the one vendored in mozjs) to not crash w/musl on versions 5.0.1 (our default), 5.1.0 (latest), or the current 'dev' branch code. That said, various musl-based distributions package it so maybe it's worth looking into further.

FWIW the build failure was due to attempting to include 'sys/sysctl.h' which doesn't exist w/musl.

@dtzWill
Copy link
Member Author

dtzWill commented Sep 10, 2018

Quick follow-up in terms of Nixpkgs and jemalloc+musl:

  • jemalloc 4.5.0 seems to work w/musl 1.1.20 (and 1.1.19)
  • jemalloc 5.x does not work with musl 1.1.20 but seems to work with musl 1.1.19
  • spidermonkey52 doesn't build against 1.1.20 or 1.1.19 as-is (sys/sysctl.h issue).

Where by jemalloc "working" I mean "jemalloc tests pass".

@orivej
Copy link
Contributor

orivej commented Sep 10, 2018

I had used musl with jemalloc 3.6.0.

sys/sysctl.h seems to be a BSD header, it does not exist on Linux even with Glibc.

Edit: it does exist and is documented here, my locate has somehow missed it.

@dtzWill
Copy link
Member Author

dtzWill commented Sep 10, 2018 via email

@vcunat
Copy link
Member

vcunat commented Sep 10, 2018

Historically musl wasn't compatible with malloc replacements, with 1.1.20 (days old) being first with support.

😱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants