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: 250694b4a1e2
Choose a base ref
...
head repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: e95eb6cb642b
Choose a head ref
  • 11 commits
  • 8 files changed
  • 2 contributors

Commits on Mar 31, 2019

  1. Reorder.

    pleroy committed Mar 31, 2019
    Copy the full SHA
    bafefb3 View commit details
  2. Copy the full SHA
    90da44a View commit details

Commits on Apr 1, 2019

  1. Replaying.

    pleroy committed Apr 1, 2019
    Copy the full SHA
    b87187e View commit details

Commits on Apr 22, 2019

  1. Copy the full SHA
    106428e View commit details

Commits on Apr 23, 2019

  1. Better locking.

    pleroy committed Apr 23, 2019
    Copy the full SHA
    caeab36 View commit details
  2. Copy the full SHA
    5fb2c07 View commit details
  3. Renaming.

    pleroy committed Apr 23, 2019
    Copy the full SHA
    1cf00cc View commit details

Commits on Apr 24, 2019

  1. Copy the full SHA
    4775afc View commit details

Commits on Apr 25, 2019

  1. Copy the full SHA
    2de1af3 View commit details

Commits on Apr 26, 2019

  1. After egg's review.

    pleroy committed Apr 26, 2019
    Copy the full SHA
    156fb74 View commit details

Commits on Apr 27, 2019

  1. Merge pull request #2141 from pleroy/2107

    Synchronize the prognostication when recording and replaying the journal
    pleroy authored Apr 27, 2019
    Copy the full SHA
    e95eb6c View commit details
Showing with 109 additions and 47 deletions.
  1. +2 −0 journal/player.cpp
  2. +3 −7 journal/player_test.cpp
  3. +1 −1 journal/recorder.cpp
  4. +1 −1 journal/recorder.hpp
  5. +6 −0 ksp_plugin/interface.cpp
  6. +3 −0 ksp_plugin/interface.hpp
  7. +74 −36 ksp_plugin/vessel.cpp
  8. +19 −2 ksp_plugin/vessel.hpp
