[Kde-pim] Review Request 119933: Optimize: Share set of \SEEN flag across items.

[Milian Wolff]

> On Aug. 26, 2014, 4:52 a.m., Kevin Krammer wrote:
> > akonadi/protocolhelper.cpp, line 546
> > <https://git.reviewboard.kde.org/r/119933/diff/1/?file=307604#file307604line546>
> >
> >     just checking: this is still fine if multiple threads execute it simultantiously?
> 
> Milian Wolff wrote:
>     According to the C++11 memory model, yes - this is threadsafe. Afaik it was threadsafe when using GCC for a long time already, not sure about MSVC though.
> 
> Dan Vrátil wrote:
>     Wouldn't declaring the variable in global scope be safe in all cases?

No, global statics are a bad idea and I've seen crashes due to them on shutdown - at least when using clang as the compiler. See e.g. https://projects.kde.org/projects/extragear/kdevelop/kdevplatform/repository/revisions/e059f07d79a58f4a755b338e89b52a34e6984472
or
https://projects.kde.org/projects/extragear/kdevelop/kdevelop/repository/revisions/03efc1faabd69368c7685ca3778bc8fd22528cb9

it probably depends on the type you create globally but generally I'd try to avoid global statics. If you guys think the current patch too unsafe for some reason, I'll provide a different patch which will then init the seen flag in a function-static. By calling this function then at a defined place before threads start, we can ensure that no issues arise, even without c++11.


- Milian


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


On Aug. 25, 2014, 6:29 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119933/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2014, 6:29 p.m.)
> 
> 
> Review request for KDEPIM-Libraries.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> Most items just have a single \SEEN flag in their list of flags.
> Currently, we share the \SEEN flag, but not the list itself. By
> special-casing this extremely often occurring case, we can decrease
> the memory consumption easily. In my case it's a difference of ca.
> 7MB (171MB before vs. 164MB after).
> 
> 
> Diffs
> -----
> 
>   akonadi/protocolhelper.cpp 3d496667fa622cac2dda7d5a0ca4c92b5a59d2fe 
> 
> Diff: https://git.reviewboard.kde.org/r/119933/diff/
> 
> 
> Testing
> -------
> 
> ran it before and after this patch and compared the memory consumption. That went down noticeably. Also added a static counter to compare cache hits/misses temporarily, which showed this cache is easily hit ca. 40k times and only missed 4k times (so a 10:1 ratio, not bad at all :))
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/