[scintilla:feature-requests] #1481 WrapBlock improvements

Zufu Liu

unread,

Mar 9, 2023, 7:07:39 AM3/9/23

to scintill...@googlegroups.com

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu
Last Updated: Thu Mar 09, 2023 12:07 PM UTC
Owner: Neil Hodgson

A. topLine seems need to be (document line) pcs->DocFromDisplay(topLine):

    const SignificantLines significantLines {
        pdoc->SciLineFromPosition(sel.MainCaret()),
        topLine,
        LinesOnScreen() + 1,
        view.llc.GetLevel(),
    };

B. it might worth to add RetrieveLineLayout() overload with SignificantLines parameter to avoid repeated call inside current RetrieveLineLayout() method, e.g. LineRange, lineCaret and LinesOnScreen() are repeated.

C. the entire LineRange to LayoutLine block for short and long lines looks very similar, could be extracted as a method.

Sent from sourceforge.net because scintill...@googlegroups.com is subscribed to https://sourceforge.net/p/scintilla/feature-requests/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/scintilla/admin/feature-requests/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

Neil Hodgson

unread,

Mar 9, 2023, 11:38:18 PM3/9/23

to scintill...@googlegroups.com

A. OK, fixed with [ca6c2d].

B. Trying it didn't improve performance even for whole file ASCII monospace. Profiling with monospace points to BreakFinder and memory allocation / free as heavy.

C. The main difference is the locking which only occurs for short lines. Extracting the code into a method requires around 8 parameters and long parameter lists are a code smell with the caller and callee too intertwined to be easily understood. Maybe some other refactoring could help.

Zufu Liu

unread,

Mar 11, 2023, 6:26:59 PM3/11/23

to scintill...@googlegroups.com

B. what about following (adding linesTotal and styleClock into SignificantLines):

std::shared_ptr<LineLayout> LineLayoutCache::Retrieve(Sci::Line lineNumber, const SignificantLines &significantlines, Sci::Position lengthLine) {
    return Retrieve(lineNumber, significantlines.lineCaret,
        static_cast<int>(lengthLine), significantlines.styleClock,
        significantlines.linesOnScreen, significantlines.linesTotal);
}

C. lengthLine >= lengthToMultiThread can be avoided in long lines loop, it can be replaced by linesAfterWrap[indexLarge] == 0 (maybe in a range-based for loop).

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri Mar 10, 2023 04:38 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,

May 26, 2026, 6:31:01 AM (8 days ago) May 26

to scintill...@googlegroups.com

Merge allocation (similar to PositionCacheEntry) for chars, styles and positions on LineLayout can reduce wrap time (tested file with mostly ASCII).
pseudo code:

unsigned length = maxLineLength_ + 2;
size_t lineAllocation = align_up(length, 16); // could be 8 or 16
chars = std::make_unique<char[]>(lineAllocation*2 + length*sizeof(XYPOSITION));
styles = chars.get() + lineAllocation;
positions = styles + lineAllocation;

this doesn't increase memory usage as system memory allocation is 8 or 16 aligned, it increased locality for chars and styles.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Sat Mar 11, 2023 11:26 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,

May 26, 2026, 5:57:46 PM (8 days ago) May 26

to scintill...@googlegroups.com

How dependable is 'system memory allocation is 8 or 16 aligned'? I could imagine an allocator (perhaps chosen by the application) wanting to be tighter for char blocks. Wouldn't want to crash or invoke UB on other CPUs accessing positions[i].

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Tue May 26, 2026 10:30 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,

May 26, 2026, 8:15:26 PM (8 days ago) May 26

to scintill...@googlegroups.com

Merging positions into the allocation gets in the way of the change I most want here: halving the size of positions by storing single precision floats instead of doubles. Doubles are currently used just to accomodate running out of precision on wide lines. A main array of floats can be augmented with occasional doubles (perhaps every 1000 positions) with the main array values then being offsets from the previous accurate value.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Tue May 26, 2026 09:57 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,

