On Mon, Jun 15, 2015 at 1:01 PM, Manuel Klimek <[email protected]> wrote: > On Mon, Jun 15, 2015 at 12:56 PM Benjamin Kramer <[email protected]> > wrote: >> >> On Mon, Jun 15, 2015 at 6:30 AM, Manuel Klimek <[email protected]> wrote: >> > Would have been interesting in the description to mention why it >> > matters. 10 >> > bytes don't sound like much :) >> >> We copy millions of ParenState objects around. On formatting >> X86ISelLowering.cpp this change saves 25 MB of vector allocations, >> which is about 7-8% of the total memory allocations performed by >> clang-format on that file. Peak memory doesn't change significantly >> though. > > > Does it affect runtime?
Runtime difference was in the noise in the things I tested. - Ben >> >> > On Fri, Jun 12, 2015, 4:47 PM Benjamin Kramer <[email protected]> >> > wrote: >> >> >> >> On Fri, Jun 12, 2015 at 4:28 PM, Daniel Jasper <[email protected]> >> >> wrote: >> >> > Thanks.. I also think we should simply use in-class initializers here >> >> > instead of the constructor initializers where we constantly have to >> >> > fix >> >> > the >> >> > order. >> >> >> >> Did so in r239606 for the integer members. Bitfields don't support >> >> in-class initializers in C++11 :( >> >> >> >> - Ben >> >> >> >> > On Fri, Jun 12, 2015 at 3:07 PM, Benjamin Kramer >> >> > <[email protected]> >> >> > wrote: >> >> >> >> >> >> Author: d0k >> >> >> Date: Fri Jun 12 08:07:03 2015 >> >> >> New Revision: 239605 >> >> >> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=239605&view=rev >> >> >> Log: >> >> >> [clang-format] Reorder and pack ParenState members to minimize >> >> >> padding >> >> >> >> >> >> sizeof(ParenState) goes from 64 bytes to 52 bytes. NFC. >> >> >> >> >> >> Modified: >> >> >> cfe/trunk/lib/Format/ContinuationIndenter.h >> >> >> >> >> >> Modified: cfe/trunk/lib/Format/ContinuationIndenter.h >> >> >> URL: >> >> >> >> >> >> >> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.h?rev=239605&r1=239604&r2=239605&view=diff >> >> >> >> >> >> >> >> >> >> >> >> ============================================================================== >> >> >> --- cfe/trunk/lib/Format/ContinuationIndenter.h (original) >> >> >> +++ cfe/trunk/lib/Format/ContinuationIndenter.h Fri Jun 12 08:07:03 >> >> >> 2015 >> >> >> @@ -148,15 +148,15 @@ struct ParenState { >> >> >> ParenState(unsigned Indent, unsigned IndentLevel, unsigned >> >> >> LastSpace, >> >> >> bool AvoidBinPacking, bool NoLineBreak) >> >> >> : Indent(Indent), IndentLevel(IndentLevel), >> >> >> LastSpace(LastSpace), >> >> >> - NestedBlockIndent(Indent), FirstLessLess(0), >> >> >> - BreakBeforeClosingBrace(false), QuestionColumn(0), >> >> >> - AvoidBinPacking(AvoidBinPacking), >> >> >> BreakBeforeParameter(false), >> >> >> - NoLineBreak(NoLineBreak), LastOperatorWrapped(true), >> >> >> ColonPos(0), >> >> >> - StartOfFunctionCall(0), StartOfArraySubscripts(0), >> >> >> + NestedBlockIndent(Indent), FirstLessLess(0), >> >> >> QuestionColumn(0), >> >> >> + ColonPos(0), StartOfFunctionCall(0), >> >> >> StartOfArraySubscripts(0), >> >> >> NestedNameSpecifierContinuation(0), CallContinuation(0), >> >> >> VariablePos(0), >> >> >> - ContainsLineBreak(false), ContainsUnwrappedBuilder(0), >> >> >> - AlignColons(true), ObjCSelectorNameFound(false), >> >> >> - HasMultipleNestedBlocks(false), NestedBlockInlined(false) >> >> >> {} >> >> >> + BreakBeforeClosingBrace(false), >> >> >> AvoidBinPacking(AvoidBinPacking), >> >> >> + BreakBeforeParameter(false), NoLineBreak(NoLineBreak), >> >> >> + LastOperatorWrapped(true), ContainsLineBreak(false), >> >> >> + ContainsUnwrappedBuilder(false), AlignColons(true), >> >> >> + ObjCSelectorNameFound(false), >> >> >> HasMultipleNestedBlocks(false), >> >> >> + NestedBlockInlined(false) {} >> >> >> >> >> >> /// \brief The position to which a specific parenthesis level >> >> >> needs >> >> >> to >> >> >> be >> >> >> /// indented. >> >> >> @@ -182,31 +182,9 @@ struct ParenState { >> >> >> /// on a level. >> >> >> unsigned FirstLessLess; >> >> >> >> >> >> - /// \brief Whether a newline needs to be inserted before the >> >> >> block's >> >> >> closing >> >> >> - /// brace. >> >> >> - /// >> >> >> - /// We only want to insert a newline before the closing brace if >> >> >> there >> >> >> also >> >> >> - /// was a newline after the beginning left brace. >> >> >> - bool BreakBeforeClosingBrace; >> >> >> - >> >> >> /// \brief The column of a \c ? in a conditional expression; >> >> >> unsigned QuestionColumn; >> >> >> >> >> >> - /// \brief Avoid bin packing, i.e. multiple parameters/elements >> >> >> on >> >> >> multiple >> >> >> - /// lines, in this context. >> >> >> - bool AvoidBinPacking; >> >> >> - >> >> >> - /// \brief Break after the next comma (or all the commas in this >> >> >> context if >> >> >> - /// \c AvoidBinPacking is \c true). >> >> >> - bool BreakBeforeParameter; >> >> >> - >> >> >> - /// \brief Line breaking in this context would break a formatting >> >> >> rule. >> >> >> - bool NoLineBreak; >> >> >> - >> >> >> - /// \brief True if the last binary operator on this level was >> >> >> wrapped >> >> >> to the >> >> >> - /// next line. >> >> >> - bool LastOperatorWrapped; >> >> >> - >> >> >> /// \brief The position of the colon in an ObjC method >> >> >> declaration/call. >> >> >> unsigned ColonPos; >> >> >> >> >> >> @@ -230,40 +208,62 @@ struct ParenState { >> >> >> /// Used to align further variables if necessary. >> >> >> unsigned VariablePos; >> >> >> >> >> >> + /// \brief Whether a newline needs to be inserted before the >> >> >> block's >> >> >> closing >> >> >> + /// brace. >> >> >> + /// >> >> >> + /// We only want to insert a newline before the closing brace if >> >> >> there >> >> >> also >> >> >> + /// was a newline after the beginning left brace. >> >> >> + bool BreakBeforeClosingBrace : 1; >> >> >> + >> >> >> + /// \brief Avoid bin packing, i.e. multiple parameters/elements >> >> >> on >> >> >> multiple >> >> >> + /// lines, in this context. >> >> >> + bool AvoidBinPacking : 1; >> >> >> + >> >> >> + /// \brief Break after the next comma (or all the commas in this >> >> >> context if >> >> >> + /// \c AvoidBinPacking is \c true). >> >> >> + bool BreakBeforeParameter : 1; >> >> >> + >> >> >> + /// \brief Line breaking in this context would break a formatting >> >> >> rule. >> >> >> + bool NoLineBreak : 1; >> >> >> + >> >> >> + /// \brief True if the last binary operator on this level was >> >> >> wrapped >> >> >> to the >> >> >> + /// next line. >> >> >> + bool LastOperatorWrapped : 1; >> >> >> + >> >> >> /// \brief \c true if this \c ParenState already contains a >> >> >> line-break. >> >> >> /// >> >> >> /// The first line break in a certain \c ParenState causes extra >> >> >> penalty so >> >> >> /// that clang-format prefers similar breaks, i.e. breaks in the >> >> >> same >> >> >> /// parenthesis. >> >> >> - bool ContainsLineBreak; >> >> >> + bool ContainsLineBreak : 1; >> >> >> >> >> >> /// \brief \c true if this \c ParenState contains multiple >> >> >> segments >> >> >> of >> >> >> a >> >> >> /// builder-type call on one line. >> >> >> - bool ContainsUnwrappedBuilder; >> >> >> + bool ContainsUnwrappedBuilder : 1; >> >> >> >> >> >> /// \brief \c true if the colons of the curren ObjC method >> >> >> expression >> >> >> should >> >> >> /// be aligned. >> >> >> /// >> >> >> /// Not considered for memoization as it will always have the >> >> >> same >> >> >> value at >> >> >> /// the same token. >> >> >> - bool AlignColons; >> >> >> + bool AlignColons : 1; >> >> >> >> >> >> /// \brief \c true if at least one selector name was found in the >> >> >> current >> >> >> /// ObjC method expression. >> >> >> /// >> >> >> /// Not considered for memoization as it will always have the >> >> >> same >> >> >> value at >> >> >> /// the same token. >> >> >> - bool ObjCSelectorNameFound; >> >> >> + bool ObjCSelectorNameFound : 1; >> >> >> >> >> >> /// \brief \c true if there are multiple nested blocks inside >> >> >> these >> >> >> parens. >> >> >> /// >> >> >> /// Not considered for memoization as it will always have the >> >> >> same >> >> >> value at >> >> >> /// the same token. >> >> >> - bool HasMultipleNestedBlocks; >> >> >> + bool HasMultipleNestedBlocks : 1; >> >> >> >> >> >> // \brief The start of a nested block (e.g. lambda introducer in >> >> >> C++ >> >> >> or >> >> >> // "function" in JavaScript) is not wrapped to a new line. >> >> >> - bool NestedBlockInlined; >> >> >> + bool NestedBlockInlined : 1; >> >> >> >> >> >> bool operator<(const ParenState &Other) const { >> >> >> if (Indent != Other.Indent) >> >> >> >> >> >> >> >> >> _______________________________________________ >> >> >> cfe-commits mailing list >> >> >> [email protected] >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> > >> >> > >> >> _______________________________________________ >> >> cfe-commits mailing list >> >> [email protected] >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
