JDevlieghere created this revision.
JDevlieghere added a reviewer: Michael137.
Herald added a project: All.
JDevlieghere requested review of this revision.

Improve memory usage by reducing the lifetime of CTF types. Once a CTF type has 
been converted to a (complete) LLDB type, there's no need to keep it in memory 
anymore. For most types, we can free them right after creating the 
corresponding LLDB types. The only exception is record types, which are only 
completed lazily.

This patch also adds LLVM RTTI support to CTF type. This was suggested in 
D156498 <https://reviews.llvm.org/D156498> but with just one caller that didn't 
seem worth it yet. This patch introduced a second call site which tipped the 
scales.


https://reviews.llvm.org/D156606

Files:
  lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h
  lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
  lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.h

Index: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.h
+++ lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.h
@@ -245,15 +245,17 @@
 
   std::optional<ctf_header_t> m_header;
 
-  std::vector<std::unique_ptr<CTFType>> m_ctf_types;
+  /// Parsed CTF types.
+  llvm::DenseMap<lldb::user_id_t, std::unique_ptr<CTFType>> m_ctf_types;
+
+  /// Parsed LLDB types.
+  llvm::DenseMap<lldb::user_id_t, lldb::TypeSP> m_types;
 
   /// To complete types, we need a way to map (imcomplete) compiler types back
   /// to parsed CTF types.
   llvm::DenseMap<lldb::opaque_compiler_type_t, const CTFType *>
       m_compiler_types;
 
-  llvm::DenseMap<lldb::user_id_t, lldb::TypeSP> m_types;
-
   std::vector<lldb::FunctionSP> m_functions;
   std::vector<lldb::VariableSP> m_variables;
 
Index: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
+++ lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
@@ -523,8 +523,7 @@
   assert(ctf_type && "m_compiler_types should only contain valid CTF types");
 
   // We only support resolving record types.
-  assert(ctf_type->kind == CTFType::Kind::eStruct ||
-         ctf_type->kind == CTFType::Kind::eUnion);
+  assert(llvm::isa<CTFRecord>(ctf_type));
 
   // Cast to the appropriate CTF type.
   const CTFRecord *ctf_record = static_cast<const CTFRecord *>(ctf_type);
@@ -551,9 +550,10 @@
   }
   m_ast->CompleteTagDeclarationDefinition(compiler_type);
 
-  // Now that the compiler type is no longer incomplete we don't need to
-  // remember it anymore.
+  // Now that the compiler type is complete, we don't need to remember it
+  // anymore and can remove the CTF record type.
   m_compiler_types.erase(compiler_type.GetOpaqueQualType());
+  m_ctf_types.erase(ctf_type->uid);
 
   return true;
 }
@@ -727,9 +727,8 @@
     llvm::Expected<std::unique_ptr<CTFType>> type_or_error =
         ParseType(type_offset, type_uid);
     if (type_or_error) {
-      m_ctf_types.emplace_back(std::move(*type_or_error));
+      m_ctf_types[(*type_or_error)->uid] = std::move(*type_or_error);
     } else {
-      m_ctf_types.emplace_back(std::unique_ptr<CTFType>());
       LLDB_LOG_ERROR(log, type_or_error.takeError(),
                      "Failed to parse type {1} at offset {2}: {0}", type_uid,
                      type_offset);
@@ -982,16 +981,16 @@
 }
 
 lldb_private::Type *SymbolFileCTF::ResolveTypeUID(lldb::user_id_t type_uid) {
-  auto find_result = m_types.find(type_uid);
-  if (find_result != m_types.end())
-    return find_result->second.get();
+  auto type_it = m_types.find(type_uid);
+  if (type_it != m_types.end())
+    return type_it->second.get();
 
-  if (type_uid == 0 || type_uid > m_ctf_types.size())
+  auto ctf_type_it = m_ctf_types.find(type_uid);
+  if (ctf_type_it == m_ctf_types.end())
     return nullptr;
 
-  CTFType *ctf_type = m_ctf_types[type_uid - 1].get();
-  if (!ctf_type)
-    return nullptr;
+  CTFType *ctf_type = ctf_type_it->second.get();
+  assert(ctf_type && "m_ctf_types should only contain valid CTF types");
 
   Log *log = GetLog(LLDBLog::Symbols);
 
@@ -1013,6 +1012,11 @@
 
   m_types[type_uid] = type_sp;
 
+  // Except for record types which we'll need to complete later, we don't need
+  // the CTF type anymore.
+  if (!isa<CTFRecord>(ctf_type))
+    m_ctf_types.erase(type_uid);
+
   return type_sp.get();
 }
 