2 changes: 2 additions & 0 deletions journal/player.cpp
Original file line number Diff line number Diff line change
@@ -16,11 +16,13 @@ namespace principia {
using base::GetLine;
using base::HexadecimalEncoder;
using base::UniqueArray;
using interface::principia__ActivatePlayer;

namespace journal {

Player::Player(std::filesystem::path const& path)
: stream_(path, std::ios::in) {
principia__ActivatePlayer();
CHECK(!stream_.fail());
}

10 changes: 3 additions & 7 deletions journal/player_test.cpp
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@

#include <list>
#include <string>
#include <thread>
#include <vector>

#include "benchmark/benchmark.h"
@@ -55,9 +54,7 @@ class PlayerTest : public ::testing::Test {
};

TEST_F(PlayerTest, PlayTiny) {
// Do the recording in a separate thread to make sure that it activates using
// a different static variable than the one in the plugin dynamic library.
std::thread recorder([this]() {
{
Recorder* const r(new Recorder(test_name_ + ".journal.hex"));
Recorder::Activate(r);

@@ -71,8 +68,7 @@ TEST_F(PlayerTest, PlayTiny) {
m.Return();
}
Recorder::Deactivate();
});
recorder.join();
}

Player player(test_name_ + ".journal.hex");

@@ -92,7 +88,7 @@ TEST_F(PlayerTest, DISABLED_Debug) {
// An example of how journaling may be used for debugging. You must set
// |path| and fill the |method_in| and |method_out_return| protocol buffers.
std::string path =
R"(\\venezia.mockingbirdnest.com\Namespaces\Public\Public Mockingbird\Principia\Crashes\1703\JOURNAL.20180130-000310)"; // NOLINT
R"(C:\Program Files\Kerbal Space Program\1.6.1\glog\Principia\JOURNAL.20190425-223810)"; // NOLINT
Player player(path);
int count = 0;
while (player.Play()) {
2 changes: 1 addition & 1 deletion journal/recorder.cpp
Original file line number Diff line number Diff line change
@@ -54,7 +54,7 @@ void Recorder::WriteLocked(serialization::Method const& method) {
stream_.flush();
}

thread_local Recorder* Recorder::active_recorder_ = nullptr;
Recorder* Recorder::active_recorder_ = nullptr;

} // namespace journal
} // namespace principia
2 changes: 1 addition & 1 deletion journal/recorder.hpp
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ class Recorder final {
absl::Mutex lock_;
std::ofstream stream_;

static thread_local Recorder* active_recorder_;
static Recorder* active_recorder_;

template<typename>
friend class Method;
6 changes: 6 additions & 0 deletions ksp_plugin/interface.cpp
Original file line number Diff line number Diff line change
@@ -268,6 +268,10 @@ NewEncoder(std::string_view const encoder) {

} // namespace

void principia__ActivatePlayer() {
Vessel::MakeSynchronous();
}

// If |activate| is true and there is no active journal, create one and
// activate it. If |activate| is false and there is an active journal,
// deactivate it. Does nothing if there is already a journal in the desired
@@ -285,9 +289,11 @@ void principia__ActivateRecorder(bool const activate) {
name << std::put_time(localtime, "JOURNAL.%Y%m%d-%H%M%S");
journal::Recorder* const recorder = new journal::Recorder(
std::filesystem::path("glog") / "Principia" / name.str());
Vessel::MakeSynchronous();
journal::Recorder::Activate(recorder);
} else if (!activate && journal::Recorder::IsActivated()) {
journal::Recorder::Deactivate();
Vessel::MakeAsynchronous();
}
}

3 changes: 3 additions & 0 deletions ksp_plugin/interface.hpp
Original file line number Diff line number Diff line change
@@ -66,6 +66,9 @@ std::unique_ptr<T[]> TakeOwnershipArray(T** pointer);

#include "ksp_plugin/interface.generated.h"

extern "C" PRINCIPIA_DLL
void CDECL principia__ActivatePlayer();

extern "C" PRINCIPIA_DLL
void CDECL principia__ActivateRecorder(bool activate);

110 changes: 74 additions & 36 deletions ksp_plugin/vessel.cpp
Original file line number Diff line number Diff line change
@@ -291,7 +291,14 @@ void Vessel::RefreshPrediction() {
psychohistory_->last().degrees_of_freedom(),
prediction_adaptive_step_parameters_,
/*shutdown=*/false};
StartPrognosticatorIfNeeded();
if (synchronous_) {
std::unique_ptr<DiscreteTrajectory<Barycentric>> prognostication;
Status const status = FlowPrognostication(*prognosticator_parameters_,
prognostication);
SwapPrognostication(prognostication, status);
} else {
StartPrognosticatorIfNeeded();
}
if (prognostication_ != nullptr) {
AttachPrediction(std::move(prognostication_));
}
@@ -302,6 +309,10 @@ void Vessel::RefreshPrediction(Instant const& time) {
prediction_->ForgetAfter(time);
}

std::string Vessel::ShortDebugString() const {
return name_ + " (" + guid_ + ")";
}

void Vessel::WriteToMessage(not_null<serialization::Vessel*> const message,
PileUp::SerializationIndexForPileUp const&
serialization_index_for_pile_up) const {
@@ -414,8 +425,12 @@ void Vessel::FillContainingPileUpsFromMessage(
}
}

std::string Vessel::ShortDebugString() const {
return name_ + " (" + guid_ + ")";
void Vessel::MakeAsynchronous() {
synchronous_ = false;
}

void Vessel::MakeSynchronous() {
synchronous_ = true;
}

Vessel::Vessel()
@@ -458,40 +473,12 @@ void Vessel::RepeatedlyFlowPrognostication() {
// produce the same effects.
if (!previous_prognosticator_parameters ||
*previous_prognosticator_parameters != prognosticator_parameters) {
auto prognostication =
std::make_unique<DiscreteTrajectory<Barycentric>>();
prognostication->Append(
prognosticator_parameters->first_time,
prognosticator_parameters->first_degrees_of_freedom);
Status status;
status = ephemeris_->FlowWithAdaptiveStep(
prognostication.get(),
Ephemeris<Barycentric>::NoIntrinsicAcceleration,
ephemeris_->t_max(),
prognosticator_parameters->adaptive_step_parameters,
FlightPlan::max_ephemeris_steps_per_frame,
/*last_point_only=*/false);
bool const reached_t_max = status.ok();
if (reached_t_max) {
// This will prolong the ephemeris by |max_ephemeris_steps_per_frame|.
status = ephemeris_->FlowWithAdaptiveStep(
prognostication.get(),
Ephemeris<Barycentric>::NoIntrinsicAcceleration,
InfiniteFuture,
prognosticator_parameters->adaptive_step_parameters,
FlightPlan::max_ephemeris_steps_per_frame,
/*last_point_only=*/false);
}
LOG_IF(INFO, !status.ok())
<< "Prognostication from " << prognosticator_parameters->first_time
<< " finished at " << prognostication->last().time() << " with "
<< status.ToString() << " for " << ShortDebugString();

// Publish the prognostication if the computation was not cancelled.
if (status.error() != Error::CANCELLED) {
std::unique_ptr<DiscreteTrajectory<Barycentric>> prognostication;
Status const status = FlowPrognostication(*prognosticator_parameters,
prognostication);
{
absl::MutexLock l(&prognosticator_lock_);
prognostication_.swap(prognostication);
prognosticator_status_ = status;
SwapPrognostication(prognostication, status);
}
previous_prognosticator_parameters = prognosticator_parameters;
}
@@ -500,6 +487,49 @@ void Vessel::RepeatedlyFlowPrognostication() {
}
}

Status Vessel::FlowPrognostication(
PrognosticatorParameters const& prognosticator_parameters,
std::unique_ptr<DiscreteTrajectory<Barycentric>>& prognostication) {
prognostication = std::make_unique<DiscreteTrajectory<Barycentric>>();
prognostication->Append(
prognosticator_parameters.first_time,
prognosticator_parameters.first_degrees_of_freedom);
Status status;
status = ephemeris_->FlowWithAdaptiveStep(
prognostication.get(),
Ephemeris<Barycentric>::NoIntrinsicAcceleration,
ephemeris_->t_max(),
prognosticator_parameters.adaptive_step_parameters,
FlightPlan::max_ephemeris_steps_per_frame,
/*last_point_only=*/false);
bool const reached_t_max = status.ok();
if (reached_t_max) {
// This will prolong the ephemeris by |max_ephemeris_steps_per_frame|.
status = ephemeris_->FlowWithAdaptiveStep(
prognostication.get(),
Ephemeris<Barycentric>::NoIntrinsicAcceleration,
InfiniteFuture,
prognosticator_parameters.adaptive_step_parameters,
FlightPlan::max_ephemeris_steps_per_frame,
/*last_point_only=*/false);
}
LOG_IF(INFO, !status.ok())
<< "Prognostication from " << prognosticator_parameters.first_time
<< " finished at " << prognostication->last().time() << " with "
<< status.ToString() << " for " << ShortDebugString();
return status;
}

void Vessel::SwapPrognostication(
std::unique_ptr<DiscreteTrajectory<Barycentric>>& prognostication,
Status const& status) {
prognosticator_lock_.AssertHeld();
if (status.error() != Error::CANCELLED) {
prognostication_.swap(prognostication);
prognosticator_status_ = status;
}
}

void Vessel::AppendToVesselTrajectory(
TrajectoryIterator const part_trajectory_begin,
TrajectoryIterator const part_trajectory_end,
@@ -559,6 +589,14 @@ void Vessel::AttachPrediction(
}
}

// Run the prognostication in both synchronous and asynchronous mode in tests to
// avoid code rot.
#if defined(_DEBUG)
std::atomic_bool Vessel::synchronous_(true);
#else
std::atomic_bool Vessel::synchronous_(false);
#endif

} // namespace internal_vessel
} // namespace ksp_plugin
} // namespace principia
21 changes: 19 additions & 2 deletions ksp_plugin/vessel.hpp
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@
#include <string>
#include <vector>

#include "absl/synchronization/mutex.h"
#include "base/status.hpp"
#include "ksp_plugin/celestial.hpp"
#include "ksp_plugin/flight_plan.hpp"
@@ -155,6 +156,9 @@ class Vessel {
// have a last time at or before |time|.
virtual void RefreshPrediction(Instant const& time);

// Returns "vessel_name (GUID)".
std::string ShortDebugString() const;

// The vessel must satisfy |is_initialized()|.
virtual void WriteToMessage(not_null<serialization::Vessel*> message,
PileUp::SerializationIndexForPileUp const&
@@ -169,8 +173,8 @@ class Vessel {
PileUp::PileUpForSerializationIndex const&
pile_up_for_serialization_index);

// Returns "vessel_name (GUID)".
std::string ShortDebugString() const;
static void MakeAsynchronous();
static void MakeSynchronous();

protected:
// For mocking.
@@ -197,6 +201,17 @@ class Vessel {
// prognostication.
void RepeatedlyFlowPrognostication();

// Runs the integrator to compute the |prognostication_| based on the given
// parameters.
Status FlowPrognostication(
PrognosticatorParameters const& prognosticator_parameters,
std::unique_ptr<DiscreteTrajectory<Barycentric>>& prognostication);

// Publishes the prognostication if the computation was not cancelled.
void SwapPrognostication(
std::unique_ptr<DiscreteTrajectory<Barycentric>>& prognostication,
Status const& status);

// Appends to |trajectory| the centre of mass of the trajectories of the parts
// denoted by |part_trajectory_begin| and |part_trajectory_end|.
void AppendToVesselTrajectory(TrajectoryIterator part_trajectory_begin,
@@ -240,6 +255,8 @@ class Vessel {
GUARDED_BY(prognosticator_lock_);

std::unique_ptr<FlightPlan> flight_plan_;

static std::atomic_bool synchronous_;
};

} // namespace internal_vessel