May 27, 2026, 6:06:57 AM (7 days ago) May 27

to scintill...@googlegroups.com

I think application can't provide class specific operator new for language builtin primary type like char. for custom global operator new (which has no type info), if alignment for it's returned pointer is less than builtin default operator new, the application is already broken.
for builtin operator new, the returned pointer seems has alignment __STDCPP_DEFAULT_NEW_ALIGNMENT__ ( I think it's >= alignof(std::max_align_t) >= alignof(double)).
https://en.cppreference.com/cpp/memory/new/operator_new

for win32 msvcrt/ucrt, the alignment is MEMORY_ALLOCATION_ALIGNMENT (https://learn.microsoft.com/en-us/windows/win32/api/heapapi/nf-heapapi-heapalloc#remarks). glibc seems is 16 (for int128/float128 or better simd?).

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed May 27, 2026 12:15 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,

May 27, 2026, 7:35:47 PM (7 days ago) May 27

to scintill...@googlegroups.com

For 32-bit MSVC builds __STDCPP_DEFAULT_NEW_ALIGNMENT__ and MEMORY_ALLOCATION_ALIGNMENT are 8 which is still fine.

clang has a -fnew-alignment option but that seems more oriented at increasing default alignment.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed May 27, 2026 10:06 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,

May 28, 2026, 6:14:29 AM (6 days ago) May 28

to scintill...@googlegroups.com

Here are changes (contains some naive timing), tested SciTE the benefit for merge memory allocation seem is less than adding if (linesAfterWrap[indexLarge] == 0) { check for long lines.

Attachments:

LineLayout-0528.diff (sourceforge.net) (7.1 kB; text/plain)

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Wed May 27, 2026 11:35 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,

May 29, 2026, 7:33:23 PM (5 days ago) May 29

to scintill...@googlegroups.com

Changing if (lengthLine for long lines seems reasonable but would indicate that the code being moved, likely the pdoc->LineRange, is costly.

I've thought about this in the past and a method that returns the starts of 2 consecutive lines would help. A variant of the called Partitioning::PositionFromPartition could return 2 values without performing some of the code twice. There'd have to be some extra end and step handling but it may be a win.

Alternatively, early in WrapBlock store the line starts for the whole range (+1 more) then use that.

Moving LineLayout member initialisation into the declaration is OK. There's a trade-off here in exposing internal decisions in a header versus hiding them from other code.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Thu May 28, 2026 10:14 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,

May 30, 2026, 9:28:27 AM (4 days ago) May 30

to scintill...@googlegroups.com

likely the pdoc->LineRange, is costly.

For general code document, most line is short, the check avoid checking all line range twice.

There's a trade-off here in exposing internal decisions in a header versus hiding them from other code.

They are flagged by clang-tidy's modernize-use-default-member-init check.

lengthToMultiThread can be changed to computed with durationWrapOneByte.ActionsInAllowedTime().

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Fri May 29, 2026 11:33 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,

May 31, 2026, 9:35:57 PM (3 days ago) May 31

to scintill...@googlegroups.com

Committed LineLayout member initialisation and wrap threading choice changes with [4709bc] and [9e085a].

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Sat May 30, 2026 01:28 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,

10:30 AM (4 hours ago) 10:30 AM

to scintill...@googlegroups.com

bool callerMultiThreaded for EditView::LayoutLine() method can be changed to a flag enum, so if (vstyle.edgeState == EdgeVisualStyle::Background) block (maybe others) can be omitted (not measured the time) for non-significant llTemporary and llLarge.

[feature-requests:#1481] WrapBlock improvements

Status: open
Group: Initial
Labels: scintilla layout
Created: Thu Mar 09, 2023 12:07 PM UTC by Zufu Liu

Last Updated: Mon Jun 01, 2026 01:35 AM UTC
Owner: Neil Hodgson

Reply all

Reply to author

Forward