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

jemalloc! #6895

Closed
wants to merge 4 commits into
base: incoming
from

Conversation

Projects
None yet
5 participants
@cmr
Copy link
Member

cmr commented Jun 2, 2013

No description provided.

@cmr

This comment has been minimized.

Copy link
Member Author

cmr commented Jun 2, 2013

r? @brson

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 2, 2013

Woo! 🌟

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 2, 2013

This looks really great. I'm glad it was so easy to integrate.

Before landing, I'd like to collect some performance numbers on the three major platforms so we can say something about the impact it has. I opened #6897 on this, but may not be able to do it for a few days.

Pushing to try now.

@brson brson referenced this pull request Jun 2, 2013

Closed

Compare jemalloc performance #6897

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 2, 2013

I see the following error when I build on x86_64-linux

                                               ^~~~~~~~~~~~~~
/home/brian/dev/rust2/src/librustc/middle/kind.rs:15:0: 15:21 warning: unused import [-W unused-imports (default)]
/home/brian/dev/rust2/src/librustc/middle/kind.rs:15 use middle::pat_util;
                                                     ^~~~~~~~~~~~~~~~~~~~~
compile_and_link: x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/bin/rustc
error: linking with `cc` failed with code 1
note: cc arguments: -L/opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib -m64 -o x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/bin/rustc x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/bin/rustc.o -L/opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib -lstd-c3ca5d77d81b46c1-0.7-pre -L/opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib -lextra-4782a756585a81-0.7-pre -L/opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib -lsyntax-f25ff37b2c135ff-0.7-pre -L/opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib -lrustc-ba32f36fc1ec12e-0.7-pre -lrustrt -lrt -lpthread -Lrustllvm -lrustllvm -lrt -ldl -lm -lmorestack -lrustrt -Wl,-rpath,$ORIGIN/../lib -Wl,-rpath,/opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib -Wl,-rpath,/usr/local/lib/rustc/x86_64-unknown-linux-gnu/lib
note: /opt/devel/rust2/build/x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/lib/librustrt.so: undefined reference to `pthread_atfork'
collect2: error: ld returned 1 exit status

error: aborting due to previous error
make: *** [x86_64-unknown-linux-gnu/stage0/lib/rustc/x86_64-unknown-linux-gnu/bin/rustc] Error 101
@pcwalton

This comment has been minimized.

Copy link
Contributor

pcwalton commented Jun 2, 2013

Isn't tcmalloc generally supposed to be faster? I feel like we should not make any moves here until we've concretely measured performance.

@cmr

This comment has been minimized.

Copy link
Member Author

cmr commented Jun 2, 2013

@thestinger

This comment has been minimized.

Copy link
Contributor

thestinger commented Jun 2, 2013

@pcwalton: the problem with tcmalloc is that Google isn't including many of their Chromium improvements in the gperftools tcmalloc. The upstream jemalloc gets improvements from Facebook, Mozilla, FreeBSD, etc. and it seems to be better than the easily obtainable tcmalloc from gperftools.

We could extract tcmalloc from the Chromium source and compare to it but.... I'd rather use something that actually has stable releases, documentation and a standalone build system even if it's slightly slower.

@pcwalton

This comment has been minimized.

Copy link
Contributor

pcwalton commented Jun 2, 2013

Aha, never mind then. That's unfortunate re. tcmalloc, but I suppose it can't be helped.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 3, 2013

@cmr

This comment has been minimized.

Copy link
Member Author

cmr commented Jun 5, 2013

librustrt needs to link against jemalloc's dependencies (in this case, pthreads). I do not know how to accomplish this.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 6, 2013

I still want to do a little testing on mac and in multithreaded scenarios. Sorry for the delay.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented on 829b5de Jun 7, 2013

This comment has been minimized.

Copy link
Contributor

bors replied Jun 7, 2013

merging cmr/rust/jemalloc = 829b5de into auto

This comment has been minimized.

Copy link
Contributor

bors replied Jun 7, 2013

cmr/rust/jemalloc = 829b5de merged ok, testing candidate = 5d2cadb

This comment has been minimized.

Copy link
Contributor

bors replied Jun 7, 2013

fast-forwarding incoming to auto = 5d2cadb

bors added a commit that referenced this pull request Jun 7, 2013

@bors bors closed this Jun 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.