Skip to content

Fix detach in TextRange::fixLookup()

Waqar Ahmed requested to merge waqar/ktexteditor:fix-detach into master

Searching the same document from utilities/kate!136 (merged) takes about ~6 seconds on my machine (Intel core i5, 4300U - 8GB Ram). This change was one of the first thing I noticed. With more optimizations, I was able to bring the search down to about ~3 seconds but the changes for that may not be correct / hold for all scenarios, so, I am listing them down here for discussion:

About 90% of the time is being spent in TextBlock::updateRange() where we have a check:

if (!isSingleLine && m_uncachedRanges.contains(range)) { return; }

The call to contains() isn't particularly expensive, only that there are too many calls to it. At first I thought there is no way we can go further here because it seems reasonable. We can try to reduce the number of calls to it which will require major changes so that's not an option. So, just to check, I printed the size of the elements in m_uncachedRanges. It turns out that there is always just 1 element. Armed with this information, I changed m_uncachedRanges to a QVarLengthArray and the performance increased by almost 1 second.

Next, I moved this check to the top of the function since its the likely case in this scenario. Just this change resulted in almost a 500ms speedup.

Moving back to TextRange::fixLookup(), I switched the following condition to have the likely case first:

        if ((endLine < block->startLine()) || (startLine >= (block->startLine() + block->lines()))) {
            block->removeRange(this);
        } else {
            block->updateRange(this); // this happens a **lot** more often,likely case
        }

This resulted in another 300ms speedup.

Moving further, caching block->startLine() and block->lines() into local variables also results in a small speedup.

Other than this there's this comment in KateSearchBar::findOrReplaceAll()`:

    // we highlight all ranges of a replace, up to some hard limit
    // e.g. if you replace 100000 things, rendering will break down otherwise ;=)
    const int maxHighlightings = 65536;

Removing this limit didn't have any noticeable affects for me with about 300,000 replaces

Merge request reports