bugGNU nano - Bugs: bug #51491, any cursor movement on a huge...

 
 

bug #51491: any cursor movement on a huge softwrapped line is slow

Submitted by:  Benno Schulenberg <bens>
Submitted on:  Mon 17 Jul 2017 11:55:56 AM UTC  
 
Severity: 3 - NormalStatus: None
Assigned to: NoneOpen/Closed: Open

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 30 Oct 2017 08:50:28 PM UTC, comment #12:

Okay. I suppose that something like it would only be acceptable if it's part of a larger attempt at caching softwrap data, but that would of course take much more time and effort to implement.

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 30 Oct 2017 08:18:52 PM UTC, comment #11:

There is some improvement, but it's "too small" -- it's not a difference of day and night, so I don't think it's worth the extra code.

Benno Schulenberg <bens>
Project Administrator
Sun 29 Oct 2017 07:25:37 PM UTC, comment #10:

Since it's been awhile, I've attached a resync of the second patch against git 3f27c31.

(file #42293)

David Lawrence Ramsey <dolorous>
Project Administrator
Thu 20 Jul 2017 10:44:25 AM UTC, comment #9:

For now, I've just applied the second patch (after updating it for the unchanged line), as it's obviously correct.

With the second patch, I can notice some improvement in the horizontal and vertical cursor movement when being far into a softwrapped line, but... I'll refrain for now.

Benno Schulenberg <bens>
Project Administrator
Thu 20 Jul 2017 03:24:32 AM UTC, comment #8:

Forget that last patch; it doesn't build in tiny mode.

The attached new patch does. Also, there's a second attached patch after it, that properly makes checks for partially-offscreen lines in edit_scroll() only get run in softwrap mode (since that's the only way lines can be partially offscreen).

(file #41258, file #41259)

David Lawrence Ramsey <dolorous>
Project Administrator
Thu 20 Jul 2017 01:44:19 AM UTC, comment #7:

How about the attached patch, against git 0e4cbd4? It saves the number of softwrapped chunks before firstcolumn in the openfilestruct, and only changes it when firstcolumn changes, thus eliminating the need to calculate it on the fly every single time.

(file #41257)

David Lawrence Ramsey <dolorous>
Project Administrator
Wed 19 Jul 2017 09:40:38 AM UTC, comment #6:

Up to five thousand columns, performance is acceptable. But when the cursor is around column ten thousand... even every move of the cursor lags, also a simple <Left> or <Right> -- which don't change chunk and should therefore be instantaneous.

The slowness for <Left> and <Right> is in place_the_cursor(), I think: it first runs through all the chunks up to firstcolumn, and then goes through all the chunks again up to xpt. Perhaps there is a way to optimize this a bit when edittop == current? (Or maybe all the movement routines, when in softwrap mode, should keep track of row and col themselves?)

Anyway, the check for "line != NULL" in that routine should be superfluous -- if it were to reach NULL, something is wrong and it should call "Mayday", or just crash.

Benno Schulenberg <bens>
Project Administrator
Tue 18 Jul 2017 12:01:41 AM UTC, comment #5:

Besides, the new softwrap treats two-column control characters and two-column Unicode characters in the same fashion, in terms of display and movement. Given that, in terms of softwrap navigation, there's no difference between a binary file (a bunch of two-column control characters, plus irregular spacing and tabs) and a text file written in, say, two-column Chinese characters, with the same irregular spacing and tabs, plus a mixture of Unix, DOS, and Mac newlines.

So it seems to me that the idea that "nano's behavior doesn't matter so much under such circumstances" is a dangerous road to go down, as similar circumstances can occur with the right (admittedly unusual) text files.

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 17 Jul 2017 07:55:09 PM UTC, comment #4:

Define "if the scrolling behaves a little weird there". nano isn't meant to be a binary editor, no, but it should still function properly if the file contains binary garbage.

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 17 Jul 2017 06:52:06 PM UTC, comment #3:

Hmmm... Working backward (without --atblanks) might put a leftedge one (single-width) character too far back, but... if my powers of imagination are right, this one-off placement cannot accumulate, the misplacement cannot get bigger than one (single-width) character off.

It will look silly, of course, when finally reaching the real start of the line, to see a bunch of characters jumping from the start of rows to the end of the preceding row, but... how often would it happen? Mostly files will consist of either single-width or double-width characters, and then there will be no ambiguity: you can walk backward perfectly and find all the correct leftedges. Binary files are a mess, of course, and one shouldn't edit them anyway, so it doesn't matter if the scrolling behaves a little weird there.

So... I would be curious to see how it would work in practice.

Benno Schulenberg <bens>
Project Administrator
Mon 17 Jul 2017 03:56:19 PM UTC, comment #2:

Actually, after some checking, edit_scroll() does have to redraw the entire line, but that's because it uses update_softwrapped_line(), which does that. That's useful if the text has changed, but in this case, we've only moved, so it hasn't.

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 17 Jul 2017 03:47:31 PM UTC, comment #1:

Redrawing the screen already tells ncurses to move the text one row down and redraw only the top row; that's what edit_scroll() does, and M+- indirectly calls it.

Moving back one chunk is the slow part, because with softwrapped chunks' now having variable length, nano has to scan the entire line to determine where the leftedges are, since their positions depend on the actual text of the line (although less so when not using --atblanks).

Moving back character by character as you suggest won't fix this, since nano would still have to know in advance where all the leftedges are in order to know when to stop moving. Some way of caching the leftedges would fix it, though.

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 17 Jul 2017 11:55:56 AM UTC, original submission:

Open some file that contains a line of enormous length. Find that line and press <End> <End> on it, so we're actually at the very end of that line. Then type M+- (that is: order nano to move the whole text one row down). This takes something like two seconds.

Where is the slowness? In the actual moving back one chunk? Or in the redrawing of the screen? (Does the entire screen get redrawn? This should not be necessary: it can just tell ncurses to move all the text one row down and then redraw only the top row, no?)

If the slowness is in moving back one chunk, maybe that routine can be modified to actually move back character by character by at most editwincols from the x position that corresponds to firstcolumn? Maybe that x position needs to be stored too so that it doesn't need to be recomputed?

Benno Schulenberg <bens>
Project Administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by dolorous (Posted a comment)
  • -unavailable- added by bens (Submitted the item)
  • -unavailable- added by bens
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 29 Oct 2017 07:25:37 PM UTCdolorousAttached File-=>Added 0001-softwrap-save-the-number-of-softwrapped-chunks-befor.patch, #42293
    Thu 20 Jul 2017 03:24:32 AM UTCdolorousAttached File-=>Added 0001-softwrap-save-the-number-of-softwrapped-chunks-befor.patch, #41258
      Attached File-=>Added 0002-softwrap-only-compensate-for-partially-offscreen-lin.patch, #41259
    Thu 20 Jul 2017 01:44:19 AM UTCdolorousAttached File-=>Added 0001-softwrap-save-the-number-of-softwrapped-chunks-befor.patch, #41257
    Wed 19 Jul 2017 09:40:38 AM UTCbensSummaryscrolling up one row on a huge softwrapped line takes too long=>any cursor movement on a huge softwrapped line is slow
    Mon 17 Jul 2017 11:55:56 AM UTCbensCarbon-Copy-=>Added dolorous

    Back to the top


    Powered by Savane 3.1-cleanup1