Review Request 116886: Refactor private variables of KCompletion

Frank Reininghaus frank78ac at googlemail.com
Wed Mar 19 11:55:36 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116886/#review53410
-----------------------------------------------------------



src/kcompletion_p.h
<https://git.reviewboard.kde.org/r/116886/#comment37578>

    This is not strictly related to your changes, but it looks a bit unusual to have one plain bool and two bool bitfields next to each other. Making all bools a bitfield won't make much difference now though because the compiler will always use more memory for this class in order to preserve the 4-byte or 8-byte alignment.
    
    Another alignment-related issue is caused by your patch though: on a 64-bit system, moving the int member away from the bools will most likely increase sizeof(KCompletionPrivate) by 8 bytes because the compiler will add some padding to both in order to preserve the alignment of the neigbouring pointers.
    
    It might not make a big difference because it's probably unusual to create thousands of KCompletionPrivate instances, but still, it seems unnecessary.
    
    If one really wanted to make use of bitfields to save memory here, one could make 'order' a bitfield and move it next to the bools. 


- Frank Reininghaus


On March 18, 2014, 11:01 p.m., David Gil Oliva wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116886/
> -----------------------------------------------------------
> 
> (Updated March 18, 2014, 11:01 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcompletion
> 
> 
> Description
> -------
> 
> Refactor private variables of KCompletion
> 
> Also: reorder variables declaration to avoid padding
> 
> 
> Diffs
> -----
> 
>   src/kcompletion_p.h e3fad26 
>   src/kcompletion.cpp 7396029 
> 
> Diff: https://git.reviewboard.kde.org/r/116886/diff/
> 
> 
> Testing
> -------
> 
> It builds. Autotests pass.
> 
> 
> Thanks,
> 
> David Gil Oliva
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140319/28fc6081/attachment.html>


More information about the Kde-frameworks-devel mailing list