From 9b405c3dc8a422ea2d7fd7a8349d2ff609ba5ff5 Mon Sep 17 00:00:00 2001
From: Michael Grupp <grupp@magazino.eu>
Date: Mon, 17 Aug 2020 19:34:35 +0200
Subject: [PATCH] Deactivate memory-critical histogram backwards compatibility.
 (#1710)

It's essentially leading to buffering all submaps twice, which
is a high (~2x) peak in memory consumption during deserialization.

Don't convert anymore since this affects _all_ pbstreams and not only
the (probably few) old pbstreams that require the conversion.

Signed-off-by: Michael Grupp <grupp@magazino.eu>
---
 .../cloud/internal/client_server_test.cc      |  2 +-
 cartographer/mapping/map_builder.cc           | 33 +++++++------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/cartographer/cloud/internal/client_server_test.cc b/cartographer/cloud/internal/client_server_test.cc
index 90c2dd7..8a08971 100644
--- a/cartographer/cloud/internal/client_server_test.cc
+++ b/cartographer/cloud/internal/client_server_test.cc
@@ -55,7 +55,7 @@ constexpr double kDuration = 4.;         // Seconds.
 constexpr double kTimeStep = 0.1;        // Seconds.
 constexpr double kTravelDistance = 1.2;  // Meters.
 
-constexpr char kSerializationHeaderProtoString[] = "format_version: 1";
+constexpr char kSerializationHeaderProtoString[] = "format_version: 2";
 constexpr char kPoseGraphProtoString[] = R"(pose_graph {
       trajectory: {
         trajectory_id: 0
diff --git a/cartographer/mapping/map_builder.cc b/cartographer/mapping/map_builder.cc
index 331ee6e..c93cd12 100644
--- a/cartographer/mapping/map_builder.cc
+++ b/cartographer/mapping/map_builder.cc
@@ -285,8 +285,14 @@ std::map<int, int> MapBuilder::LoadState(
                                  true);
   }
 
-  MapById<SubmapId, mapping::proto::Submap> submap_id_to_submap;
-  MapById<NodeId, mapping::proto::Node> node_id_to_node;
+  if (options_.use_trajectory_builder_3d()) {
+    CHECK_NE(deserializer.header().format_version(),
+             io::kFormatVersionWithoutSubmapHistograms)
+        << "The pbstream file contains submaps without rotational histograms. "
+           "This can be converted with the 'pbstream migrate' tool, see the "
+           "Cartographer documentation for details. ";
+  }
+
   SerializedData proto;
   while (deserializer.ReadNextSerializedData(&proto)) {
     switch (proto.data_case()) {
@@ -303,10 +309,10 @@ std::map<int, int> MapBuilder::LoadState(
         proto.mutable_submap()->mutable_submap_id()->set_trajectory_id(
             trajectory_remapping.at(
                 proto.submap().submap_id().trajectory_id()));
-        submap_id_to_submap.Insert(
-            SubmapId{proto.submap().submap_id().trajectory_id(),
-                     proto.submap().submap_id().submap_index()},
-            proto.submap());
+        const SubmapId submap_id(proto.submap().submap_id().trajectory_id(),
+                                 proto.submap().submap_id().submap_index());
+        pose_graph_->AddSubmapFromProto(submap_poses.at(submap_id),
+                                        proto.submap());
         break;
       }
       case SerializedData::kNode: {
@@ -316,7 +322,6 @@ std::map<int, int> MapBuilder::LoadState(
                              proto.node().node_id().node_index());
         const transform::Rigid3d& node_pose = node_poses.at(node_id);
         pose_graph_->AddNodeFromProto(node_pose, proto.node());
-        node_id_to_node.Insert(node_id, proto.node());
         break;
       }
       case SerializedData::kTrajectoryData: {
@@ -361,20 +366,6 @@ std::map<int, int> MapBuilder::LoadState(
     }
   }
 
-  // TODO(schwoere): Remove backwards compatibility once the pbstream format
-  // version 2 is established.
-  if (deserializer.header().format_version() ==
-      io::kFormatVersionWithoutSubmapHistograms) {
-    submap_id_to_submap =
-        cartographer::io::MigrateSubmapFormatVersion1ToVersion2(
-            submap_id_to_submap, node_id_to_node, pose_graph_proto);
-  }
-
-  for (const auto& submap_id_submap : submap_id_to_submap) {
-    pose_graph_->AddSubmapFromProto(submap_poses.at(submap_id_submap.id),
-                                    submap_id_submap.data);
-  }
-
   if (load_frozen_state) {
     // Add information about which nodes belong to which submap.
     // Required for 3D pure localization.