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 futures_helper_thread not Resettable #7811

Conversation

Projects
None yet
3 participants
@blorente
Copy link
Contributor

commented May 29, 2019

Problem

After removing forking, a bunch of code is leftover that has to be removed, for a slight perf improvement.

Related issues: #7670

Solution

This PR removes one of the uses of Resettable, as a step towards removing fork_context

@blorente blorente requested review from stuhood and illicitonion May 29, 2019

Show resolved Hide resolved src/rust/engine/process_execution/src/remote.rs Outdated
Show resolved Hide resolved src/rust/engine/src/context.rs Outdated

@blorente blorente merged commit c0e6b7b into pantsbuild:master May 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@stuhood
Copy link
Member

left a comment

Thanks!

@@ -312,7 +312,7 @@ impl CommandRunner {
platform_properties: BTreeMap<String, String>,
thread_count: usize,
store: Store,
futures_timer_thread: resettable::Resettable<futures_timer::HelperThread>,
futures_timer_thread: Arc<futures_timer::HelperThread>,

This comment has been minimized.

Copy link
@stuhood

stuhood May 29, 2019

Member

Unrelated, but: now that we guarantee the tokio runtime everywhere, we should definitely switch to using tokio's timers.

This comment has been minimized.

Copy link
@illicitonion
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.