#1502 Reduce LineLayoutCache::Invalidate() time for replacing text

[Zufu Liu]

Performance measurement shows Editor::CheckModificationForWrap() is a hot function for replace all, where most time spend on view.llc.Invalidate(LineLayout::ValidLevel::checkTextAndStyle);.

The time can be reduced by change allInvalidated to record rough max validity of all line layout:

 void LineLayoutCache::Invalidate(LineLayout::ValidLevel validity_) noexcept {
-   if (!cache.empty() && !allInvalidated) {
+   if (!cache.empty() && maxValidity > validity_) {
+       maxValidity = validity_;
        for (const std::shared_ptr<LineLayout> &ll : cache) {
            if (ll) {
                ll->Invalidate(validity_);
            }
        }
-       if (validity_ == LineLayout::ValidLevel::invalid) {
-           allInvalidated = true;
-       }
    }
 }

so the loop will only be entered once.

On my i5 with page cache, this change reduced about 200ms for SciTEBase::DoReplaceAll() on replacing all dot to comma for 0N 9V.txt at https://github.com/notepad-plus-plus/notepad-plus-plus/issues/10930#issuecomment-998760967

[Neil Hodgson]

This seems worthwhile. The removal of the if also seems to make Visual C++ inline this code which helps further.

Its delayed by the macOS crash fix release.

[Zufu Liu]

Should me add another patch to inline Editor::CheckModificationForWrap()? which is basically https://github.com/zufuliu/notepad2/commit/4e872ae2300a9b2fd2169ed8b811cdca94399d5a

It also moved code after // Some lines are hidden so may need shown. into a separate function as the block not frequent executed, also to reduce code size of Editor::NotifyModified().

[Zufu Liu]

!cache.empty() can also be removed, as the range based for loop already checks empty.

[Zufu Liu]

Patch removed !cache.empty() from Invalidate() and added maxValidity = LineLayout::ValidLevel::invalid; into Deallocate().

[Neil Hodgson]

Committed with [2d34aa].

[Zufu Liu]

A somewhat unrelated patch: simplify some FlagSet() codes, https://github.com/zufuliu/notepad2/commit/9de4a994ceb266f1bca583dd4eb1009e641e80be

[Neil Hodgson]

This produces different results, probably the ordering of the clauses in first hunk:

@@ -2266,17 +2266,11 @@ void DrawFoldLines(Surface *surface, const EditModel &model, const ViewStyle &vs
            vsDraw.markers[static_cast<int>(MarkerOutline::Folder)].fore);
        // Paint the line above the fold
        // Paint the line above the fold
-       if ((subLine == 0) &&
-           ((expanded && (FlagSet(model.foldFlags, FoldFlag::LineBeforeExpanded)))
-           ||
-           (!expanded && (FlagSet(model.foldFlags, FoldFlag::LineBeforeContracted))))) {
+       if ((subLine == 0) && FlagSet(model.foldFlags, (expanded ? FoldFlag::LineBeforeContracted : FoldFlag::LineBeforeExpanded))) {
            surface->FillRectangleAligned(Side(rcLine, Edge::top, 1.0), foldLineColour);
        }
        // Paint the line below the fold
-       if (lastSubLine &&
-           ((expanded && (FlagSet(model.foldFlags, FoldFlag::LineAfterExpanded)))
-           ||
-           (!expanded && (FlagSet(model.foldFlags, FoldFlag::LineAfterContracted))))) {
+       if (lastSubLine && FlagSet(model.foldFlags, (expanded ? FoldFlag::LineAfterExpanded : FoldFlag::LineAfterContracted))) {
            surface->FillRectangleAligned(Side(rcLine, Edge::bottom, 1.0), foldLineColour);
            // If contracted fold line drawn then don't overwrite with hidden line
            // as fold lines are more specific then hidden lines.

There is also a duplicated "Paint the line above the fold" comment that makes merging difficult.

[Zufu Liu]

Fixed the typo.

[Neil Hodgson]

This hunk appears incorrect: what is the second mh.modificationType doing?

        SetScrollBars();
    }

-   if ((FlagSet(mh.modificationType, ModificationFlags::ChangeMarker)) || (FlagSet(mh.modificationType, ModificationFlags::ChangeMargin))) {
+   if (FlagSet(mh.modificationType, (ModificationFlags::ChangeMarker | mh.modificationType, ModificationFlags::ChangeMargin))) {
        if ((!willRedrawAll) && ((paintState == PaintState::notPainting) || !PaintContainsMargin())) {
            if (FlagSet(mh.modificationType, ModificationFlags::ChangeFold)) {
                // Fold changes can affect the drawing of following lines so redraw whole margin

[Zufu Liu]

Fixed.

[Neil Hodgson]

Committed as [7f2549].