Flatten msgpack by replacing pack_array() with pack()

msgpack::unpacker is not a complete streaming deserializer. It returns maps/arrays as a whole but does not allow us to step into individual elements. There is some memory overhead and it is also likely less efficient. By flattening maps, we also no longer need to track how many fields a struct has, which is less error-prone.

src/indexer.h

@@ -239,7 +239,7 @@ template <typename TVisitor,
           typename Range>
 void Reflect(TVisitor& visitor,
              TypeDefDefinitionData<TypeId, FuncId, VarId, Range>& value) {
-  REFLECT_MEMBER_START(12);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(short_name);
   REFLECT_MEMBER(detailed_name);
   REFLECT_MEMBER(kind);
@@ -338,7 +338,7 @@ template <typename TVisitor,
 void Reflect(
     TVisitor& visitor,
     FuncDefDefinitionData<TypeId, FuncId, VarId, FuncRef, Range>& value) {
-  REFLECT_MEMBER_START(12);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(short_name);
   REFLECT_MEMBER(detailed_name);
   REFLECT_MEMBER(kind);
@@ -452,7 +452,7 @@ template <typename TVisitor,
           typename Range>
 void Reflect(TVisitor& visitor,
              VarDefDefinitionData<TypeId, FuncId, VarId, Range>& value) {
-  REFLECT_MEMBER_START(9);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(short_name);
   REFLECT_MEMBER(detailed_name);
   REFLECT_MEMBER(kind);

src/language_server_api.cc

@@ -194,7 +194,7 @@ void lsResponseError::Write(Writer& visitor) {
   auto& value = *this;
   int code2 = static_cast<int>(this->code);
 
-  visitor.StartObject(2 + !!data);
+  visitor.StartObject();
   REFLECT_MEMBER2("code", code2);
   REFLECT_MEMBER(message);
   if (data) {

src/language_server_api.h

@@ -226,7 +226,7 @@ struct lsCommand {
 };
 template <typename TVisitor, typename T>
 void Reflect(TVisitor& visitor, lsCommand<T>& value) {
-  REFLECT_MEMBER_START(3);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(title);
   REFLECT_MEMBER(command);
   REFLECT_MEMBER(arguments);
@@ -245,7 +245,7 @@ struct lsCodeLens {
 };
 template <typename TVisitor, typename TData, typename TCommandArguments>
 void Reflect(TVisitor& visitor, lsCodeLens<TData, TCommandArguments>& value) {
-  REFLECT_MEMBER_START(3);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(range);
   REFLECT_MEMBER(command);
   REFLECT_MEMBER(data);
@@ -979,7 +979,7 @@ struct Out_TextDocumentPublishDiagnostics
 template <typename TVisitor>
 void Reflect(TVisitor& visitor, Out_TextDocumentPublishDiagnostics& value) {
   std::string method = "textDocument/publishDiagnostics";
-  REFLECT_MEMBER_START(3);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(jsonrpc);
   REFLECT_MEMBER2("method", method);
   REFLECT_MEMBER(params);
@@ -1044,7 +1044,7 @@ struct Out_ShowLogMessage : public lsOutMessage<Out_ShowLogMessage> {
 };
 template <typename TVisitor>
 void Reflect(TVisitor& visitor, Out_ShowLogMessage& value) {
-  REFLECT_MEMBER_START(3);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(jsonrpc);
   std::string method = value.method();
   REFLECT_MEMBER2("method", method);

src/messages/text_document_hover.cc

@@ -58,7 +58,7 @@ struct Out_TextDocumentHover : public lsOutMessage<Out_TextDocumentHover> {
 };
 MAKE_REFLECT_STRUCT(Out_TextDocumentHover::Result, contents, range);
 void Reflect(Writer& visitor, Out_TextDocumentHover& value) {
-  REFLECT_MEMBER_START(3);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(jsonrpc);
   REFLECT_MEMBER(id);
   if (value.result)

src/position.cc

@@ -66,9 +66,9 @@ bool Position::operator!=(const Position& that) const {
 }
 
 bool Position::operator<(const Position& that) const {
-  if (line < that.line)
+  if (line != that.line)
-    return true;
+    return line < that.line;
-  return line == that.line && column < that.column;
+  return column < that.column;
 }
 
 Range::Range() {}
@@ -137,9 +137,9 @@ bool Range::operator!=(const Range& that) const {
 }
 
 bool Range::operator<(const Range& that) const {
-  if (start < that.start)
+  if (start != that.start)
-    return true;
+    return start < that.start;
-  return start == that.start && end < that.end;
+  return end < that.end;
 }
 
 // Position
@@ -148,18 +148,8 @@ void Reflect(Reader& visitor, Position& value) {
     std::string s = visitor.GetString();
     value = Position(s.c_str());
   } else {
-    int i = 0;
+    Reflect(visitor, value.line);
-    // FIXME
+    Reflect(visitor, value.column);
-    static_cast<MessagePackReader&>(visitor).IterArray([&](Reader& entry) {
-      switch (i++) {
-        case 0:
-          Reflect(entry, value.line);
-          break;
-        case 1:
-          Reflect(entry, value.column);
-          break;
-      }
-    });
   }
 }
 void Reflect(Writer& visitor, Position& value) {
@@ -167,10 +157,8 @@ void Reflect(Writer& visitor, Position& value) {
     std::string output = value.ToString();
     visitor.String(output.c_str(), output.size());
   } else {
-    REFLECT_MEMBER_START(2);
     Reflect(visitor, value.line);
     Reflect(visitor, value.column);
-    REFLECT_MEMBER_END();
   }
 }
 
@@ -180,25 +168,10 @@ void Reflect(Reader& visitor, Range& value) {
     std::string s = visitor.GetString();
     value = Range(s.c_str());
   } else {
-    int i = 0;
+    Reflect(visitor, value.start.line);
-    // FIXME
+    Reflect(visitor, value.start.column);
-    static_cast<MessagePackReader&>(visitor).IterArray([&](Reader& entry) {
+    Reflect(visitor, value.end.line);
-      switch (i++) {
+    Reflect(visitor, value.end.column);
-        case 0:
-          Reflect(entry, value.start.line);
-          break;
-        case 1:
-          Reflect(entry, value.start.column);
-          break;
-        case 2:
-          Reflect(entry, value.end.line);
-          break;
-        case 3:
-          Reflect(entry, value.end.column);
-          break;
-      }
-      i++;
-    });
   }
 }
 void Reflect(Writer& visitor, Range& value) {
@@ -206,11 +179,9 @@ void Reflect(Writer& visitor, Range& value) {
     std::string output = value.ToString();
     visitor.String(output.c_str(), output.size());
   } else {
-    REFLECT_MEMBER_START(4);
     Reflect(visitor, value.start.line);
     Reflect(visitor, value.start.column);
     Reflect(visitor, value.end.line);
     Reflect(visitor, value.end.column);
-    REFLECT_MEMBER_END();
   }
 }

src/query.h

@@ -161,7 +161,7 @@ struct MergeableUpdate {
 };
 template <typename TVisitor, typename TId, typename TValue>
 void Reflect(TVisitor& visitor, MergeableUpdate<TId, TValue>& value) {
-  REFLECT_MEMBER_START(3);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(id);
   REFLECT_MEMBER(to_add);
   REFLECT_MEMBER(to_remove);
@@ -177,7 +177,7 @@ struct WithUsr {
 };
 template <typename TVisitor, typename T>
 void Reflect(TVisitor& visitor, WithUsr<T>& value) {
-  REFLECT_MEMBER_START(2);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(usr);
   REFLECT_MEMBER(value);
   REFLECT_MEMBER_END();

src/serializer.cc

@@ -85,13 +85,13 @@ void ReflectMember(Writer& visitor, const char* name, std::string& value) {
 
 // TODO: Move this to indexer.cc
 void Reflect(Reader& visitor, IndexInclude& value) {
-  REFLECT_MEMBER_START(2);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(line);
   REFLECT_MEMBER(resolved_path);
   REFLECT_MEMBER_END();
 }
 void Reflect(Writer& visitor, IndexInclude& value) {
-  REFLECT_MEMBER_START(2);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER(line);
   if (gTestOutputMode) {
     std::string basename = GetBaseName(value.resolved_path);
@@ -106,7 +106,7 @@ void Reflect(Writer& visitor, IndexInclude& value) {
 
 template <typename TVisitor>
 void Reflect(TVisitor& visitor, IndexType& value) {
-  REFLECT_MEMBER_START(17);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER2("id", value.id);
   REFLECT_MEMBER2("usr", value.usr);
   REFLECT_MEMBER2("short_name", value.def.short_name);
@@ -129,7 +129,7 @@ void Reflect(TVisitor& visitor, IndexType& value) {
 
 template <typename TVisitor>
 void Reflect(TVisitor& visitor, IndexFunc& value) {
-  REFLECT_MEMBER_START(17);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER2("id", value.id);
   REFLECT_MEMBER2("is_operator", value.def.is_operator);
   REFLECT_MEMBER2("usr", value.usr);
@@ -152,7 +152,7 @@ void Reflect(TVisitor& visitor, IndexFunc& value) {
 
 template <typename TVisitor>
 void Reflect(TVisitor& visitor, IndexVar& value) {
-  REFLECT_MEMBER_START(13);
+  REFLECT_MEMBER_START();
   REFLECT_MEMBER2("id", value.id);
   REFLECT_MEMBER2("usr", value.usr);
   REFLECT_MEMBER2("short_name", value.def.short_name);
@@ -170,7 +170,7 @@ void Reflect(TVisitor& visitor, IndexVar& value) {
 }
 
 // IndexFile
-bool ReflectMemberStart(Writer& visitor, IndexFile& value, size_t n) {
+bool ReflectMemberStart(Writer& visitor, IndexFile& value) {
   auto it = value.id_cache.usr_to_type_id.find("");
   if (it != value.id_cache.usr_to_type_id.end()) {
     value.Resolve(it->second)->def.short_name = "<fundamental>";
@@ -178,12 +178,12 @@ bool ReflectMemberStart(Writer& visitor, IndexFile& value, size_t n) {
   }
 
   value.version = IndexFile::kCurrentVersion;
-  DefaultReflectMemberStart(visitor, n);
+  DefaultReflectMemberStart(visitor);
   return true;
 }
 template <typename TVisitor>
 void Reflect(TVisitor& visitor, IndexFile& value) {
-  REFLECT_MEMBER_START(5 + (gTestOutputMode ? 0 : 6));
+  REFLECT_MEMBER_START();
   if (!gTestOutputMode) {
     REFLECT_MEMBER(version);
     REFLECT_MEMBER(last_modification_time);
@@ -257,10 +257,12 @@ std::unique_ptr<IndexFile> Deserialize(SerializeFormat format,
       if (serialized.empty())
         return nullptr;
       try {
-        msgpack::object_handle oh =
+        msgpack::unpacker upk;
-            msgpack::unpack(serialized.data(), serialized.size());
+        upk.reserve_buffer(serialized.size());
+        memcpy(upk.buffer(), serialized.data(), serialized.size());
+        upk.buffer_consumed(serialized.size());
         file = MakeUnique<IndexFile>(path);
-        MessagePackReader reader(oh.get());
+        MessagePackReader reader(&upk);
         Reflect(reader, *file);
         if (file->version != expected_version)
           return nullptr;

src/serializer.h

@@ -23,6 +23,7 @@ class Reader {
   //virtual bool IsUint64() = 0;
   virtual bool IsString() = 0;
 
+  virtual void GetNull() = 0;
   virtual bool GetBool() = 0;
   virtual int GetInt() = 0;
   virtual int64_t GetInt64() = 0;
@@ -50,19 +51,19 @@ class Writer {
   virtual void String(const char* x, size_t len) = 0;
   virtual void StartArray(size_t) = 0;
   virtual void EndArray() = 0;
-  virtual void StartObject(size_t) = 0;
+  virtual void StartObject() = 0;
   virtual void EndObject() = 0;
   virtual void Key(const char* name) = 0;
 };
 
 struct IndexFile;
 
-#define REFLECT_MEMBER_START(n)             \
+#define REFLECT_MEMBER_START()                  \
-  if (!ReflectMemberStart(visitor, value, n)) \
+  if (!ReflectMemberStart(visitor, value))      \
-  return
+    return
-#define REFLECT_MEMBER_START1(value)       \
+#define REFLECT_MEMBER_START1(value)            \
-  if (!ReflectMemberStart(visitor, value)) \
+  if (!ReflectMemberStart(visitor, value))      \
-  return
+    return
 #define REFLECT_MEMBER_END() ReflectMemberEnd(visitor, value);
 #define REFLECT_MEMBER_END1(value) ReflectMemberEnd(visitor, value);
 #define REFLECT_MEMBER(name) ReflectMember(visitor, #name, value.name)
@@ -76,38 +77,38 @@ struct IndexFile;
     value = static_cast<type>(value0);           \
   }
 
-// clang-format off
-// Config has many fields, we need to support at least its number of fields.
-#define NUM_VA_ARGS_IMPL(_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,_11,_12,_13,_14,_15,_16,_17,_18,_19,_20,_21,_22,_23,_24,_25,_26,_27,_28,_29,_30,N,...) N
-#define NUM_VA_ARGS(...) NUM_VA_ARGS_IMPL(__VA_ARGS__,30,29,28,27,26,25,24,23,22,21,20,19,18,17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,1)
-// clang-format on
-
 #define _MAPPABLE_REFLECT_MEMBER(name) REFLECT_MEMBER(name);
 
 #define MAKE_REFLECT_EMPTY_STRUCT(type, ...)     \
   template <typename TVisitor>                   \
   void Reflect(TVisitor& visitor, type& value) { \
-    REFLECT_MEMBER_START(0);                     \
+    REFLECT_MEMBER_START();                      \
     REFLECT_MEMBER_END();                        \
   }
 
 #define MAKE_REFLECT_STRUCT(type, ...)               \
   template <typename TVisitor>                       \
   void Reflect(TVisitor& visitor, type& value) {     \
-    REFLECT_MEMBER_START(NUM_VA_ARGS(__VA_ARGS__));  \
+    REFLECT_MEMBER_START();                          \
     MACRO_MAP(_MAPPABLE_REFLECT_MEMBER, __VA_ARGS__) \
     REFLECT_MEMBER_END();                            \
   }
 
+// clang-format off
+// Config has many fields, we need to support at least its number of fields.
+#define NUM_VA_ARGS_IMPL(_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,_11,_12,_13,_14,_15,_16,_17,_18,_19,_20,_21,_22,_23,_24,_25,_26,_27,_28,_29,_30,N,...) N
+#define NUM_VA_ARGS(...) NUM_VA_ARGS_IMPL(__VA_ARGS__,30,29,28,27,26,25,24,23,22,21,20,19,18,17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,1)
+// clang-format on
+
 #define _MAPPABLE_REFLECT_ARRAY(name) Reflect(visitor, value.name);
 
 // Reflects the struct so it is serialized as an array instead of an object.
 // This currently only supports writers.
-#define MAKE_REFLECT_STRUCT_WRITER_AS_ARRAY(type, ...) \
+#define MAKE_REFLECT_STRUCT_WRITER_AS_ARRAY(type, ...)  \
-  inline void Reflect(Writer& visitor, type& value) {  \
+  inline void Reflect(Writer& visitor, type& value) {   \
-    visitor.StartArray(NUM_VA_ARGS(__VA_ARGS__));      \
+    visitor.StartArray(NUM_VA_ARGS(__VA_ARGS__));       \
-    MACRO_MAP(_MAPPABLE_REFLECT_ARRAY, __VA_ARGS__)    \
+    MACRO_MAP(_MAPPABLE_REFLECT_ARRAY, __VA_ARGS__)     \
-    visitor.EndArray();                                \
+        visitor.EndArray();                             \
   }
 
 // API:
@@ -157,8 +158,10 @@ void Reflect(Writer& visitor, std::string& value);
 // std::optional
 template <typename T>
 void Reflect(Reader& visitor, optional<T>& value) {
-  if (visitor.IsNull())
+  if (visitor.IsNull()) {
+    visitor.GetNull();
     return;
+  }
   T real_value{};
   Reflect(visitor, real_value);
   value = real_value;
@@ -201,12 +204,12 @@ void Reflect(Writer& visitor, std::vector<T>& values) {
 
 // Writer:
 
-inline void DefaultReflectMemberStart(Writer& visitor, size_t n) {
+inline void DefaultReflectMemberStart(Writer& visitor) {
-  visitor.StartObject(n);
+  visitor.StartObject();
 }
 template <typename T>
-bool ReflectMemberStart(Writer& visitor, T& value, size_t n) {
+bool ReflectMemberStart(Writer& visitor, T& value) {
-  visitor.StartObject(n);
+  visitor.StartObject();
   return true;
 }
 template <typename T>
@@ -238,9 +241,9 @@ void ReflectMember(Writer& visitor, const char* name, std::string& value);
 // Reader:
 
 
-inline void DefaultReflectMemberStart(Reader& visitor, size_t n) {}
+inline void DefaultReflectMemberStart(Reader& visitor) {}
 template <typename T>
-bool ReflectMemberStart(Reader& visitor, T& value, size_t n) {
+bool ReflectMemberStart(Reader& visitor, T& value) {
   return true;
 }
 template <typename T>

src/serializers/json.h

@@ -20,6 +20,7 @@ class JsonReader : public Reader {
   //bool IsUint64() override { return m_->IsUint64(); }
   bool IsString() override { return m_->IsString(); }
 
+  void GetNull() override {}
   bool GetBool() override { return m_->GetBool(); }
   int GetInt() override { return m_->GetInt(); }
   int64_t GetInt64() override { return m_->GetInt64(); }
@@ -67,7 +68,7 @@ class JsonWriter : public Writer {
   void String(const char* x, size_t len) override { m_->String(x, len); }
   void StartArray(size_t) override { m_->StartArray(); }
   void EndArray() override { m_->EndArray(); }
-  void StartObject(size_t) override { m_->StartObject(); }
+  void StartObject() override { m_->StartObject(); }
   void EndObject() override { m_->EndObject(); }
   void Key(const char* name) override { m_->Key(name); }
 };

src/serializers/msgpack.h

@@ -5,27 +5,34 @@
 #include <msgpack.hpp>
 
 class MessagePackReader : public Reader {
-  msgpack::object o_;
+  msgpack::unpacker* pk_;
-  size_t idx_ = 0;
+  msgpack::object_handle oh_;
-  std::vector<msgpack::object> children_;
+
+  template <typename T>
+  T Get() {
+    T ret = oh_.get().as<T>();
+    pk_->next(oh_);
+    return ret;
+  }
 
  public:
-  MessagePackReader(msgpack::object o) : o_(o) {}
+  MessagePackReader(msgpack::unpacker* pk) : pk_(pk) { pk->next(oh_); }
   SerializeFormat Format() const override { return SerializeFormat::MessagePack; }
 
-  bool IsNull() override { return o_.is_nil(); }
+  bool IsNull() override { return oh_.get().is_nil(); }
-  bool IsArray() override { return o_.type == msgpack::type::ARRAY; }
+  bool IsArray() override { return oh_.get().type == msgpack::type::ARRAY; }
   bool IsInt() override {
-    return o_.type == msgpack::type::POSITIVE_INTEGER ||
+    return oh_.get().type == msgpack::type::POSITIVE_INTEGER ||
-           o_.type == msgpack::type::NEGATIVE_INTEGER;
+           oh_.get().type == msgpack::type::NEGATIVE_INTEGER;
   }
-  bool IsString() override { return o_.type == msgpack::type::STR; }
+  bool IsString() override { return oh_.get().type == msgpack::type::STR; }
 
-  bool GetBool() override { return o_.as<bool>(); }
+  void GetNull() override { pk_->next(oh_); }
-  int GetInt() override { return o_.as<int>(); }
+  bool GetBool() override { return Get<bool>(); }
-  int64_t GetInt64() override { return o_.as<int64_t>(); }
+  int GetInt() override { return Get<int>(); }
-  uint64_t GetUint64() override { return o_.as<uint64_t>(); }
+  int64_t GetInt64() override { return Get<int64_t>(); }
-  std::string GetString() override { return o_.as<std::string>(); }
+  uint64_t GetUint64() override { return Get<uint64_t>(); }
+  std::string GetString() override { return Get<std::string>(); }
 
   bool HasMember(const char* x) override { return true; }
   std::unique_ptr<Reader> operator[](const char* x) override {
@@ -33,17 +40,13 @@ class MessagePackReader : public Reader {
   }
 
   void IterArray(std::function<void(Reader&)> fn) override {
-    for (auto& entry : o_.as<std::vector<msgpack::object>>()) {
+    size_t n = Get<size_t>();
-      MessagePackReader sub(entry);
+    for (size_t i = 0; i < n; i++)
-      fn(sub);
+      fn(*this);
-    }
   }
 
-  void DoMember(const char* name, std::function<void(Reader&)> fn) override {
+  void DoMember(const char*, std::function<void(Reader&)> fn) override {
-    if (idx_ == 0)
+    fn(*this);
-      children_ = o_.as<std::vector<msgpack::object>>();
-    MessagePackReader sub(children_[idx_++]);
-    fn(sub);
   }
 };
 
@@ -62,9 +65,9 @@ class MessagePackWriter : public Writer {
   void String(const char* x) override { m_->pack(x); }
   // TODO Remove std::string
   void String(const char* x, size_t len) override { m_->pack(std::string(x, len)); }
-  void StartArray(size_t n) override { m_->pack_array(uint32_t(n)); }
+  void StartArray(size_t n) override { m_->pack(n); }
   void EndArray() override {}
-  void StartObject(size_t n) override { m_->pack_array(uint32_t(n)); }
+  void StartObject() override {}
   void EndObject() override {}
   void Key(const char* name) override {}
 };