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