Skip to content

Optimize copy of State

Jonathan Poelen requested to merge jpoelen/syntax-highlighting:ft_opti_copy_state into master

Previous time with highlight_benchmark: 20.2s

New Time with highlight_benchmark: 19.5s

My analysis for the next stage:

Looking at how State is used and how it could be used more efficiently, I think we should change the interface for KF6:

  • Prohibit copying and only allow moving
  • no longer have QExplicitlySharedDataPointer as a member

Zero copy

The 3 projects I looked at make copies of objects that are ultimately temporary. Copies that can be avoided with references and moves. Note that when I say copy, I mean increment the internal counter. But as this commit shows, which reduces its use, there is an impact that is not so negligible (3.4% here (which seems a lot to me, but I've done the measurements several times and used ksyntaxhighlighter6 -fansi on 25K, 100K and 600K line files, I still get around 3%)).

    const KSyntaxHighlighting::State initialState(!prevLine ? KSyntaxHighlighting::State() : prevLine->highlightingState()); // copy
    const KSyntaxHighlighting::State endOfLineState = highlightLine(textLine->text(), initialState);

    if (textLine->highlightingState() != endOfLineState) {
        textLine->setHighlightingState(endOfLineState);  // copy internally
    }

The first copy can be avoided by first declaring a State variable initialized by default and using a reference like this:

    /*static*/ const KSyntaxHighlighting::State emptyState; // we could provide `static const State &State::emptyState() noexcept`
                                                            // or global variable
    const KSyntaxHighlighting::State &initialState(!prevLine ? emptyState : prevLine->highlightingState()); // no-copy
    const KSyntaxHighlighting::State endOfLineState = highlightLine(textLine->text(), initialState);

The second copy can be replaced by a move: textLine->setHighlightingState(std::move(endOfLineState))

    KSyntaxHighlighting::State previousLineState;
    if (TextBlockUserData *data = TextDocumentLayout::textUserData(previousBlock))
        previousLineState = data->syntaxState(); // copy + copy (syntaxState returns a copy instead of a reference)
    KSyntaxHighlighting::State oldState;
    if (TextBlockUserData *data = TextDocumentLayout::textUserData(block)) {
        oldState = data->syntaxState(); // copy + copy
    }
    KSyntaxHighlighting::State state = highlightLine(text, previousLineState);
    if (oldState != state) {
        TextBlockUserData *data = TextDocumentLayout::userData(block);
        data->setSyntaxState(state); // copy + copy
    }

Same principle as for KTE: initialized variable + use of reference / pointer. 6 copies replaced by just one construct and 1 move.

  • This project with SyntaxHighlighter behaves in the same way as the other two.

Following this analysis, I think that disallowing copying will result in optimal code. We could add a clone() function that does explicit copying, but I doubt it would have any real use.

No SharedDataPointer

The second point concerns QExplicitlySharedDataPointer. Since copying is useless, so is sharing. At worst, the user can put State in his own class that implements sharing. Something already done in KTE, for example, where textLine is, if I've understood correctly, always in a std::shared_ptr. I get the impression that shared classes containing shared values is a very pre-C++11 recurring pattern, when move semantics did not exist...

Merge request reports