pahole changes in various

I'm not seeing as much as I would expect here, mostly because pahole
seems to be having trouble parsing quite a few of our structures, and
consequently producing useless data than I then ignore.

XDash 24bytes -> 20bytes
vcl::font::FeatureDefinition 64bytes -> 56bytes
SvXMLTokenMapEntry 16bytes -> 12bytes
SvXMLItemMapEntry 16bytes -> 12bytes
SwContentAtPos 40bytes -> 32bytes

Change-Id: I74c8b93f74b8352f48ef552d7d4239aa7f4237d4
Reviewed-on: https://gerrit.libreoffice.org/69304
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 38 insertions, 94 deletions

diff --git a/compilerplugins/clang/pahole-all-classes.py b/compilerplugins/clang/pahole-all-classes.py
index 9fda73c8789f..088e9cad2949 100755
--- a/compilerplugins/clang/pahole-all-classes.py
+++ b/compilerplugins/clang/pahole-all-classes.py
@@ -3,11 +3,15 @@
 # Find holes in structures, so that we can pack them and improve our memory density.
 #
 # In order to make this work, you need to
-# (1) be operating in a workspace where you have a debug build of LibreOffice
-# (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) ./compilerplugins/clang/pahole-all-classes.py > ./compilerplugins/clang/pahole.results
-#     Warning: running this script will make GDB soak up about 8G of RAM
+# (1) Be operating in a workspace where you have a __NON-DEBUG__ build of LibreOffice, but __WITH SYMBOLS__.
+#     (A debug build has different sizes for some things in the standard library.)
+# (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
 #
 
 import _thread
@@ -19,7 +23,7 @@ import re
 
 # search for all the class names in the file produced by the unusedfields loplugin
 #a = subprocess.Popen("grep 'definition:' workdir/loplugin.unusedfields.log | sort -u", stdout=subprocess.PIPE, shell=True)
-a = subprocess.Popen("cat n1", stdout=subprocess.PIPE, shell=True)
+a = subprocess.Popen("cat ../libo/n1", stdout=subprocess.PIPE, shell=True)
 
 classSourceLocDict = dict()
 classSet = set()
@@ -28,13 +32,14 @@ with a.stdout as txt:
         tokens = line.decode('utf8').strip().split("\t")
         className = tokens[2].strip()
         srcLoc = tokens[5].strip()
+        # ignore things like unions
         if "anonymous" in className: continue
-        # for now, just check the stuff in /sc/inc
-        if not srcLoc.startswith("sc/inc/"):
-                continue
+        # ignore duplicates
         if className in classSet: continue
-        classSourceLocDict[srcLoc] = className
-        classSet.add(className)
+        # for now, just check the stuff in /sc/inc
+        if srcLoc.startswith("a"):
+            classSourceLocDict[srcLoc] = className
+            classSet.add(className)
 a.terminate()
 
 gdbProc = subprocess.Popen("gdb", stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True)
@@ -65,8 +70,6 @@ def write_pahole_commands():
 
 _thread.start_new_thread( write_pahole_commands, () )
 
-time.sleep(2)
-
 # Use generator because lines often end up merged together in gdb's output, and we need
 # to split them up, and that creates a mess in the parsing logic.
 def read_generator():
@@ -79,13 +82,17 @@ def read_generator():
             yield split
 
 firstLineRegex = re.compile("/\*\s+(\d+)\s+\*/ struct")
-fieldLineRegex = re.compile("/\*\s+\d+\s+(\d+)\s+\*/ ")
+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
-found8ByteField = False
+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():
     structLines.append(line)
     firstLineMatch = firstLineRegex.match(line)
@@ -97,27 +104,30 @@ for line in read_generator():
         cumulativeHoleBits += int(holeLineMatch.group(1))
     fieldLineMatch = fieldLineRegex.match(line)
     if fieldLineMatch:
-        fieldSize = int(fieldLineMatch.group(1))
-        if fieldSize == 8:
-            found8ByteField = True
+        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:
-            # If we have an 8-byte field, then the whole structure must be 8-byte aligned, otherwise
-            # it must be 4-byte aligned. (that's my approximation of the rules, the real ones are probably
-            # more complicated). So check if removing the holes will remove enough space to actually shrink
-            # this structure.
-            alignBytes = 4
-            if found8ByteField: alignBytes = 8
-            if (cumulativeHoleBits / 8) >= alignBytes:
-                # print("Found one " + str(structSize) + " " + str(cumulativeHoleBits/8) + " " + str(newStructSize%4))
+        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 satifies 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
-        found8ByteField = False
+        foundBogusLine = False
+        actualStructSize = 0
 
 gdbProc.terminate()
