Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

Undefined symbol _je_malloc_usable_size on macOS without jemalloc #80

Closed
rillian opened this issue Apr 25, 2017 · 9 comments
Closed

Undefined symbol _je_malloc_usable_size on macOS without jemalloc #80

rillian opened this issue Apr 25, 2017 · 9 comments

Comments

@rillian
Copy link

rillian commented Apr 25, 2017

Building the Firefox C++ code with the clang address sanitizer requires --disable-jemalloc, but this fails in optmized builds on macOS because heapsize is expecting jemalloc's C api:

 9:54.66 Undefined symbols for architecture x86_64:
 9:54.66   "_je_malloc_usable_size", referenced from:
 9:54.66       heapsize::heap_size_of_impl::h66890e23e3d3471b in libgkrust.a(gkrust-bbd5660dd29a1aed.0.o)
 9:54.66 ld: symbol(s) not found for architecture x86_64
 9:54.66 clang: error: linker command failed with exit code 1 (use -v to see invocation)

See https://bugzilla.mozilla.org/show_bug.cgi?id=1359475 for specific configuration details.

@rillian
Copy link
Author

rillian commented Apr 25, 2017

Looks like there needs to be another conditional on https://github.com/servo/heapsize/blob/master/src/lib.rs#L41 to handle this case?

@SimonSapin
Copy link
Member

I think this is https://bugzilla.mozilla.org/show_bug.cgi?id=1350581.

@nnethercote, you wrote there you wanted to redesign the HeapSizeOf trait so that its method takes a "malloc_size_of" function parameter, which would fix this issue. How is that going?

@nnethercote
Copy link

It's going slowly due to bindgen issues. Also, I'm now thinking about leaving HeapSizeOf alone and introducing a new trait that takes the malloc_size_of parameter, because (a) that's kind of specific to Gecko/Firefox, and (b) that will avoid the need for me to update all the crates that use HeapSizeOf.

@pcwalton
Copy link

pcwalton commented Jul 2, 2017

This breaks any crate which transitively uses heapsize when crate-type = "dylib" is specified in Cargo.toml; for example, it breaks euclid in this way. This hurts my C bindings for Pathfinder…

@SimonSapin
Copy link
Member

Until some other design materializes, this crate effectively depends on jemalloc. We could make it an optional dependency in euclid.

@nnethercote
Copy link

Yes, it's basically a flawed design. In Stylo I've introduced a MallocSizeOf trait, which is identical to HeapSizeOf except that the function that measures the heap block size is passed in. This avoids the linking problems, and also matches how Firefox's memory reporting works.

I'd like for HeapSizeOf to be replaced by MallocSizeOf everywhere within Servo, but this is complicated by the fact that HeapSizeOf is in its own public crate, which is used by other crates, including ones that are depended on by Servo. Also, HeapSizeOf is supported by a plugin that supports auto-deriving. So I'm unsure about how to go about replacing it. I'd be happy to hear suggestions.

@jdm
Copy link
Member

jdm commented Jul 3, 2017

What's the issue with updating HeapSizeOf to accept an argument and publishing a new version?

@jdm
Copy link
Member

jdm commented Jul 3, 2017

I have gone ahead and done that in #84.

@SimonSapin
Copy link
Member

#84 is replaced with a separate malloc_size_of crate with that design: https://github.com/servo/servo/tree/master/components/malloc_size_of, which Servo and Gecko now uses instead of heapsize.

Closing as “wontfix”, heapsize will keep depending on jemalloc.

The next update of WebRender in Gecko should include servo/webrender#2192 and remove heapsize from the dependency graph.

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

No branches or pull requests

5 participants