Format() is used as invalid return value in some places.
At the moment that does at least one heap allocation.
Now only for formats loaded from the XML a heap allocation is done
Details
make && make test
Diff Detail
- Repository
- R216 Syntax Highlighting
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
There is no test missing, these code paths are anyways already tested.
;=) Otherwise I would not have found all segfaults.
But I can call all accessors on some default constructed Format() if that increases the confidence, if this patch is at all wanted.
It saves heap allocations but adds some branches.
Btw, would it also work to point to an internal static d-pointer to avoid all the d-pointer checking? The current patch is likely fine, but it feels a bit messy... But I'd agree to this if other solutions are not feasible.
A shared static null object should work with QExplicitlySharedDataPointer as with any other d pointer type: just copy the null object into d in the default ctor. Trades the conditions per call for an atomic increment/decrement per instance.
If that is more elegant, I would be happy with this a solution, too.
The Format() allocations just showed up as a few MBs during heaptrack profiling of KWrite (just loading one large file and rehighlighting it).
I have no strong preference for either of those approaches, but I do agree with this optimization of course :)
Just for the implementation: Is the static default than just a e.g. function static QExplicitlySharedDataPointer<FormatPrivate>(new FormatPrivate()) like:
QExplicitlySharedDataPointer<FormatPrivate> &staticDefault()
{
static QExplicitlySharedDataPointer<FormatPrivate> default(new FormatPrivate()); return default;
}
and the constructor will do
Format::Format() : d(staticDefault())
{
}
or is that some other trick?
That looks much nicer an avoids the !d checks, which are tedious.
I think the one ref count change is no issue, at least that is orders of magnitude better than some malloc on Windows :)
I will adapt the patch.
Hmm, if we go that route, what to do in:
FormatPrivate* FormatPrivate::get(const Format &format)
shall one detach there always?