Review Request 122861: Micro-optimize LayoutManager

[Sebastian Kügler]

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


Looks good to me. It would be good if Eike could also have a look since he's refactoring this code right now, and his refactoring might benefit from the same changes, if not avoid conflicts.

One stylistic remark (spread over three lines) inline.


containments/desktop/package/contents/code/LayoutManager.js
<https://git.reviewboard.kde.org/r/122861/#comment53030>

    I think it's more readable to init j before the loop.



containments/desktop/package/contents/code/LayoutManager.js
<https://git.reviewboard.kde.org/r/122861/#comment53031>

    same here



containments/desktop/package/contents/code/LayoutManager.js
<https://git.reviewboard.kde.org/r/122861/#comment53032>

    init j before loop for readability


- Sebastian Kügler


On March 8, 2015, 9:08 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122861/
> -----------------------------------------------------------
> 
> (Updated March 8, 2015, 9:08 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> This micro-optimizes the LayoutManager by:
> - using Array and Object literals rather than new Object/Array, and also creating the whole structure at once if applicable
> - Store end values in for loops rather than calculating them on each iteration
> - Adjust coding style here and there
> 
> 
> Diffs
> -----
> 
>   containments/desktop/package/contents/code/LayoutManager.js 14d0dfc 
> 
> Diff: https://git.reviewboard.kde.org/r/122861/diff/
> 
> 
> Testing
> -------
> 
> Moving applets (especially using Eike's press-and-hold when using it on a touchscreen :P) feels snappier, doesn't print any new warnings on console and seems to work as before. However, I can no longer cause plasmashell to go berserk when moving a small applet ontop of a huge one (eg. small sticky note on wide fuzzy clock) where it desperately tries to find a place and fails.
> 
> I think the grid size should be based on units somehow, having a 24x24 grid on a high dpi screen also benefits the aforementioned behavior.
> 
> One surprising discovery I made is that using Qt.point instead of a handcrafted JS Object is one order of magnitude(!) slower.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150309/b6e8d028/attachment.html>