[Libreoffice-commits] core.git: editeng: Eliminate unecessary padding in classes

[Norbert Thiebaud]

On Wed, Nov 4, 2015 at 3:42 AM, Stephan Bergmann <sbergman at redhat.com> wrote:
> On 11/04/2015 09:16 AM, Daniel Robertson wrote:
>>
>> commit f57a6593e21d45008173352fc29ededdbc2c8878
>> Author: Daniel Robertson <danlrobertson89 at gmail.com>
>> Date:   Tue Nov 3 14:14:08 2015 -0500
>>
>>      editeng: Eliminate unecessary padding in classes
>>
>>      Edit the order of SvxLRSpaceItem and PaintFirstLineInfo members to
>>      remove unecessary padding due to data alignment.
>>
>>      Change-Id: Icf2c92ef86a32384e51d1cb6f1a079b10995dfd5
>>      Reviewed-on: https://gerrit.libreoffice.org/19763
>>      Tested-by: Jenkins <ci at libreoffice.org>
>>      Reviewed-by: Norbert Thiebaud <nthiebaud at gmail.com>
>>
> [...]
>>
>> diff --git a/include/editeng/lrspitem.hxx b/include/editeng/lrspitem.hxx
>> index 40da999..94daed8 100644
>> --- a/include/editeng/lrspitem.hxx
>> +++ b/include/editeng/lrspitem.hxx
>> @@ -50,15 +50,15 @@
>>
>>   class EDITENG_DLLPUBLIC SvxLRSpaceItem : public SfxPoolItem
>>   {
>> -    short   nFirstLineOfst;     // First-line indent _always_ relative to
>> nTxtLeft
>>       long    nTxtLeft;           // We spend a sal_uInt16
>>       long    nLeftMargin;        // nLeft or the negative first-line
>> indent
>>       long    nRightMargin;       // The unproblematic right edge
>>
>>       sal_uInt16  nPropFirstLineOfst, nPropLeftMargin, nPropRightMargin;
>> +    short   nFirstLineOfst;     // First-line indent _always_ relative to
>> nTxtLeft
>
>
> With micro-optimization changes like these, I would leave it to some domain
> expert to decide whether it is actually worth it (i.e., whether on the one
> hand the existing member order had been chosen deliberately, to aid human
> comprehension, and whether on the other hand enough instances of that class
> are created to warrant any such optimization).

Domain expert are the last one to reliably evaluate 'human
comprehension", since they _are_ domain expert
wrt to the number of instance... even if there is only one. is it a
reason to be sloppy ?

>
>>       bool        bAutoFirst  : 1;    // Automatic calculation of the
>> first line indent
>> -    bool        bExplicitZeroMarginValRight;
>> -    bool        bExplicitZeroMarginValLeft;
>> +    bool        bExplicitZeroMarginValRight : 1;
>> +    bool        bExplicitZeroMarginValLeft : 1;
>
>
> It is unclear whether that is an optimization or a pessimization.

you can choose size or perf.. but having a
bool foo:1
followed by 2 plain bool
is wrong either way.

Norbert