[Libreoffice-commits] core.git: compilerplugins/clang include/svx include/vcl include/xmloff sc/inc sc/source svx/source sw/inc sw/source vcl/source
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Sat Mar 16 06:17:35 UTC 2019
compilerplugins/clang/pahole-all-classes.py | 66 ++++++++++++++++------------
compilerplugins/clang/pahole.results | 66 ----------------------------
include/svx/xdash.hxx | 2
include/vcl/font/Feature.hxx | 2
include/xmloff/xmltkmap.hxx | 10 ++--
sc/inc/simplerangelist.hxx | 4 -
sc/source/core/tool/simplerangelist.cxx | 2
svx/source/xoutdev/xattr.cxx | 2
sw/inc/crsrsh.hxx | 5 --
sw/source/filter/xml/xmlitmap.hxx | 10 +++-
vcl/source/font/Feature.cxx | 16 +++---
11 files changed, 66 insertions(+), 119 deletions(-)
New commits:
commit 45968141934ac4ea10ad7fe4c2b074152aa2e635
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Fri Mar 15 14:56:08 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Sat Mar 16 07:17:15 2019 +0100
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 at collabora.co.uk>
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
-}
diff --git a/include/svx/xdash.hxx b/include/svx/xdash.hxx
index e415e8b4dc19..ef28add1debd 100644
--- a/include/svx/xdash.hxx
+++ b/include/svx/xdash.hxx
@@ -33,8 +33,8 @@
class SVX_DLLPUBLIC XDash final
{
css::drawing::DashStyle eDash;
- sal_uInt16 nDots;
sal_uInt32 nDotLen;
+ sal_uInt16 nDots;
sal_uInt16 nDashes;
sal_uInt32 nDashLen;
sal_uInt32 nDistance;
diff --git a/include/vcl/font/Feature.hxx b/include/vcl/font/Feature.hxx
index 390abe33f8b7..47092f9bb56e 100644
--- a/include/vcl/font/Feature.hxx
+++ b/include/vcl/font/Feature.hxx
@@ -56,10 +56,10 @@ public:
class VCL_DLLPUBLIC FeatureDefinition
{
private:
- uint32_t m_nCode;
OUString m_sDescription;
const char* m_pDescriptionID;
OUString m_sNumericPart;
+ uint32_t m_nCode;
FeatureParameterType m_eType;
// the index of the parameter defines the enum value, string is the description
std::vector<FeatureParameter> m_aEnumParameters;
diff --git a/include/xmloff/xmltkmap.hxx b/include/xmloff/xmltkmap.hxx
index 1bed470c2535..df4de8e4848f 100644
--- a/include/xmloff/xmltkmap.hxx
+++ b/include/xmloff/xmltkmap.hxx
@@ -34,17 +34,17 @@ class SvXMLTokenMap_Impl;
struct SvXMLTokenMapEntry
{
- sal_uInt16 const nPrefixKey;
enum xmloff::token::XMLTokenEnum const eLocalName;
- sal_uInt16 const nToken;
sal_Int32 nFastToken;
+ sal_uInt16 const nPrefixKey;
+ sal_uInt16 const nToken;
SvXMLTokenMapEntry( sal_uInt16 nPrefix, xmloff::token::XMLTokenEnum eName,
sal_uInt16 nTok, sal_Int32 nFastTok = 0 ) :
- nPrefixKey( nPrefix ),
eLocalName( eName ),
- nToken( nTok ),
- nFastToken( sal_uInt32( nPrefixKey + 1 ) << 16 | eLocalName )
+ nFastToken( sal_uInt32( nPrefix + 1 ) << 16 | eLocalName ),
+ nPrefixKey( nPrefix ),
+ nToken( nTok )
{
if ( nFastTok ) // alternative value for duplicate/dummy tokens
nFastToken = nFastTok;
diff --git a/sc/inc/simplerangelist.hxx b/sc/inc/simplerangelist.hxx
index a16cf0042325..442d5c4e7a68 100644
--- a/sc/inc/simplerangelist.hxx
+++ b/sc/inc/simplerangelist.hxx
@@ -37,10 +37,10 @@ class ScSimpleRangeList
public:
struct Range
{
- SCCOL mnCol1;
SCROW mnRow1;
- SCCOL mnCol2;
SCROW mnRow2;
+ SCCOL mnCol1;
+ SCCOL mnCol2;
explicit Range(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2);
};
typedef std::shared_ptr< ::std::list<Range> > RangeListRef;
diff --git a/sc/source/core/tool/simplerangelist.cxx b/sc/source/core/tool/simplerangelist.cxx
index 14f7cf94341a..0dacf14d0464 100644
--- a/sc/source/core/tool/simplerangelist.cxx
+++ b/sc/source/core/tool/simplerangelist.cxx
@@ -29,7 +29,7 @@ using ::std::pair;
using ::std::max;
ScSimpleRangeList::Range::Range(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2) :
- mnCol1(nCol1), mnRow1(nRow1), mnCol2(nCol2), mnRow2(nRow2) {}
+ mnRow1(nRow1), mnRow2(nRow2), mnCol1(nCol1), mnCol2(nCol2) {}
ScSimpleRangeList::ScSimpleRangeList()
{
diff --git a/svx/source/xoutdev/xattr.cxx b/svx/source/xoutdev/xattr.cxx
index 54e0a354b975..de1717d293f7 100644
--- a/svx/source/xoutdev/xattr.cxx
+++ b/svx/source/xoutdev/xattr.cxx
@@ -384,8 +384,8 @@ sal_uInt16 XLineStyleItem::GetValueCount() const
XDash::XDash(css::drawing::DashStyle eTheDash, sal_uInt16 nTheDots, sal_uInt32 nTheDotLen,
sal_uInt16 nTheDashes, sal_uInt32 nTheDashLen, sal_uInt32 nTheDistance) :
eDash(eTheDash),
- nDots(nTheDots),
nDotLen(nTheDotLen),
+ nDots(nTheDots),
nDashes(nTheDashes),
nDashLen(nTheDashLen),
nDistance(nTheDistance)
diff --git a/sw/inc/crsrsh.hxx b/sw/inc/crsrsh.hxx
index 5ba02d648620..cfe9ef023cd0 100644
--- a/sw/inc/crsrsh.hxx
+++ b/sw/inc/crsrsh.hxx
@@ -95,8 +95,6 @@ namespace o3tl {
struct SwContentAtPos
{
- IsAttrAtPos eContentAtPos;
-
union {
const SwField* pField;
const SfxPoolItem* pAttr;
@@ -104,9 +102,8 @@ struct SwContentAtPos
SwContentNode * pNode;
const sw::mark::IFieldmark* pFieldmark;
} aFnd;
-
+ IsAttrAtPos eContentAtPos;
int nDist;
-
OUString sStr;
const SwTextAttr* pFndTextAttr;
diff --git a/sw/source/filter/xml/xmlitmap.hxx b/sw/source/filter/xml/xmlitmap.hxx
index 3123dae2255e..bf54b476c814 100644
--- a/sw/source/filter/xml/xmlitmap.hxx
+++ b/sw/source/filter/xml/xmlitmap.hxx
@@ -40,15 +40,21 @@ struct SvXMLItemMapEntry
{
sal_uInt16 const nNameSpace; // declares the Namespace in which this item
// exists
+ sal_uInt16 nWhichId; // the WhichId to identify the item
+ // in the pool
enum ::xmloff::token::XMLTokenEnum const eLocalName;
// the local name for the item inside
// the Namespace (as an XMLTokenEnum)
- sal_uInt16 nWhichId; // the WhichId to identify the item
- // in the pool
sal_uInt32 const nMemberId; // the memberid specifies which part
// of the item should be imported or
// exported with this Namespace
// and localName
+ SvXMLItemMapEntry(
+ sal_uInt16 nameSpace,
+ enum ::xmloff::token::XMLTokenEnum localName,
+ sal_uInt16 whichId,
+ sal_uInt32 memberId)
+ : nNameSpace(nameSpace), nWhichId(whichId), eLocalName(localName), nMemberId(memberId) {}
};
class SvXMLItemMapEntries_impl;
diff --git a/vcl/source/font/Feature.cxx b/vcl/source/font/Feature.cxx
index 3a809cb11e8a..2516da2bc4c0 100644
--- a/vcl/source/font/Feature.cxx
+++ b/vcl/source/font/Feature.cxx
@@ -92,8 +92,8 @@ uint32_t FeatureParameter::getCode() const { return m_nCode; }
// FeatureDefinition
FeatureDefinition::FeatureDefinition()
- : m_nCode(0)
- , m_pDescriptionID(nullptr)
+ : m_pDescriptionID(nullptr)
+ , m_nCode(0)
, m_eType(FeatureParameterType::BOOL)
{
}
@@ -101,9 +101,9 @@ FeatureDefinition::FeatureDefinition()
FeatureDefinition::FeatureDefinition(uint32_t nCode, OUString const& rDescription,
FeatureParameterType eType,
std::vector<FeatureParameter> const& rEnumParameters)
- : m_nCode(nCode)
- , m_sDescription(rDescription)
+ : m_sDescription(rDescription)
, m_pDescriptionID(nullptr)
+ , m_nCode(nCode)
, m_eType(eType)
, m_aEnumParameters(rEnumParameters)
{
@@ -111,17 +111,17 @@ FeatureDefinition::FeatureDefinition(uint32_t nCode, OUString const& rDescriptio
FeatureDefinition::FeatureDefinition(uint32_t nCode, const char* pDescriptionID,
OUString const& rNumericPart)
- : m_nCode(nCode)
- , m_pDescriptionID(pDescriptionID)
+ : m_pDescriptionID(pDescriptionID)
, m_sNumericPart(rNumericPart)
+ , m_nCode(nCode)
, m_eType(FeatureParameterType::BOOL)
{
}
FeatureDefinition::FeatureDefinition(uint32_t nCode, const char* pDescriptionID,
std::vector<FeatureParameter> aEnumParameters)
- : m_nCode(nCode)
- , m_pDescriptionID(pDescriptionID)
+ : m_pDescriptionID(pDescriptionID)
+ , m_nCode(nCode)
, m_eType(FeatureParameterType::ENUM)
, m_aEnumParameters(std::move(aEnumParameters))
{
More information about the Libreoffice-commits
mailing list