[DebugInfo][NFC] Erase capacity in DWARFUnit::clearDIEs().

[avl]

Summary

DWARFUnit::clearDIEs() uses std::vector::shrink_to_fit() to make
capacity of DieArray matched with its size(). The shrink_to_fit()
is not binding request to make capacity match with size().
Thus the memory could still be reserved after DWARFUnit::clearDIEs()
is called. This patch erases capacity when DWARFUnit::clearDIEs() is requested.
So the memory occupied by dies would be freed.

[dblaikie]

Have you observed this to be a problem in practice? Might be worth getting that implementation fixed, rather than working around the low-quality implementation (if that's the issue - if the implementation has a good reason for not taking the shrink_to_fit hint, then maybe this patch is the way to go)?

[avl]

I used clearDIEs() in this patch https://reviews.llvm.org/D96035 and observed that memory is not freed. Looking at the reason I found that shrink_to_fit() does not change a capacity and as the result the memory stay used.

The use case in D96035 is: load dies of DWARFUnit, clone dies to output, clean loaded dies by calling DWARFUnit::clear().

The comment says that it is not guaranteed that capacity would be reduced:

/**  A non-binding request to reduce capacity() to size().  */
void
shrink_to_fit()

[dblaikie]

Which standard library implementation did you observe that behavior in?

[avl]

The used implementation come from /usr/include/c++/9/bits/allocator.h :

  template<typename _Tp>
    struct __shrink_to_fit_aux<_Tp, true>
    {
      static bool
      _S_do_it(_Tp& __c) noexcept
      {
#if __cpp_exceptions
	try
	  {
	    _Tp(__make_move_if_noexcept_iterator(__c.begin()),
		__make_move_if_noexcept_iterator(__c.end()),
		__c.get_allocator()).swap(__c);
	    return true;
	  }
	catch(...)
	  { return false; }
#else
	return false;   <<< this path is taken which do nothing.
#endif
      }
    };

which belongs to gcc 9.3.0 libstdc++ library.

[dblaikie]

eh, curious. Fair enough.

Might be a simpler/tidier way to write this, not sure...

DieArray = (KeepCUDie && !DieArray.empty()) ? std::vector<DWARFDebugInfoEntry>({DieArray[0]}) : std::vector<DWARFDebugInfoEntry>();

Maybe that'd work? Maybe include a comment about not using shrink_to_fit because some implementations don't respect it under certain conditions? (possibly naming the specific implementation/conditions to make it easier for someone to know whether the workaround is still useful if they're revisiting this code in the future)

[avl]

addressed comments.

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

if (getVersion() == 4) {
     auto DWORangesBase = UnitDie.getRangesBaseAttribute();
     DWO->setRangesSection(RangeSection, DWORangesBase ? *DWORangesBase : 0);
   }
 
   return true;
 }
 
 void DWARFUnit::clearDIEs(bool KeepCUDie) {
-  if (DieArray.size() > (unsigned)KeepCUDie) {
-    DieArray.resize((unsigned)KeepCUDie);
-    DieArray.shrink_to_fit();
-  }
+  // Do not use resize() + shrink_to_fit() to free memory occupied by dies.
+  // shrink_to_fit() is a *non-binding* request to reduce capacity() to size().
+  // It depends on the implementation whether the request is fulfilled.
+  // Create a new vector with a small capacity and assign it to the DieArray to
+  // have previous contents freed.
+  DieArray = (KeepCUDie && !DieArray.empty())
+                 ? std::vector<DWARFDebugInfoEntry>({DieArray[0]})
+                 : std::vector<DWARFDebugInfoEntry>();
 }
 
 Expected<DWARFAddressRangesVector>
 DWARFUnit::findRnglistFromOffset(uint64_t Offset) {
   if (getVersion() <= 4) {
     DWARFDebugRangeList RangeList;
     if (Error E = extractRangeList(Offset, RangeList))
       return std::move(E);