less space used by the object itself and only one vector instead of two => half of the needed allocations
Details
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.
compare of references works without diff, just missing diff output
this avoid spawning processes at all if all tests are fine
> nicer to use unit test (as long as they work) for perf
I improved the unit tests, too.
No need to call diff if the files are really equal, improves testing speed + makes the tests much nicer to use as benchmarks.
I fixed the tests again by adding the m_defData back.
I am not sure, if that really is the right fix.
That was like this before, but can't by accident the refData just point again to a new definition object that is newly allocated?
Should we use a DefinitionRef there?
I can't answer the m_defData question without digging deeper into this unfortunately. The rest looks good to me though.
src/lib/state.cpp | ||
---|---|---|
113 | last() has a non-const (detaching) overload, is there a risk we are hitting this here due to the d-> indirection? All other occurrences seem to be clearly on const members. |
For: I can't answer the m_defData question without digging deeper into this unfortunately. The rest looks good to me though.
Ok. I think in 99% of the cases it is save enough, thought some 100% save thing would be nicer. Actually one could just add a shared::ptr to keep the definition alive to the state.
For the indentationBasedFoldingEnabled() detach: Actually, do we want to have the implicit stuff at all there?
I would tend to just use a std::vector as stack as we do the sharing ourself (and ensure we return pointer equal states in highlightLine)
Regarding implicit sharing, is this actually shared between two (subsequent) lines with the same state (think of e.g. a multi-line license header block)? Then it might be interesting to keep it, otherwise I agree, the increased control of a std::vector might be preferable here indeed.
Regarding implicit sharing, is this actually shared between two (subsequent) lines with the same state (think of e.g. a multi-line license header block)? Then it might be interesting to keep it, otherwise I agree, the increased control of a std::vector might be preferable here indeed.
At the moment, KTextEditor compares the returned states and reuses the input state.
I would do the same in highlightLine, then you get that for free.
Proposed solution that is "safe": We use a weak reference to check for state validity.
This is bit more expensive than the raw-pointer but even save if e.g. by accident a new definition gets allocated to the same address as an old one.
I added shortcut to check for pointer equality for shared states.