Too bad. Thx for explaining... On Mon, Jun 15, 2015 at 1:30 PM Benjamin Kramer <[email protected]> wrote:
> On Mon, Jun 15, 2015 at 1:10 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. > > > > > > Curious that peak memory doesn't change. Probably because we always need > to > > push_back and can't know how many are in there when we add them in > > LineState. Would be interesting whether we can make this change have > impact > > on peak memory by changing the LineState.Stack to some different data > > structure (perhaps SmallVector might already be enough, as depths more > than > > 5 seem unlikely) > > The problem with SmallVector is that it bloats LineState enough to > kill any gain from saving ParenState allocations. Also in this file > the peak memory is dominated by FormatTokens (~45%), > WhiteSpaceManger::Change (~30%) and UnwrappedLineNodes (~15%) so > changes here won't have a large effect on peak anyways. > > - 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
