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

Make SyncDroplessArena allocations thread-local in the fast path #61873

Open
wants to merge 2 commits into
base: master
from

Conversation

@Zoxc
Copy link
Contributor

commented Jun 15, 2019

This works by having a thread local chunk to allocate from and a global list of chunks for in_arena to use.

r? @eddyb

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Jun 15, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:138e0ab4:start=1560636922330806312,finish=1560636924688547365,duration=2357741053
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:07:20]    Compiling rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:07:22] error[E0308]: mismatched types
[00:07:22]    --> src/librustc_data_structures/sync.rs:229:35
[00:07:22]     |
[00:07:22] 229 |                 SharedWorkerLocal(OneThread::new(f(0)))
[00:07:22]     |                                   ^^^^^^^^^^^^^^^^^^^^ expected type parameter, found struct `sync::OneThread`
[00:07:22]     = note: expected type `T`
[00:07:22]     = note: expected type `T`
[00:07:22]                found type `sync::OneThread<T>`
[00:07:23] error: aborting due to previous error
[00:07:23] 
[00:07:23] For more information about this error, try `rustc --explain E0308`.
[00:07:23] error: Could not compile `rustc_data_structures`.
---
travis_time:end:0301eff5:start=1560637392091412071,finish=1560637392096760232,duration=5348161
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:03a6275a
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:2f9866ec
travis_time:start:2f9866ec
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:014422e0
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Zoxc Zoxc force-pushed the Zoxc:tls-arena branch from 5b5cea8 to 512a1cd Jun 15, 2019

@eddyb

This comment has been minimized.

Copy link
Member

commented Jun 16, 2019

I don't know how any of this concurrent code works, sorry! (Or rather, why it might be safe)
Maybe if @aturon gets back, he can help with reviewing it?

cc @rust-lang/compiler Is anyone else comfortable with concurrent data structures and willing to review?

@nikomatsakis nikomatsakis assigned nikomatsakis and aturon and unassigned eddyb Jun 18, 2019

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

I'll review it and/or get @aturon to do so =)

@Dylan-DPC

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

ping from triage @nikomatsakis @aturon any updates on this?

@aturon

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Hey @Dylan-DPC, I plan to review this one, however at the moment I'm getting up to speed on the concurrency approach in general.

@Dylan-DPC

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

thanks. No issues.

@eddyb

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

Also pinging @gankro and @stjepang, in case they want to review as well.

@Gankra

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

I do not

@hdhoang

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

ping from triage @stjepang, could you provide comment on this PR?

pub fn clear(&mut self) {
self.lock.get_mut().clear();
fn grow(&self, n: usize, chunks: &mut Vec<TypedArenaChunk<T>>) {
unsafe {

This comment has been minimized.

Copy link
@Centril

Centril Aug 8, 2019

Member

Would be great if these large unsafe { .. } blocks would have comments justifying their soundness.
(Applies throughout the PR.)

@ProgrammaticNajel

This comment has been minimized.

Copy link

commented Aug 16, 2019

Ping from triage. @aturon any updates on this? Thanks

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