D14353: Improve alignment of types

2018-07-31 Thread Frederik Gladhorn
gladhorn abandoned this revision. gladhorn added a comment. seems like this is not wanted REPOSITORY R110 KScreen Library REVISION DETAIL https://phabricator.kde.org/D14353 To: gladhorn, #plasma, romangg Cc: zzag, romangg, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,

D14353: Improve alignment of types

2018-07-30 Thread Roman Gilg
romangg added a comment. As already said on IRC and also how Vlad sees it: this optimization like for most of our classes is not worth it. Since we are an open source project, which strives to motivate people to contribute, good readability is much more important. So when ordering the cl

D14353: Improve alignment of types

2018-07-27 Thread Vlad Zagorodniy
zzag added a comment. In D14353#299286 , @gladhorn wrote: > OK, this should be the default for everyone. To have structures packed tight. Then the compiler can decide on alignment/padding (potentially depending on optimization space vs speed)...

D14353: Improve alignment of types

2018-07-27 Thread Frederik Gladhorn
gladhorn added a comment. In D14353#299282 , @zzag wrote: > I suggest to describe in the summary why data members are ordered from largest to smallest. OK, this should be the default for everyone. To have structures packed tight. Then th

D14353: Improve alignment of types

2018-07-27 Thread Vlad Zagorodniy
zzag added a comment. I suggest to describe in the summary why data members are ordered from largest to smallest. REPOSITORY R110 KScreen Library REVISION DETAIL https://phabricator.kde.org/D14353 To: gladhorn, #plasma, romangg Cc: zzag, romangg, plasma-devel, ragreen, Pitel, ZrenBot, l

D14353: Improve alignment of types

2018-07-27 Thread Frederik Gladhorn
gladhorn updated this revision to Diff 38596. gladhorn added a comment. initializer order fixed REPOSITORY R110 KScreen Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14353?vs=38595&id=38596 BRANCH arcpatch-D14353 REVISION DETAIL https://phabricator.kde.org/D14353 A

D14353: Improve alignment of types

2018-07-27 Thread Frederik Gladhorn
gladhorn updated this revision to Diff 38595. gladhorn added a comment. Move two more members to the right spot REPOSITORY R110 KScreen Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14353?vs=38379&id=38595 BRANCH arcpatch-D14353 REVISION DETAIL https://phabricator.k

D14353: Improve alignment of types

2018-07-26 Thread Frederik Gladhorn
gladhorn added a comment. Of course it's questionable if this is worth it, it's unlikely that we'll have so many instances of it that it'll ever make a difference. INLINE COMMENTS > output.cpp:89 > +int id; > qreal scale; > +Type type; Actually scale should move up maybe, since

D14353: Improve alignment of types

2018-07-26 Thread Frederik Gladhorn
gladhorn added a comment. In D14353#298417 , @romangg wrote: > Why push id down? It should stay at the top imo (or alphabetically). Rest is fine. Because of padding. On 64 bit when int is usually 4 bit, there will be a hole of 4 bits in

D14353: Improve alignment of types

2018-07-26 Thread Roman Gilg
romangg requested changes to this revision. romangg added a comment. This revision now requires changes to proceed. Why push id down? It should stay at the top imo (or alphabetically). Rest is fine. REPOSITORY R110 KScreen Library REVISION DETAIL https://phabricator.kde.org/D14353 To: g

D14353: Improve alignment of types

2018-07-24 Thread Frederik Gladhorn
gladhorn created this revision. gladhorn added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. gladhorn requested review of this revision. REPOSITORY R110 KScreen Library REVISION DETAIL https://phabricator.kde.org/D