Shared pointers are not good for hierarchies
Several(!) unrelated people in their talks mentioned that using shared pointers to represent hierarchies is considered a bad practice now. We do it for KisNode, and we have exactly the same problems they mentioned: memory leaks, cyclic dependencies and construction problems. We had two patches that were targeted to refactor our raw-pointer problems, but both failed because of the construction problems here and there: [1], [2].
[1] - https://phabricator.kde.org/D3279
[2] - https://phabricator.kde.org/D3307
Instead of storing a hierarchy in shared pointers like that:
struct KisNode {
KisNodeWSP m_parent;
QVector<KisNodeSP> m_children;
KisNodeSP parent() const;
KisNodeSP firstChild() const;
KisNodeSP lastChild() const;
KisNodeSP nextSibling() const;
KisNodeSP prevSibling() const;
};It is recommended to store hierarchy in a separate class, named "forest":
struct KisNode {
// only business logic inside, no hierarchy
};
// preferredly, no pointers should be used, only values and references
using KisNodeSmth = KisNode;
// as a fallback option the forest may also operate with shared pointer
// using KisNodeSmth = KisNodeSP;
struct KisNodeForest {
void addNode(...);
void removeNode(...);
void moveNode(...);
// these methods should not be used, unless it is really needed
KisNodeSmth parent() const;
KisNodeSmth firstChild() const;
KisNodeSmth lastChild() const;
KisNodeSmth nextSibling() const;
KisNodeSmth prevSibling() const;
// instead, more high-level itegarion algorithms should be used
// we already have functions like that in KisLayerUtils
template<class Func>
void iterateSubtree(KisNodeSmth root, Func func);
template<class Func>
void iterateChildren(KisNodeSmth parent, Func func);
template<class Func>
void iterateSubtreeReverse(KisNodeSmth root, Func func);
template<class Func>
void iterateChildrenReverse(KisNodeSmth parent, Func func);
template<class Func>
KisNodeSmth findNode(KisNodeSmth parent, Func func);
};The benefits of this approach:
- The logic of parent-child relations is split into a separate class (we tried to do that with splitting KisNode and KisBaseNode, but failed)
- The problem of node construction/cloning is solved (at least in case when KisNodeSmth is not a shared pointer).
- Different tree/iteration strategies can be implemented easily. Right now, we have separate KisProjectionLeaf and KisDummiesFacade classes to implement diffferent iteration/decoupling. If we have a "forest" with lazy-copying capabilities, tese problems would be solved easier.
Conclusion
The fact that the forest if a better approach doesn't mean that we should drop everithing now and start refactoring Krita to a "modern c++ practices". Though, I think, we should take these considerations into account when designing new code and refactoring old one. E.g. our resources branch fully relies on shared pointers now. Perhaps we could benefit from using the shared-pointer-free registry approach?
GUI widgets updates/signals hell (e.g. in brush editor)
The problem of our brush editor is that update/change signals don't have any granularity. If one changes, say, brush opacity, GUI issues "preset changed" signals and the entire Krita GUI should be updated. This atom-bomb-update can easily cause cycling updates, partial updates and UI freezes. That is bad, and that is why we havev the brush editor broken at least twice per year...
I tried to address this problem several times, see e.g. KisDerivedResourceConverter. It kind of works, but is not very general and causes some boilerplate code. It looks like there is a general solution to this:
Library Lager (MIT license): https://github.com/arximboldi/lager
Video 1: https://www.youtube.com/watch?v=_oBx_NbLghY
Video 2: https://www.youtube.com/watch?v=y_m0ce1rzRI
This library's purpose is to do proper UI updates with decent granularity and to avoid signals hell like we have in the brush editor. The library is compatible with Qt(!). It also has some capabilities for doing iterative refactoring. That it, you don't have to refactor everything at once, you can do it incrementally in a bottom-up approach.
Conclusion
I think we should try the library in some new code. Or just next time, when the brush editor breaks :)
KisUpdateScheduler multithreading problem
To resolve the problem we should google for: "work stealing scheduler". Work-stealing approach supports lock-free implementation. It can resolve our 6+ cores problems.
Known implementations (none of them can be used directly, though):
- Staccato work-stealing scheduler (https://github.com/rkuchumov/staccato)
- Boost.Fiber (https://www.boost.org/doc/libs/1_67_0/libs/fiber/doc/html/index.html)
Both libraries cannot be used directly. Staccato seem to not support thread sleeping during inactivity. And Boost.Fiber as a whole is just too weird for us. It demands code to use it's own mutexes, which is not acceptable. Though I have a feeling that at least a part of work-stealing code from Boost.Fiber can be reused.
AVX512 and std-simd library
Just some notes:
- AVX512 is only available in std-simd library, not in Vc
- Current implementation in std-simd doesn't support gather/scatter functions
- The paper proposed to C++23 doesn't include them either
- The C++ committee wants std-simd to become a standard in C++23
- The C++ committee also wants to get gather/scatter into the standard in time for C++23
- Mathias seem to be a bit busy atm and has no time to implement gather/scatter :)
GPU memory management concerns
We had an old question about "how to store image in GPU memory if it is too big?". It looks like CUDA now has internal swapping system, it is called "cuda unified memory". I guess openCL should also have something like that.
Link: https://devblogs.nvidia.com/unified-memory-cuda-beginners/