\ No newline at end of file
diff --git a/compilerplugins/clang/pahole.results b/compilerplugins/clang/pahole.results
deleted file mode 100644
index 5d7129eb31a5..000000000000
--- a/compilerplugins/clang/pahole.results
+++ /dev/null
@@ -1,66 +0,0 @@
-ScColorScaleEntry sc/inc/colorscale.hxx:46
-/*   48     */ struct ScColorScaleEntry {
-/*   0    8 */    double mnVal
-/*   8    4 */    class Color maColor
-/* XXX 32 bit hole, try to pack */
-/*  16    8 */    class std::unique_ptr<ScFormulaCell, std::default_delete<ScFormulaCell> > mpCell
-/*  24    8 */    class std::unique_ptr<ScFormulaListener, std::default_delete<ScFormulaListener> > mpListener
-/*  32    4 */    enum ScColorScaleEntryType meType
-/* XXX 32 bit hole, try to pack */
-/*  40    8 */    class ScConditionalFormat * mpFormat
-}
-ScDetOpList sc/inc/detdata.hxx:66
-/*   64     */ struct ScDetOpList {
-/*   0    1 */    bool bHasAddError
-/* XXX 56 bit hole, try to pack */
-/*   8   56 */    class std::__debug::vector<std::unique_ptr<ScDetOpData, std::default_delete<ScDetOpData> >, std::allocator<std::unique_ptr<ScDetOpData, std::default_delete<ScDetOpData> > > > aDetOpDataVector
-}
-ScDocRowHeightUpdater::TabRanges sc/inc/dociter.hxx:569
-/*   24     */ struct ScDocRowHeightUpdater::TabRanges {
-/*   0    2 */    short mnTab
-/* XXX 48 bit hole, try to pack */
-/*   8   16 */    class std::shared_ptr<ScFlatBoolRowSegments> mpRanges
-}
-ScPivotField sc/inc/pivot.hxx:122
-/*   56     */ struct ScPivotField {
-/*   0    2 */    short nCol
-/* XXX 48 bit hole, try to pack */
-/*   8    8 */    long mnOriginalDim
-/*  16    4 */    enum PivotFunc nFuncMask
-/*  20    1 */    unsigned char mnDupCount
-/* XXX 24 bit hole, try to pack */
-/*  24   32 */    struct com::sun::star::sheet::DataPilotFieldReference maFieldRef
-}
-ScPivotFuncData sc/inc/pivot.hxx:164
-/*   56     */ struct ScPivotFuncData {
-/*   0    2 */    short mnCol
-/* XXX 48 bit hole, try to pack */
-/*   8    8 */    long mnOriginalDim
-/*  16    4 */    enum PivotFunc mnFuncMask
-/*  20    1 */    unsigned char mnDupCount
-/* XXX 24 bit hole, try to pack */
-/*  24   32 */    struct com::sun::star::sheet::DataPilotFieldReference maFieldRef
-}
-ScExternalSingleRefToken sc/inc/token.hxx:131
-/*   56     */ struct ScExternalSingleRefToken {
-/*   0   16 */    class formula::FormulaToken formula::FormulaToken
-/*  16    2 */    const unsigned short mnFileId
-/* XXX 48 bit hole, try to pack */
-/*  24   16 */    const class svl::SharedString maTabName
-/*  40   12 */    struct ScSingleRefData maSingleRef
-}
-ScExternalDoubleRefToken sc/inc/token.hxx:155
-/*   64     */ struct ScExternalDoubleRefToken {
-/*   0   16 */    class formula::FormulaToken formula::FormulaToken
-/*  16    2 */    const unsigned short mnFileId
-/* XXX 48 bit hole, try to pack */
-/*  24   16 */    const class svl::SharedString maTabName
-/*  40   24 */    struct ScComplexRefData maDoubleRef
-}
-ScExternalNameToken sc/inc/token.hxx:182
-/*   40     */ struct ScExternalNameToken {
-/*   0   16 */    class formula::FormulaToken formula::FormulaToken
-/*  16    2 */    const unsigned short mnFileId
-/* XXX 48 bit hole, try to pack */
-/*  24   16 */    const class svl::SharedString maName
-}