[Lldb-commits] [PATCH] D50271: [IRMemoryMap] Shrink Allocation by sizeof(addr_t) (NFC)

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: teemperor, lhames.

Profiling data show that Allocation::operator= is hot, see:

  https://teemperor.de/lldb-bench/data/arithmetic.svg

Reorder a few fields within Allocation to avoid implicit structure
padding and shrink the structure. This should make copies cheaper and
reduce lldb's RSS.

There should be more low-hanging fruit here for performance
optimization: we don't need std::map, we can make Allocation move-only,
etc.


https://reviews.llvm.org/D50271

Files:
  lldb/include/lldb/Expression/IRMemoryMap.h
  lldb/source/Expression/IRMemoryMap.cpp


Index: lldb/source/Expression/IRMemoryMap.cpp
===================================================================
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -272,8 +272,8 @@
                                     uint32_t permissions, uint8_t alignment,
                                     AllocationPolicy policy)
     : m_process_alloc(process_alloc), m_process_start(process_start),
-      m_size(size), m_permissions(permissions), m_alignment(alignment),
-      m_policy(policy), m_leak(false) {
+      m_size(size), m_policy(policy), m_leak(false), m_permissions(permissions),
+      m_alignment(alignment) {
   switch (policy) {
   default:
     assert(0 && "We cannot reach this!");
Index: lldb/include/lldb/Expression/IRMemoryMap.h
===================================================================
--- lldb/include/lldb/Expression/IRMemoryMap.h
+++ lldb/include/lldb/Expression/IRMemoryMap.h
@@ -39,7 +39,7 @@
   IRMemoryMap(lldb::TargetSP target_sp);
   ~IRMemoryMap();
 
-  enum AllocationPolicy {
+  enum AllocationPolicy : uint8_t {
     eAllocationPolicyInvalid =
         0, ///< It is an error for an allocation to have this policy.
     eAllocationPolicyHostOnly, ///< This allocation was created in the host and
@@ -91,32 +91,36 @@
 private:
   struct Allocation {
     lldb::addr_t
-        m_process_alloc; ///< The (unaligned) base for the remote allocation
+        m_process_alloc; ///< The (unaligned) base for the remote allocation.
     lldb::addr_t
-        m_process_start; ///< The base address of the allocation in the process
-    size_t m_size;       ///< The size of the requested allocation
-    uint32_t m_permissions; ///< The access permissions on the memory in the
-                            ///process.  In the host, the memory is always
-                            ///read/write.
-    uint8_t m_alignment;    ///< The alignment of the requested allocation
+        m_process_start; ///< The base address of the allocation in the process.
+    size_t m_size;       ///< The size of the requested allocation.
     DataBufferHeap m_data;
 
-    ///< Flags
+    /// Flags. Keep these grouped together to avoid structure padding.
     AllocationPolicy m_policy;
     bool m_leak;
+    uint8_t m_permissions; ///< The access permissions on the memory in the
+                           /// process. In the host, the memory is always
+                           /// read/write.
+    uint8_t m_alignment;   ///< The alignment of the requested allocation.
 
   public:
     Allocation(lldb::addr_t process_alloc, lldb::addr_t process_start,
                size_t size, uint32_t permissions, uint8_t alignment,
                AllocationPolicy m_policy);
 
     Allocation()
         : m_process_alloc(LLDB_INVALID_ADDRESS),
-          m_process_start(LLDB_INVALID_ADDRESS), m_size(0), m_permissions(0),
-          m_alignment(0), m_data(), m_policy(eAllocationPolicyInvalid),
-          m_leak(false) {}
+          m_process_start(LLDB_INVALID_ADDRESS), m_size(0), m_data(),
+          m_policy(eAllocationPolicyInvalid), m_leak(false), m_permissions(0),
+          m_alignment(0) {}
   };
 
+  static_assert(sizeof(Allocation) <=
+                    (4 * sizeof(lldb::addr_t)) + sizeof(DataBufferHeap),
+                "IRMemoryMap::Allocation is larger than expected");
+
   lldb::ProcessWP m_process_wp;
   lldb::TargetWP m_target_wp;
   typedef std::map<lldb::addr_t, Allocation> AllocationMap;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50271.159090.patch
Type: text/x-patch
Size: 3482 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180803/7c534c32/attachment.bin>