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

[jan iversen]

On 4 November 2015 at 11:21, Norbert Thiebaud <nthiebaud at gmail.com> wrote:

> 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.
>
Correct, things like that make many compilers drop optimization for the
whole class.

Always foo:1 first and then others.

As a side note, also remember that when using initializers in the
constructor, while digging into the code I have seen a couple of places
where the
sequence of initializers do not match the declaration sequence.

rgds
jan i

>
> Norbert
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20151104/4bcf2a9e/attachment.html>