improve pahole script and pack a few classes

(*) fix: I was substracting the padding space instead of adding it
when calculating how much free space we had to improve.

(*) sort input data, so we process structs located in the same DSO
together, which reduces GDB's memory usage

(*) handle another error condition, where gdbs output is sufficiently
mixed up that we miss the end of commands terminator

Change-Id: Ic4bb92b736f38a2b3d90e4a14485152b7f869b43
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95041
Tested-by: Jenkins
Reviewed-by: Noel Grandin <[email protected]>

comphelper/source/container/enumhelper.cxx

@@ -27,8 +27,8 @@ namespace comphelper
 
 OEnumerationByName::OEnumerationByName(const css::uno::Reference<css::container::XNameAccess>& _rxAccess)
     :m_aNames(_rxAccess->getElementNames())
-    ,m_nPos(0)
     ,m_xAccess(_rxAccess)
+    ,m_nPos(0)
     ,m_bListening(false)
 {
     impl_startDisposeListening();
@@ -38,8 +38,8 @@ OEnumerationByName::OEnumerationByName(const css::uno::Reference<css::container:
 OEnumerationByName::OEnumerationByName(const css::uno::Reference<css::container::XNameAccess>& _rxAccess,
                                        const css::uno::Sequence< OUString >&           _aNames  )
     :m_aNames(_aNames)
-    ,m_nPos(0)
     ,m_xAccess(_rxAccess)
+    ,m_nPos(0)
     ,m_bListening(false)
 {
     impl_startDisposeListening();

compilerplugins/clang/pahole-all-classes.py

@@ -8,10 +8,8 @@
 # (2) First run the unusedfields loplugin to generate a log file
 # (3) Install the pahole stuff into your gdb, I used this one:
 #     https://github.com/PhilArmstrong/pahole-gdb
-# (4) Edit the loop near the top of the script to only produce results for one of our modules.
-#     Note that this will make GDB soak up about 8G of RAM, which is why I don't do more than one module at a time
-# (5) Run the script
-#     ./compilerplugins/clang/pahole-all-classes.py > ./compilerplugins/clang/pahole.results
+# (4) Run the script
+#     ./compilerplugins/clang/pahole-all-classes.py
 #
 
 import _thread
@@ -27,6 +25,7 @@
 
 classSet = set()
 classSourceLocDict = dict()
+locToClassDict = dict()
 with a.stdout as txt:
     for line in txt:
         tokens = line.decode('utf8').strip().split("\t")
@@ -38,6 +37,7 @@
         if className in classSet: continue
         classSet.add(className)
         classSourceLocDict[className] = srcLoc
+        locToClassDict[srcLoc] = className
 a.terminate()
 
 # Some of the pahole commands are going to fail, and I cannot read the error stream and the input stream
@@ -57,83 +57,100 @@ def write_pahole_commands(classes):
 # to split them up, and that creates a mess in the parsing logic.
 def read_generator(gdbOutput):
     while True:
-        line = gdbOutput.readline().decode('utf8').strip()
+        line = gdbOutput.readline();
+        if line == "": return # end of file
+        line = line.decode('utf8').strip()
+        print("gdb: " + line)
         for split in line.split("(gdb)"):
             split = split.strip()
             if len(split) == 0: continue
             if "all-done" in split: return
             yield split
 
-classList = sorted(classSet)
+# build list of classes sorted by source location to increase the chances of
+# processing stuff stored in the same DSO together
+sortedLocs = sorted(locToClassDict.keys())
+classList = list()
+for src in sortedLocs:
+    if "include/" in src:
+        classList.append(locToClassDict[src])
 
-# Process 200 classes at a time, otherwise gdb's memory usage blows up and kills the machine
-#
-while len(classList) > 0:
+with open("compilerplugins/clang/pahole.results", "wt") as f:
+    # Process 400 classes at a time, otherwise gdb's memory usage blows up and kills the machine
+    # This number is chosen to make gdb peak at around 8G.
+    while len(classList) > 0:
 
-    currClassList = classList[1:200];
-    classList = classList[200:]
+        currClassList = classList[0:500];
+        classList = classList[500:]
 
-    gdbProc = subprocess.Popen("gdb", stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True)
+        gdbProc = subprocess.Popen("gdb", stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True)
 
-    stdin = io.TextIOWrapper(gdbProc.stdin, 'utf-8')
+        stdin = io.TextIOWrapper(gdbProc.stdin, 'utf-8')
 
-    # make gdb load all the debugging info
-    stdin.write("set confirm off\n")
-    for filename in sorted(os.listdir('instdir/program')):
-        if filename.endswith(".so"):
-            stdin.write("add-symbol-file instdir/program/" + filename + "\n")
-    stdin.flush()
+        # make gdb load all the debugging info
+        stdin.write("set confirm off\n")
+        # make gdb not wrap output and mess up my parsing
+        stdin.write("set width unlimited\n")
+        for filename in sorted(os.listdir('instdir/program')):
+            if filename.endswith(".so"):
+                stdin.write("add-symbol-file instdir/program/" + filename + "\n")
+        stdin.flush()
 
 
-    _thread.start_new_thread( write_pahole_commands, (currClassList,) )
+        _thread.start_new_thread( write_pahole_commands, (currClassList,) )
 
-    firstLineRegex = re.compile("/\*\s+(\d+)\s+\*/ struct")
-    fieldLineRegex = re.compile("/\*\s+(\d+)\s+(\d+)\s+\*/ ")
-    holeLineRegex = re.compile("/\* XXX (\d+) bit hole, try to pack \*/")
-    # sometimes pahole can't determine the size of a sub-struct, and then it returns bad data
-    bogusLineRegex = re.compile("/\*\s+\d+\s+0\s+\*/")
-    structLines = list()
-    foundHole = False
-    cumulativeHoleBits = 0
-    structSize = 0
-    foundBogusLine = False
-    # pahole doesn't report space at the end of the structure, so work it out myself
-    sizeOfFields = 0
-    for line in read_generator(gdbProc.stdout):
-        structLines.append(line)
-        firstLineMatch = firstLineRegex.match(line)
-        if firstLineMatch:
-            structSize = int(firstLineMatch.group(1))
-        holeLineMatch = holeLineRegex.match(line)
-        if holeLineMatch:
-            foundHole = True
-            cumulativeHoleBits += int(holeLineMatch.group(1))
-        fieldLineMatch = fieldLineRegex.match(line)
-        if fieldLineMatch:
-            fieldSize = int(fieldLineMatch.group(2))
-            sizeOfFields = int(fieldLineMatch.group(1)) + fieldSize
-        if bogusLineRegex.match(line):
-            foundBogusLine = True
-        if line == "}":
-            # Ignore very large structs, packing those is not going to help much, and
-            # re-organising them can make them much less readable.
-            if foundHole and len(structLines) < 12 and structSize < 100 and not foundBogusLine:
-                # Verify that we have enough hole-space that removing it will result in a structure
-                # that still satisfies alignment requirements, otherwise the compiler will just put empty
-                # space at the end of the struct.
-                # TODO improve detection of the required alignment for a structure
-                potentialSpace = (cumulativeHoleBits / 8) + (sizeOfFields - structSize)
-                if potentialSpace >= 8:
-                    for line in structLines:
-                        print(line)
-                    if (sizeOfFields - structSize) > 0:
-                        print("hole at end of struct: " + str(sizeOfFields - structSize))
-            #  reset state
-            structLines.clear()
-            foundHole = False
-            cumulativeHoleBits = 0
-            structSize = 0
-            foundBogusLine = False
-            actualStructSize = 0
+        firstLineRegex = re.compile("/\*\s+(\d+)\s+\*/ struct") # /* 16 */ struct Foo
+        fieldLineRegex = re.compile("/\*\s+(\d+)\s+(\d+)\s+\*/ ") # /* 12 8 */ class rtl::OUString aName
+        holeLineRegex = re.compile("/\* XXX (\d+) bit hole, try to pack \*/")
+        # sometimes pahole can't determine the size of a sub-struct, and then it returns bad data
+        bogusLineRegex = re.compile("/\*\s+\d+\s+0\s+\*/")
+        structLines = list()
+        foundHole = False
+        cumulativeHoleBits = 0
+        alignedStructSize = 0
+        foundBogusLine = False
+        # pahole doesn't report space at the end of the structure, so work it out myself
+        sizeOfStructWithoutPadding = 0
+        for line in read_generator(gdbProc.stdout):
+            structLines.append(line)
+            firstLineMatch = firstLineRegex.match(line)
+            if firstLineMatch:
+                alignedStructSize = int(firstLineMatch.group(1))
+                structLines.clear()
+                structLines.append(line)
+            holeLineMatch = holeLineRegex.match(line)
+            if holeLineMatch:
+                foundHole = True
+                cumulativeHoleBits += int(holeLineMatch.group(1))
+            fieldLineMatch = fieldLineRegex.match(line)
+            if fieldLineMatch:
+                fieldPosInBytes = int(fieldLineMatch.group(1))
+                fieldSizeInBytes = int(fieldLineMatch.group(2))
+                sizeOfStructWithoutPadding = fieldPosInBytes + fieldSizeInBytes
+            if bogusLineRegex.match(line):
+                foundBogusLine = True
+            if line == "}":
+                # Ignore very large structs, packing those is not going to help much, and
+                # re-organising them can make them much less readable.
+                if foundHole and len(structLines) < 16 and alignedStructSize < 100 and not foundBogusLine:
+                    # Verify that, after packing, and compiler alignment, the new structure will be actually smaller.
+                    # Sometimes, we can save space, but the compiler will align the structure such that we don't
+                    # actually save any space.
+                    # TODO improve detection of the required alignment for a structure
+                    holeAtEnd = alignedStructSize - sizeOfStructWithoutPadding
+                    potentialSpace = (cumulativeHoleBits / 8) + holeAtEnd
+                    if potentialSpace >= 8:
+                        for line in structLines:
+                            f.write(line + "\n")
+                        if holeAtEnd > 0:
+                            f.write("hole at end of struct: " + str(holeAtEnd) + "\n")
+                        f.write("\n")
+                #  reset state
+                structLines.clear()
+                foundHole = False
+                cumulativeHoleBits = 0
+                structSize = 0
+                foundBogusLine = False
+                actualStructSize = 0
 
-    gdbProc.terminate()
+        gdbProc.terminate()

editeng/source/outliner/paralist.cxx

@@ -54,8 +54,8 @@ Paragraph::Paragraph( sal_Int16 nDDepth )
 }
 
 Paragraph::Paragraph( const ParagraphData& rData )
-: nFlags( ParaFlag::NONE )
-, aBulSize( -1, -1)
+: aBulSize( -1, -1)
+, nFlags( ParaFlag::NONE )
 , bVisible( true )
 {
     nDepth = rData.nDepth;

include/basegfx/DrawCommands.hxx

@@ -43,8 +43,8 @@ enum class GradientType
 class GradientStop
 {
 public:
-    float mfOffset;
     basegfx::BColor maColor;
+    float mfOffset;
     float mfOpacity;
 };
 

include/comphelper/PropertyInfoHash.hxx

@@ -28,15 +28,20 @@ namespace comphelper
 {
     struct PropertyInfo
     {
-        OUString const maName;
-        sal_Int32 const mnHandle;
-        css::uno::Type const maType;
-        sal_Int16 const mnAttributes;
+        OUString maName;
+        css::uno::Type maType;
+        sal_Int32 mnHandle;
+        sal_Int16 mnAttributes;
+
+        PropertyInfo(OUString const & aName, sal_Int32 nHandle, css::uno::Type const & aType, sal_Int16 nAttributes)
+            : maName(aName), maType(aType), mnHandle(nHandle), mnAttributes(nAttributes) {}
+        PropertyInfo(OUString && aName, sal_Int32 nHandle, css::uno::Type const & aType, sal_Int16 nAttributes)
+            : maName(std::move(aName)), maType(aType), mnHandle(nHandle), mnAttributes(nAttributes) {}
     };
     struct PropertyData
     {
-        sal_uInt8 const mnMapId;
-        PropertyInfo const *mpInfo;
+        sal_uInt8 mnMapId;
+        const PropertyInfo *mpInfo;
         PropertyData ( sal_uInt8 nMapId, PropertyInfo const *pInfo )
         : mnMapId ( nMapId )
         , mpInfo ( pInfo ) {}

include/comphelper/enumhelper.hxx

@@ -46,9 +46,9 @@ class COMPHELPER_DLLPUBLIC OEnumerationByName final : private OEnumerationLock
                                                           css::lang::XEventListener    >
 {
     css::uno::Sequence< OUString > const                m_aNames;
+    css::uno::Reference< css::container::XNameAccess >  m_xAccess;
     sal_Int32                                           m_nPos;
-    css::uno::Reference< css::container::XNameAccess >    m_xAccess;
-    bool                                            m_bListening;
+    bool                                                m_bListening;
 
 public:
     OEnumerationByName(const css::uno::Reference< css::container::XNameAccess >& _rxAccess);

include/comphelper/interfacecontainer2.hxx

@@ -100,9 +100,9 @@ public:
 
 private:
     OInterfaceContainerHelper2 & rCont;
-    bool const                   bIsList;
     detail::element_alias2       aData;
     sal_Int32                    nRemain;
+    bool                         bIsList;
 
     OInterfaceIteratorHelper2( const OInterfaceIteratorHelper2 & ) = delete;
     OInterfaceIteratorHelper2 &  operator = ( const OInterfaceIteratorHelper2 & ) = delete;

include/comphelper/propertysetinfo.hxx

@@ -44,8 +44,8 @@ namespace comphelper
 struct PropertyMapEntry
 {
     OUString       maName;
-    sal_Int32      mnHandle;
     css::uno::Type maType;
+    sal_Int32      mnHandle;
     /// flag bitmap, @see css::beans::PropertyAttribute
     sal_Int16      mnAttributes;
     sal_uInt8      mnMemberId;
@@ -54,8 +54,8 @@ struct PropertyMapEntry
     PropertyMapEntry(OUString _aName, sal_Int32 _nHandle, css::uno::Type const & _rType,
                      sal_Int16 _nAttributes, sal_uInt8 _nMemberId, PropertyMoreFlags _nMoreFlags = PropertyMoreFlags::NONE)
         : maName( _aName )
-        , mnHandle( _nHandle )
         , maType( _rType )
+        , mnHandle( _nHandle )
         , mnAttributes( _nAttributes )
         , mnMemberId( _nMemberId )
         , mnMoreFlags( _nMoreFlags )

include/editeng/outliner.hxx

@@ -126,9 +126,9 @@ private:
 
     Paragraph& operator=(const Paragraph& rPara ) = delete;
 
-    ParaFlag            nFlags;
     OUString            aBulText;
     Size                aBulSize;
+    ParaFlag            nFlags;
     bool                bVisible;
 
     bool                IsVisible() const { return bVisible; }
@@ -526,16 +526,16 @@ public:
     SdrPage*        GetSdrPage() const { return mpSdrPage; }
 };
 
-struct EBulletInfo
+ struct EBulletInfo
 {
-    bool        bVisible;
-    sal_uInt16  nType;          // see SvxNumberType
-    OUString    aText;
-    SvxFont     aFont;
-    sal_Int32   nParagraph;
-    tools::Rectangle   aBounds;
-
-    EBulletInfo() : bVisible( false ), nType( 0 ), nParagraph( EE_PARA_NOT_FOUND ) {}
+    SvxFont           aFont;
+    tools::Rectangle  aBounds;
+    OUString          aText;
+    sal_Int32         nParagraph;
+    sal_uInt16        nType;          // see SvxNumberType
+    bool              bVisible;
+
+    EBulletInfo() : nParagraph( EE_PARA_NOT_FOUND ), nType( 0 ), bVisible( false ) {}
 };
 
 enum class OutlinerMode {

include/formula/FormulaCompiler.hxx

@@ -60,8 +60,8 @@ struct FormulaArrayStack
 {
     FormulaArrayStack*  pNext;
     FormulaTokenArray*  pArr;
-    sal_uInt16          nIndex;
     FormulaTokenRef     mpLastToken;
+    sal_uInt16          nIndex;
     bool bTemp;
 };
 

include/xmloff/maptype.hxx

@@ -32,9 +32,9 @@ struct XMLPropertyMapEntry
 {
     const char*     msApiName;      /// Property-Name
     sal_Int32       nApiNameLength; /// length of property name
+    enum ::xmloff::token::XMLTokenEnum meXMLName;       /// XML-Name
     sal_uInt16      mnNameSpace;    /** declares the Namespace in which this
                                         property exists */
-    enum ::xmloff::token::XMLTokenEnum meXMLName;       /// XML-Name
 
     /**
      * The lowest 14 bits specify the basic XML type of the property value, of
@@ -98,6 +98,21 @@ struct XMLPropertyMapEntry
         Property-Name exist, all except one must have this flag set.
      */
     bool            mbImportOnly;
+
+    XMLPropertyMapEntry(
+            const char*     sApiName,
+            sal_Int32       nApiNameLength_,
+            sal_uInt16      nNameSpace,
+            enum ::xmloff::token::XMLTokenEnum eXMLName,
+            sal_uInt32 nType,
+            sal_Int16       nContextId,
+            SvtSaveOptions::ODFSaneDefaultVersion nEarliestODFVersionForExport,
+            bool            bImportOnly)
+        : msApiName(sApiName), nApiNameLength(nApiNameLength_),
+        meXMLName(eXMLName), mnNameSpace(nNameSpace), mnType(nType),
+        mnContextId(nContextId), mnEarliestODFVersionForExport(nEarliestODFVersionForExport),
+        mbImportOnly(bImportOnly)
+    {}
 };