Merge pull request #3805 from pleroy/3230

More microoptimizations

ksp_plugin/identification.hpp

@@ -1,10 +1,9 @@
 #pragma once
 
-#include <limits>
-#include <map>
 #include <set>
 #include <string>
 
+#include "absl/container/btree_map.h"
 #include "base/macros.hpp"  // 🧙 For forward declarations.
 #include "base/not_null.hpp"
 
@@ -20,7 +19,7 @@ namespace internal {
 using namespace principia::base::_not_null;
 
 // The GUID of a vessel, obtained by |v.id.ToString()| in C#. We use this as a
-// key in an |std::map|.
+// key in a map.
 using GUID = std::string;
 
 // Corresponds to KSP's |Part.flightID|, *not* to |Part.uid|.  C#'s |uint|
@@ -44,9 +43,9 @@ struct VesselByGUIDComparator {
 };
 
 template<typename T>
-using PartTo = std::map<not_null<Part*>,
-                        T,
-                        PartByPartIdComparator>;
+using PartTo = absl::btree_map<not_null<Part*>,
+                               T,
+                               PartByPartIdComparator>;
 using VesselSet = std::set<not_null<Vessel*>,
                            VesselByGUIDComparator>;
 using VesselConstSet = std::set<not_null<Vessel const*>,

ksp_plugin/plugin.cpp

@@ -360,9 +360,9 @@ void Plugin::InsertOrKeepVessel(GUID const& vessel_guid,
   if (vit == vessels_.end()) {
     // Restore the zombie parameters if we have some, otherwise use the default.
     auto prediction_parameters = DefaultPredictionParameters();
-    auto const pit =
-        zombie_prediction_adaptive_step_parameters_.find(vessel_guid);
-    if (pit != zombie_prediction_adaptive_step_parameters_.end()) {
+    if (auto const pit =
+            zombie_prediction_adaptive_step_parameters_.find(vessel_guid);
+        pit != zombie_prediction_adaptive_step_parameters_.end()) {
       prediction_parameters = pit->second;
       zombie_prediction_adaptive_step_parameters_.erase(pit);
     }
@@ -447,9 +447,8 @@ void Plugin::InsertOrKeepLoadedPart(
       main_body_frame.FromThisFrameAtTime(previous_time) *
       world_to_main_body_centred;
 
-  auto it = part_id_to_vessel_.find(part_id);
-  bool const part_found = it != part_id_to_vessel_.end();
-  if (part_found) {
+  if (auto const it = part_id_to_vessel_.find(part_id);
+      it != part_id_to_vessel_.end()) {
     not_null<Vessel*>& associated_vessel = it->second;
     not_null<Vessel*> const current_vessel = associated_vessel;
     if (vessel == current_vessel) {
@@ -509,11 +508,12 @@ void Plugin::ApplyPartIntrinsicTorque(
 }
 
 bool Plugin::PartIsTruthful(PartId const part_id) const {
-  auto const it = part_id_to_vessel_.find(part_id);
-  if (it == part_id_to_vessel_.end()) {
+  if (auto const it = part_id_to_vessel_.find(part_id);
+      it == part_id_to_vessel_.end()) {
     return false;
+  } else {
+    return it->second->part(part_id)->truthful();
   }
-  return it->second->part(part_id)->truthful();
 }
 
 void Plugin::PrepareToReportCollisions() {
@@ -662,7 +662,7 @@ void Plugin::SetPartApparentRigidMotion(
       Identity<Barycentric, Apparent>()(angular_velocity_of_world_),
       Apparent::unmoving};
 
-  not_null<Vessel*> vessel = FindOrDie(part_id_to_vessel_, part_id);
+  not_null<Vessel*> const vessel = FindOrDie(part_id_to_vessel_, part_id);
   CHECK(is_loaded(vessel));
   not_null<Part*> const part = vessel->part(part_id);
   CHECK(part->is_piled_up());
@@ -1693,10 +1693,11 @@ void Plugin::AddPart(not_null<Vessel*> const vessel,
                      PartId const part_id,
                      std::string const& name,
                      Args... args) {
-  auto const [it, inserted] = part_id_to_vessel_.emplace(part_id, vessel);
+  auto const [_, inserted] = part_id_to_vessel_.emplace(part_id, vessel);
   CHECK(inserted) << NAMED(part_id);
-  auto deletion_callback = [it = it, &map = part_id_to_vessel_] {
-    map.erase(it);
+  auto deletion_callback = [part_id, &map = part_id_to_vessel_] {
+    // This entails a lookup, but iterators are not stable in |flat_hash_map|.
+    map.erase(part_id);
   };
   auto part = make_not_null_unique<Part>(part_id,
                                          name,

ksp_plugin/plugin.hpp

@@ -11,6 +11,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
 #include "base/disjoint_sets.hpp"
 #include "base/monostable.hpp"
@@ -99,7 +100,7 @@ using namespace principia::quantities::_quantities;
 using namespace principia::quantities::_si;
 
 // The index of a body in |FlightGlobals.Bodies|, obtained by
-// |b.flightGlobalsIndex| in C#. We use this as a key in an |std::map|.
+// |b.flightGlobalsIndex| in C#. We use this as a key in a map.
 using Index = int;
 
 class Plugin {
@@ -536,7 +537,7 @@ class Plugin {
   GUIDToOwnedVessel vessels_;
   // For each part, the vessel that this part belongs to. The part is guaranteed
   // to be in the parts() map of the vessel, and owned by it.
-  std::map<PartId, not_null<Vessel*>> part_id_to_vessel_;
+  absl::flat_hash_map<PartId, not_null<Vessel*>> part_id_to_vessel_;
   IndexToOwnedCelestial celestials_;
 
   // Not null after initialization.

ksp_plugin/vessel.hpp

@@ -1,14 +1,13 @@
 #pragma once
 
-#include <list>
-#include <map>
 #include <memory>
 #include <queue>
-#include <set>
 #include <string>
 #include <variant>
 #include <vector>
 
+#include "absl/container/btree_map.h"
+#include "absl/container/flat_hash_set.h"
 #include "absl/status/status.h"
 #include "absl/synchronization/mutex.h"
 #include "base/not_null.hpp"
@@ -405,8 +404,10 @@ class Vessel {
   // non-collapsible as we don't know anything about it.
   bool is_collapsible_ = false;
 
-  std::map<PartId, not_null<std::unique_ptr<Part>>> parts_;
-  std::set<PartId> kept_parts_;
+  // Use an ordered map for consistent order of iteration (e.g., when computing
+  // the barycentre).
+  absl::btree_map<PartId, not_null<std::unique_ptr<Part>>> parts_;
+  absl::flat_hash_set<PartId> kept_parts_;
 
   // The vessel trajectory is made of a number of history segments ending at the
   // backstory and (most of the time) the psychohistory and prediction.  The

ksp_plugin_adapter/ksp_plugin_adapter.cs

@@ -537,9 +537,10 @@ private void UpdateVessel(Vessel vessel, double universal_time) {
           var part_rotation = (UnityEngine.QuaternionD)part_actual_motion.r;
           var part_angular_velocity = (Vector3d)part_actual_motion.w;
           part_rb.position = root_part_rb.position + part_position;
-          part_rb.transform.position = root_part_rb.position + part_position;
           part_rb.rotation = part_rotation;
-          part_rb.transform.rotation = part_rotation;
+          part_rb.transform.SetPositionAndRotation(
+              root_part_rb.position + part_position,
+              part_rotation);
           part_rb.velocity = root_part_rb.velocity + part_velocity;
           part_rb.angularVelocity = part_angular_velocity;
         }
@@ -587,10 +588,16 @@ private bool is_manageable(Vessel vessel) {
     return UnmanageabilityReasons(vessel) == null;
   }
 
-  public static bool is_lit_solid_booster(ModuleEngines module) {
-    return module != null &&
-           module.EngineIgnited &&
-           module.engineType == EngineType.SolidBooster;
+  public static bool is_lit_solid_booster(Part part) {
+    foreach (PartModule module in part.Modules) {
+      var module_engines = module as ModuleEngines;
+      if (module_engines != null &&
+          module_engines.EngineIgnited &&
+          module_engines.engineType == EngineType.SolidBooster) {
+        return true;
+      }
+    }
+    return false;
   }
 
   private string UnmanageabilityReasons(Vessel vessel) {
@@ -1169,6 +1176,7 @@ private System.Collections.IEnumerator WaitedForFixedUpdate() {
       // |WaitForFixedUpdate|, since some may be destroyed (by collisions) during
       // the physics step.  See also #1281.
       foreach (Vessel vessel in FlightGlobals.Vessels) {
+        int main_body_index = vessel.mainBody.flightGlobalsIndex;
         string vessel_guid = vessel.id.ToString();
         string unmanageability_reasons = UnmanageabilityReasons(vessel);
         if (unmanageability_reasons != null) {
@@ -1181,19 +1189,17 @@ private System.Collections.IEnumerator WaitedForFixedUpdate() {
 
         plugin_.InsertOrKeepVessel(vessel_guid,
                                    vessel.vesselName,
-                                   vessel.mainBody.flightGlobalsIndex,
+                                   main_body_index,
                                    !vessel.packed,
                                    out bool inserted);
         if (!vessel.packed) {
           Profiler.BeginSample("InsertOrKeepLoadedPart");
           foreach (Part part in vessel.parts.Where(PartIsFaithful)) {
             UnityEngine.Rigidbody part_rb = part.rb;
             uint part_id = part.flightID;
-            QP degrees_of_freedom;
-            if (part_id_to_degrees_of_freedom_.ContainsKey(part_id)) {
-              degrees_of_freedom =
-                  part_id_to_degrees_of_freedom_[part_id];
-            } else {
+            if (!part_id_to_degrees_of_freedom_.TryGetValue(
+                    part_id,
+                    out QP degrees_of_freedom)) {
               // Assumptions about the invariants of KSP will invariably fail.
               // This is a graceful fallback.
               Log.Error("Unpacked part " + part.name + " (" +
@@ -1219,35 +1225,33 @@ private System.Collections.IEnumerator WaitedForFixedUpdate() {
                 (XYZ)part_rb.centerOfMass,
                 (XYZ)part_rb.inertiaTensor,
                 (WXYZ)part_rb.inertiaTensorRotation,
-                (from PartModule module in part.Modules
-                 select module as ModuleEngines).Any(is_lit_solid_booster),
+                is_lit_solid_booster(part),
                 vessel_guid,
-                vessel.mainBody.flightGlobalsIndex,
+                main_body_index,
                 main_body_degrees_of_freedom,
                 degrees_of_freedom,
                 (WXYZ)part_rb.rotation,
                 (XYZ)part_rb.angularVelocity,
                 Δt);
-            if (part_id_to_intrinsic_torque_.ContainsKey(part_id)) {
-              plugin_.PartApplyIntrinsicTorque(part_id,
-                                               (XYZ)part_id_to_intrinsic_torque_
-                                                   [part_id]);
+            if (part_id_to_intrinsic_torque_.TryGetValue(
+                    part_id,
+                    out Vector3d intrinsic_torque)) {
+              plugin_.PartApplyIntrinsicTorque(part_id, (XYZ)intrinsic_torque);
             }
-            if (part_id_to_intrinsic_force_.ContainsKey(part_id)) {
+            if (part_id_to_intrinsic_force_.TryGetValue(part_id,
+                  out Vector3d intrinsic_force)) {
               // When a Kerbal is doing an EVA and holding on to a ladder, the
               // ladder imbues them with their weight at the location of the
               // vessel to which the ladder is attached.  This leads to fantastic
               // effects where doing an EVA accelerates the vessel, see #1415.
               // Just say no to stupidity.
               if (!(vessel.isEVA && vessel.evaController.OnALadder)) {
-                plugin_.PartApplyIntrinsicForce(
-                    part_id,
-                    (XYZ)part_id_to_intrinsic_force_[part_id]);
+                plugin_.PartApplyIntrinsicForce(part_id, (XYZ)intrinsic_force);
               }
             }
-            if (part_id_to_intrinsic_forces_.ContainsKey(part_id)) {
-              foreach (var part_centred_force in part_id_to_intrinsic_forces_[
-                  part_id]) {
+            if (part_id_to_intrinsic_forces_.TryGetValue(part_id,
+                    out PartCentredForceHolder[] intrinsic_forces)) {
+              foreach (var part_centred_force in intrinsic_forces) {
                 plugin_.PartApplyIntrinsicForceAtPosition(
                     part_id,
                     (XYZ)part_centred_force.force,
@@ -1480,9 +1484,10 @@ private System.Collections.IEnumerator WaitedForFixedUpdate() {
             // See https://github.com/mockingbirdnest/Principia/pull/1427,
             // https://github.com/mockingbirdnest/Principia/issues/1307#issuecomment-478337241.
             part_rb.position = part_position;
-            part_rb.transform.position = part_position;
             part_rb.rotation = part_rotation;
-            part_rb.transform.rotation = part_rotation;
+            part_rb.transform.SetPositionAndRotation(
+                part_position,
+                part_rotation);
 
             part_rb.velocity = part_velocity;
             part_rb.angularVelocity = part_angular_velocity;
@@ -1704,10 +1709,9 @@ private void JaiFailliAttendre() {
           Part physical_parent = closest_physical_parent(part);
           if (part.bodyLiftLocalVector != UnityEngine.Vector3.zero ||
               part.dragVector != UnityEngine.Vector3.zero) {
-            if (part_id_to_intrinsic_forces_.ContainsKey(
-                physical_parent.flightID)) {
-              var previous_holder =
-                  part_id_to_intrinsic_forces_[physical_parent.flightID];
+            if (part_id_to_intrinsic_forces_.TryGetValue(
+                    physical_parent.flightID,
+                    out PartCentredForceHolder[] previous_holder)) {
               part_id_to_intrinsic_forces_[physical_parent.flightID] =
                   new PartCentredForceHolder[previous_holder.Length + 2];
               previous_holder.CopyTo(