Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 7d6bff8645fb
Choose a base ref
...
head repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 85e7f634ac33
Choose a head ref
  • 5 commits
  • 5 files changed
  • 1 contributor

Commits on Nov 24, 2020

  1. Copy the full SHA
    1da27c4 View commit details
  2. Fix a bug.

    pleroy committed Nov 24, 2020
    Copy the full SHA
    675e012 View commit details

Commits on Nov 25, 2020

  1. After egg's review.

    pleroy committed Nov 25, 2020
    Copy the full SHA
    8a99ac6 View commit details
  2. Fix compilation error.

    pleroy committed Nov 25, 2020
    Copy the full SHA
    d1a8d67 View commit details
  3. Merge pull request #2795 from pleroy/UseJThread

    Use jthread in the orbit analyzer
    pleroy authored Nov 25, 2020
    Copy the full SHA
    85e7f63 View commit details
Showing with 37 additions and 27 deletions.
  1. +9 −3 base/jthread.hpp
  2. +19 −7 base/jthread_body.hpp
  3. +1 −1 base/jthread_test.cpp
  4. +5 −10 ksp_plugin/orbit_analyser.cpp
  5. +3 −6 ksp_plugin/orbit_analyser.hpp
12 changes: 9 additions & 3 deletions base/jthread.hpp
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ class StopState;
// https://en.cppreference.com/w/cpp/thread/stop_token
class stop_token {
public:
stop_token();
stop_token() = default;

bool stop_requested() const;

@@ -31,7 +31,7 @@ class stop_token {

StopState& get_stop_state() const;

StopState* stop_state_;
StopState* stop_state_ = nullptr;

friend class jthread;
friend class stop_callback;
@@ -86,8 +86,13 @@ class jthread {
template<typename Function, typename... Args>
jthread(Function&& f, Args&&... args);

jthread(jthread&& other);
jthread& operator=(jthread&& other);

~jthread();

bool joinable() const;

void join();

void detach();
@@ -103,6 +108,7 @@ class jthread {
std::thread thread_;
};

// |f| *does not* take a stop_token as its first parameter.
template<typename Function, typename... Args>
static jthread MakeStoppableThread(Function&& f, Args&&... args);

@@ -111,7 +117,7 @@ class this_stoppable_thread {
static stop_token get_stop_token();

private:
static thread_local stop_token stop_token_;
inline static thread_local stop_token stop_token_;

template<typename Function, typename... Args>
friend jthread MakeStoppableThread(Function&& f, Args&&... args);
26 changes: 19 additions & 7 deletions base/jthread_body.hpp
Original file line number Diff line number Diff line change
@@ -65,8 +65,6 @@ inline void StopState::Unregister(not_null<stop_callback*> const callback) {
callbacks_.erase(callback);
}

inline stop_token::stop_token() : stop_state_(nullptr) {}

inline bool stop_token::stop_requested() const {
return stop_state_->stop_requested();
}
@@ -115,13 +113,29 @@ jthread::jthread(Function&& f, Args&&... args)
stop_token(stop_state_.get()),
std::forward<Args>(args)...) {}

inline jthread::jthread(jthread&& other)
: stop_state_(std::move(other.stop_state_)),
thread_(std::move(other.thread_)) {}

inline jthread& jthread::operator=(jthread&& other) {
stop_state_ = std::move(other.stop_state_);
thread_ = std::move(other.thread_);
return *this;
}

inline jthread::~jthread() {
stop_state_->request_stop();
if (stop_state_ != nullptr) {
stop_state_->request_stop();
}
if (thread_.joinable()) {
thread_.join();
}
}

inline bool jthread::joinable() const {
return thread_.joinable();
}

inline void jthread::join() {
thread_.join();
}
@@ -148,17 +162,15 @@ jthread MakeStoppableThread(Function&& f, Args&&... args) {
[f](stop_token const& st, Args&&... args) {
// This assignment happens on the thread of the jthread.
this_stoppable_thread::stop_token_ = st;
f(st, args...);
f(args...);
},
args...);
}

stop_token this_stoppable_thread::get_stop_token() {
inline stop_token this_stoppable_thread::get_stop_token() {
return stop_token_;
}

thread_local stop_token this_stoppable_thread::stop_token_;

} // namespace internal_jthread
} // namespace base
} // namespace principia
2 changes: 1 addition & 1 deletion base/jthread_test.cpp
Original file line number Diff line number Diff line change
@@ -120,7 +120,7 @@ TEST(JThreadTest, ThisJThread) {
bool observed_stop = false;

auto sleepy_worker = MakeStoppableThread(
[](stop_token /*unused*/, bool* const observed_stop) {
[](bool* const observed_stop) {
for (int i = 0; i < 10; i++) {
absl::SleepFor(absl::Milliseconds(10));
if (this_stoppable_thread::get_stop_token().stop_requested()) {
15 changes: 5 additions & 10 deletions ksp_plugin/orbit_analyser.cpp
Original file line number Diff line number Diff line change
@@ -12,6 +12,8 @@ namespace principia {
namespace ksp_plugin {
namespace internal_orbit_analyser {

using base::stop_token;
using base::this_stoppable_thread;
using geometry::Frame;
using geometry::NonRotating;
using physics::BodyCentredNonRotatingDynamicFrame;
@@ -26,20 +28,13 @@ OrbitAnalyser::OrbitAnalyser(
analysed_trajectory_parameters_(
std::move(analysed_trajectory_parameters)) {}

OrbitAnalyser::~OrbitAnalyser() {
if (analyser_.joinable()) {
keep_analysing_ = false;
analyser_.join();
}
}

void OrbitAnalyser::RequestAnalysis(
Instant const& first_time,
DegreesOfFreedom<Barycentric> const& first_degrees_of_freedom,
Time const& mission_duration,
not_null<RotatingBody<Barycentric> const*> primary) {
if (!analyser_.joinable()) {
analyser_ = std::thread([this] { RepeatedlyAnalyseOrbit(); });
analyser_ = base::MakeStoppableThread([this] { RepeatedlyAnalyseOrbit(); });
}
Ephemeris<Barycentric>::Guard guard(ephemeris_);
if (ephemeris_->t_min() > first_time) {
@@ -76,7 +71,7 @@ void OrbitAnalyser::RepeatedlyAnalyseOrbit() {
std::chrono::steady_clock::time_point const wakeup_time =
std::chrono::steady_clock::now() + std::chrono::milliseconds(20);

if (!keep_analysing_) {
if (this_stoppable_thread::get_stop_token().stop_requested()) {
return;
}

@@ -112,7 +107,7 @@ void OrbitAnalyser::RepeatedlyAnalyseOrbit() {
progress_of_next_analysis_ =
(trajectory.back().time - parameters->first_time) /
parameters->mission_duration;
if (!keep_analysing_) {
if (this_stoppable_thread::get_stop_token().stop_requested()) {
return;
}
}
9 changes: 3 additions & 6 deletions ksp_plugin/orbit_analyser.hpp
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@
#include "astronomy/orbit_ground_track.hpp"
#include "astronomy/orbit_recurrence.hpp"
#include "astronomy/orbital_elements.hpp"
#include "base/jthread.hpp"
#include "base/not_null.hpp"
#include "geometry/named_quantities.hpp"
#include "ksp_plugin/frames.hpp"
@@ -23,6 +24,7 @@ namespace internal_orbit_analyser {
using astronomy::OrbitalElements;
using astronomy::OrbitGroundTrack;
using astronomy::OrbitRecurrence;
using base::jthread;
using base::not_null;
using geometry::Instant;
using physics::DegreesOfFreedom;
@@ -86,7 +88,6 @@ class OrbitAnalyser {
OrbitAnalyser(not_null<Ephemeris<Barycentric>*> ephemeris,
Ephemeris<Barycentric>::FixedStepParameters
analysed_trajectory_parameters);
~OrbitAnalyser();

// Sets the parameters that will be used for the computation of the next
// analysis.
@@ -125,7 +126,7 @@ class OrbitAnalyser {
std::optional<Analysis> analysis_;

mutable absl::Mutex lock_;
std::thread analyser_;
jthread analyser_;
// |parameters_| is set by the main thread; it is read and cleared by the
// |analyser_| thread.
std::optional<Parameters> parameters_ GUARDED_BY(lock_);
@@ -135,10 +136,6 @@ class OrbitAnalyser {
// |progress_of_next_analysis_| is set by the |analyser_| thread; it tracks
// progress in computing |next_analysis_|.
std::atomic<double> progress_of_next_analysis_ = 0;
// |keep_analysing_| is tested by the |analyser_| thread, which cooperatively
// aborts if it is false; it is set at construction, and cleared by the main
// thread at destruction.
std::atomic_bool keep_analysing_ = true;
};

} // namespace internal_orbit_analyser