remove je_ link to libc

[NikVolf]

[SimonSapin]

This would fix this crate for recent nightlies, but break it for older nightlies and stable/beta.

We probably need a build script that parses $RUSTC --version to conditionally print cargo:rustc-cfg=unprefixed_jemalloc and use something like #[cfg_attr(unprefixed_jemalloc, link_name("malloc_usable_size"))] on the extern declaration.

[NikVolf]

why not just
[cfg_attr(feature = "nightly", link_name("malloc_usable_size"))]
?

[SimonSapin]

Because then you leave it to users to figure out when they need to enable it or not. And it’s not just nightly: Servo for example is using an older nigthly that doesn’t have this change yet. And in a few weeks this change is gonna reach beta and then stable.

[larsbergstrom]

CC @alexcrichton @brson for heads-up on breakage from rust-lang/rust@e3b414d

[SimonSapin]

To be fair, we’re using extern to circumvent privacy and rely and very much non-public implementation details.

[quininer]

@SimonSapin How it looks now?

[larsbergstrom]

lgtm after squash

[SimonSapin]

This is a good start, thanks! I think we also want to check the commit date in order to support older 1.8 nightlies (such as the one Servo is using at the moment) I’m looking into this now.

Also, do we need 3 names rather than 2? je_mallocx / malloc_usable_size / je_malloc_usable_size

[SimonSapin]

On further thought, this seems very fragile. Maybe we’d be better of doing what @NikVolf initially proposed in a semver-breaking version of heapsize that only supports Rust 1.8.0-nightly 2016-02-16 and later.

Many libraries implementing the HeapSizeOf do so with an optional Cargo feature to enable the heapsize_plugin crate and derive the trait, which requires unstable Rust anyway.

[emilio]

Also, from the rustc offending commit, it seems that the je_ prefix is still used in android.

[quininer]

@SimonSapin parity is currently not used heapsize_plugin, but I think the semver-breaking was good.

@ecoal95 You can test it, I think this work in android and OSX.

[SimonSapin]

@quininer @NikVolf Do you use heapsize on Rust beta/stable?

[quininer]

@SimonSapin I use Rust night, parity just switching from night to beta.

[NikVolf]

@SimonSapin currently no, we switched to beta

[SimonSapin]

So, from rust-lang/rust@e3b414d it looks like je_mallocx takes a size and some flags and so is not at all the same as je_malloc_usable_size which takes a pointer.

[quininer]

@SimonSapin My fault, fix it here

[brson]

Interesting breakage. I've never considered the jemalloc symbols part of our interface. Here's an issue about it, though it seems unlikely Rust will revert it.

[SimonSapin]

@brson FWIW I agree that using extern like this is a hack that allows us to access functionality that is not at all part of std’s public API, and therefore not covered by the stability promises.

Eventually, I’d like std to properly expose this functionality, but I don’t know what the API for that should look like.

[nox]

@brson IMO there should be at least a function similar to unsafe fn heap_size_of(ptr: *const c_void) -> usize in Rust itself. The hack needed in #45 is a bit sad, though it could be a bit alleviated by making this function unsafe fn heap_size_of<T>(ptr: *const T) -> usize.

[bors-servo]

☔ The latest upstream changes (presumably #46) made this pull request unmergeable. Please resolve the merge conflicts.

[SimonSapin]

I’ve merged this with some tweaks in #46. Thanks @NikVolf and @quininer!

[MagaTailor]

So now I'm getting this error on ARM:

Compiling heapsize v0.3.3 (file:///tmp/heapsize)
     Running `/tmp/parity-master/target/release/build/heapsize-80af5a4fdd6377bd/build-script-build`
failed to run custom build command for `heapsize v0.3.3 (file:///tmp/heapsize)`
Process didn't exit successfully: `/tmp/parity-master/target/release/build/heapsize-80af5a4fdd6377bd/build-script-build` (exit code: 101)
--- stderr
thread '<main>' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:330
stack backtrace:
   1: 0xb6fc0c13 - sys::backtrace::tracing::imp::write::h7a915cce7da704dazyu
   2: 0xb6fc4b8f - panicking::default_handler::_$u7b$$u7b$closure$u7d$$u7d$::closure.43638
   3: 0xb6fc471b - panicking::default_handler::hbc4727249312e5e2Ofz
   4: 0xb6fb52f3 - sys_common::unwind::begin_unwind_inner::h62426d48343caeb7Smt
   5: 0xb6fb5827 - sys_common::unwind::begin_unwind_fmt::hc0d86094a2fefe4eYlt
   6: 0xb6fc0263 - rust_begin_unwind
   7: 0xb6fcee3f - panicking::panic_fmt::h8e92de25aabc69726YL
   8: 0xb6fcf177 - panicking::panic::h5fd81d80ff589a0dDXL
   9: 0xb6f76457 - option::Option<T>::unwrap::h3581985636882006382
                at src/libcore/macros.rs:21
  10: 0xb6f71597 - main::ha91966c46cd08253iaa
                at /tmp/heapsize/build.rs:13
  11: 0xb6fc42e3 - sys_common::unwind::try::try_fn::h2032953291300600666

whereas before it was getting to the link stage. Rustc nightly is dated Feb 22nd.

rustc 1.8.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: arm-unknown-linux-gnueabihf
release: 1.8.0-dev

[jdm]

What's the precise output of rustc --version for you?

[MagaTailor]

Updated. Building from a source snapshot instead of a git clone looks like this.

[SimonSapin]

@petevine What’s rustc --version without --verbose? (The formatting matters for the regex in build.rs.)

Looks like tarball source build don’t hold enough information to know if they have unprefixed jemalloc or not. We tweak the build script so that it’s assumed unprefixed on 1.8.0. (So it will guess incorrectly for 1.8.0 sources from before 2016-02-14 rather than after.)

[MagaTailor]

That would be just the first line rustc 1.8.0-dev

[SimonSapin]

@petevine Could you try #49?

[MagaTailor]

Did that, still no good.

[SimonSapin]

Ok, I’m make a tarball-source build tomorrow.

[SimonSapin]

@petevine I tweaked the regex in #49 and it worked for me with a tarball-source build. Try again?

[MagaTailor]

Is it on crates.io yet?

[SimonSapin]

@petevine https://crates.io/crates/heapsize/0.3.3 includes #46, but #49 is not merged yet.

[MagaTailor]

Ok, it works.