Index: lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h
===================================================================
--- lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h
+++ lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h
@@ -46,6 +46,8 @@
              uint32_t encoding)
       : CTFType(eInteger, uid, name), bits(bits), encoding(encoding) {}
 
+  static bool classof(const CTFType *T) { return T->kind == eInteger; }
+
   uint32_t bits;
   uint32_t encoding;
 };
@@ -55,6 +57,11 @@
   CTFModifier(Kind kind, lldb::user_id_t uid, uint32_t type)
       : CTFType(kind, uid, ""), type(type) {}
 
+  static bool classof(const CTFType *T) {
+    return T->kind == ePointer || T->kind == eConst || T->kind == eVolatile ||
+           T->kind == eRestrict;
+  }
+
 public:
   uint32_t type;
 };
@@ -62,27 +69,36 @@
 struct CTFPointer : public CTFModifier {
   CTFPointer(lldb::user_id_t uid, uint32_t type)
       : CTFModifier(ePointer, uid, type) {}
+
+  static bool classof(const CTFType *T) { return T->kind == ePointer; }
 };
 
 struct CTFConst : public CTFModifier {
   CTFConst(lldb::user_id_t uid, uint32_t type)
       : CTFModifier(eConst, uid, type) {}
+
+  static bool classof(const CTFType *T) { return T->kind == eConst; }
 };
 
 struct CTFVolatile : public CTFModifier {
   CTFVolatile(lldb::user_id_t uid, uint32_t type)
       : CTFModifier(eVolatile, uid, type) {}
+
+  static bool classof(const CTFType *T) { return T->kind == eVolatile; }
 };
 
 struct CTFRestrict : public CTFModifier {
   CTFRestrict(lldb::user_id_t uid, uint32_t type)
       : CTFModifier(eRestrict, uid, type) {}
+  static bool classof(const CTFType *T) { return T->kind == eRestrict; }
 };
 
 struct CTFTypedef : public CTFType {
   CTFTypedef(lldb::user_id_t uid, llvm::StringRef name, uint32_t type)
       : CTFType(eTypedef, uid, name), type(type) {}
 
+  static bool classof(const CTFType *T) { return T->kind == eTypedef; }
+
   uint32_t type;
 };
 
@@ -91,6 +107,8 @@
            uint32_t index, uint32_t nelems)
       : CTFType(eArray, uid, name), type(type), index(index), nelems(nelems) {}
 
+  static bool classof(const CTFType *T) { return T->kind == eArray; }
+
   uint32_t type;
   uint32_t index;
   uint32_t nelems;
@@ -110,6 +128,8 @@
     assert(this->values.size() == nelems);
   }
 
+  static bool classof(const CTFType *T) { return T->kind == eEnum; }
+
   uint32_t nelems;
   uint32_t size;
   std::vector<Value> values;
@@ -121,6 +141,8 @@
       : CTFType(eFunction, uid, name), nargs(nargs), return_type(return_type),
         args(std::move(args)), variadic(variadic) {}
 
+  static bool classof(const CTFType *T) { return T->kind == eFunction; }
+
   uint32_t nargs;
   uint32_t return_type;
 
@@ -144,6 +166,10 @@
       : CTFType(kind, uid, name), nfields(nfields), size(size),
         fields(std::move(fields)) {}
 
+  static bool classof(const CTFType *T) {
+    return T->kind == eStruct || T->kind == eUnion;
+  }
+
   uint32_t nfields;
   uint32_t size;
   std::vector<Field> fields;
@@ -153,17 +179,23 @@
   CTFStruct(lldb::user_id_t uid, llvm::StringRef name, uint32_t nfields,
             uint32_t size, std::vector<Field> fields)
       : CTFRecord(eStruct, uid, name, nfields, size, std::move(fields)){};
+
+  static bool classof(const CTFType *T) { return T->kind == eStruct; }
 };
 
 struct CTFUnion : public CTFRecord {
   CTFUnion(lldb::user_id_t uid, llvm::StringRef name, uint32_t nfields,
            uint32_t size, std::vector<Field> fields)
       : CTFRecord(eUnion, uid, name, nfields, size, std::move(fields)){};
+
+  static bool classof(const CTFType *T) { return T->kind == eUnion; }
 };
 
 struct CTFForward : public CTFType {
   CTFForward(lldb::user_id_t uid, llvm::StringRef name)
       : CTFType(eForward, uid, name) {}
+
+  static bool classof(const CTFType *T) { return T->kind == eForward; }
 };
 
 } // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Jonas Devlieghere via Phabricator via lldb-commits

Reply via email to