vim-patch:9.1.0138: too many STRLEN calls when getting a memline (#27799)

Problem:  too many STRLEN calls when getting a memline
Solution: Optimize calls to STRLEN(), add a few functions in memline.c
          that return the byte length instead of relying on STRLEN()
          (John Marriott)

closes: vim/vim#14052

02d7a6c6cf

Cherry-pick line break changes from patch 8.1.0226.
Cherry-pick ml_line_len from patch 8.1.0579.
Cherry-pick test_comments.vim change from patch 9.1.0153.

Co-authored-by: John Marriott <basilisk@internode.on.net>

runtime/doc/dev_vimpatch.txt

@@ -209,6 +209,8 @@ information.
   utf_off2cells                                       grid_off2cells
   ml_get_curline                                    get_cursor_line_ptr
   ml_get_cursor                                     get_cursor_pos_ptr
+  ml_get_curline_len                                get_cursor_line_len
+  ml_get_cursor_len                                 get_cursor_pos_len
   screen_char                                             ui_line
   screen_line                                        grid_put_linebuf
   screen_* (most functions)                               grid_*

src/nvim/change.c

@@ -926,21 +926,24 @@ int del_bytes(colnr_T count, bool fixpos_arg, bool use_delcombine)
     count = oldlen - col;
     movelen = 1;
   }
+  colnr_T newlen = oldlen - count;
 
   // If the old line has been allocated the deletion can be done in the
   // existing line. Otherwise a new line has to be allocated.
-  bool was_alloced = ml_line_alloced();     // check if oldp was allocated
+  bool alloc_newp = !ml_line_alloced();     // check if oldp was allocated
   char *newp;
-  if (was_alloced) {
+  if (!alloc_newp) {
     ml_add_deleted_len(curbuf->b_ml.ml_line_ptr, oldlen);
     newp = oldp;                            // use same allocated memory
   } else {                                  // need to allocate a new line
-    newp = xmalloc((size_t)(oldlen + 1 - count));
+    newp = xmalloc((size_t)newlen + 1);
     memmove(newp, oldp, (size_t)col);
   }
   memmove(newp + col, oldp + col + count, (size_t)movelen);
-  if (!was_alloced) {
+  if (alloc_newp) {
     ml_replace(lnum, newp, false);
+  } else {
+    curbuf->b_ml.ml_line_len -= count;
   }
 
   // mark the buffer as changed and prepare for displaying

src/nvim/cursor.c

@@ -514,3 +514,15 @@ char *get_cursor_pos_ptr(void)
 {
   return ml_get_buf(curbuf, curwin->w_cursor.lnum) + curwin->w_cursor.col;
 }
+
+/// @return  length (excluding the NUL) of the cursor line.
+colnr_T get_cursor_line_len(void)
+{
+  return ml_get_buf_len(curbuf, curwin->w_cursor.lnum);
+}
+
+/// @return  length (excluding the NUL) of the cursor position.
+colnr_T get_cursor_pos_len(void)
+{
+  return ml_get_buf_len(curbuf, curwin->w_cursor.lnum) - curwin->w_cursor.col;
+}

src/nvim/edit.c

@@ -3785,9 +3785,10 @@ static bool ins_bs(int c, int mode, int *inserted_space_p)
         if (has_format_option(FO_AUTO)
             && has_format_option(FO_WHITE_PAR)) {
           char *ptr = ml_get_buf_mut(curbuf, curwin->w_cursor.lnum);
-          int len = (int)strlen(ptr);
+          int len = get_cursor_line_len();
           if (len > 0 && ptr[len - 1] == ' ') {
             ptr[len - 1] = NUL;
+            curbuf->b_ml.ml_line_len--;
           }
         }
 
@@ -4411,13 +4412,13 @@ static bool ins_tab(void)
       if (i > 0) {
         STRMOVE(ptr, ptr + i);
         // correct replace stack.
-        if ((State & REPLACE_FLAG)
+        if ((State & REPLACE_FLAG) && !(State & VREPLACE_FLAG)) {
-            && !(State & VREPLACE_FLAG)) {
           for (temp = i; --temp >= 0;) {
             replace_join(repl_off);
           }
         }
         if (!(State & VREPLACE_FLAG)) {
+          curbuf->b_ml.ml_line_len -= i;
           inserted_bytes(fpos.lnum, change_col,
                          cursor->col - change_col, fpos.col - change_col);
         }
@@ -4462,8 +4463,7 @@ bool ins_eol(int c)
   // Strange Vi behaviour: In Replace mode, typing a NL will not delete the
   // character under the cursor.  Only push a NUL on the replace stack,
   // nothing to put back when the NL is deleted.
-  if ((State & REPLACE_FLAG)
+  if ((State & REPLACE_FLAG) && !(State & VREPLACE_FLAG)) {
-      && !(State & VREPLACE_FLAG)) {
     replace_push(NUL);
   }
 

src/nvim/memline.c

@@ -1174,7 +1174,7 @@ void ml_recover(bool checkext)
   } else {
     for (idx = 1; idx <= lnum; idx++) {
       // Need to copy one line, fetching the other one may flush it.
-      p = xstrdup(ml_get(idx));
+      p = xstrnsave(ml_get(idx), (size_t)ml_get_len(idx));
       int i = strcmp(p, ml_get(idx + lnum));
       xfree(p);
       if (i != 0) {
@@ -1834,6 +1834,22 @@ char *ml_get_pos(const pos_T *pos)
   return ml_get_buf(curbuf, pos->lnum) + pos->col;
 }
 
+/// @return  length (excluding the NUL) of the given line.
+colnr_T ml_get_len(linenr_T lnum)
+{
+  return ml_get_buf_len(curbuf, lnum);
+}
+
+/// @return  length (excluding the NUL) of the given line in the given buffer.
+colnr_T ml_get_buf_len(buf_T *buf, linenr_T lnum)
+{
+  if (*ml_get_buf(buf, lnum) == NUL) {
+    return 0;
+  }
+
+  return buf->b_ml.ml_line_len - 1;
+}
+
 /// @return  codepoint at pos. pos must be either valid or have col set to MAXCOL!
 int gchar_pos(pos_T *pos)
   FUNC_ATTR_NONNULL_ARG(1)
@@ -1865,6 +1881,7 @@ static char *ml_get_buf_impl(buf_T *buf, linenr_T lnum, bool will_change)
     ml_flush_line(buf, false);
 errorret:
     STRCPY(questions, "???");
+    buf->b_ml.ml_line_len = 4;
     buf->b_ml.ml_line_lnum = lnum;
     return questions;
   }
@@ -1873,6 +1890,7 @@ errorret:
   }
 
   if (buf->b_ml.ml_mfp == NULL) {       // there are no lines
+    buf->b_ml.ml_line_len = 1;
     return "";
   }
 
@@ -1903,8 +1921,14 @@ errorret:
 
     DataBlock *dp = hp->bh_data;
 
-    char *ptr = (char *)dp + (dp->db_index[lnum - buf->b_ml.ml_locked_low] & DB_INDEX_MASK);
+    int idx = lnum - buf->b_ml.ml_locked_low;
-    buf->b_ml.ml_line_ptr = ptr;
+    unsigned start = (dp->db_index[idx] & DB_INDEX_MASK);
+    // The text ends where the previous line starts.  The first line ends
+    // at the end of the block.
+    unsigned end = idx == 0 ? dp->db_txt_end : (dp->db_index[idx - 1] & DB_INDEX_MASK);
+
+    buf->b_ml.ml_line_ptr = (char *)dp + start;
+    buf->b_ml.ml_line_len = (colnr_T)(end - start);
     buf->b_ml.ml_line_lnum = lnum;
     buf->b_ml.ml_flags &= ~(ML_LINE_DIRTY | ML_ALLOCATED);
   }
@@ -1922,7 +1946,8 @@ errorret:
 #ifdef ML_GET_ALLOC_LINES
   if ((buf->b_ml.ml_flags & (ML_LINE_DIRTY | ML_ALLOCATED)) == 0) {
     // make sure the text is in allocated memory
-    buf->b_ml.ml_line_ptr = xstrdup(buf->b_ml.ml_line_ptr);
+    buf->b_ml.ml_line_ptr = xmemdup(buf->b_ml.ml_line_ptr,
+                                    (size_t)buf->b_ml.ml_line_len);
     buf->b_ml.ml_flags |= ML_ALLOCATED;
     if (will_change) {
       // can't make the change in the data block
@@ -2468,6 +2493,7 @@ int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy, bool noallo
   }
 
   buf->b_ml.ml_line_ptr = line;
+  buf->b_ml.ml_line_len = (colnr_T)strlen(line) + 1;
   buf->b_ml.ml_line_lnum = lnum;
   buf->b_ml.ml_flags = (buf->b_ml.ml_flags | ML_LINE_DIRTY) & ~ML_EMPTY;
   if (noalloc) {
@@ -2765,7 +2791,7 @@ static void ml_flush_line(buf_T *buf, bool noalloc)
       } else {  // text of previous line follows
         old_len = (int)(dp->db_index[idx - 1] & DB_INDEX_MASK) - start;
       }
-      colnr_T new_len = (colnr_T)strlen(new_line) + 1;
+      colnr_T new_len = buf->b_ml.ml_line_len;
       int extra = new_len - old_len;            // negative if lines gets smaller
 
       // if new line fits in data block, replace directly
@@ -3456,7 +3482,7 @@ static char *findswapname(buf_T *buf, char **dirp, char *old_fname, bool *found_
 
             char *const name = xmalloc(name_len);
             memcpy(name, sw_msg_1, sw_msg_1_len + 1);
-            home_replace(NULL, fname, &name[sw_msg_1_len], fname_len, true);
+            home_replace(NULL, fname, name + sw_msg_1_len, fname_len, true);
             xstrlcat(name, sw_msg_2, name_len);
             int dialog_result
               = do_dialog(VIM_WARNING,
@@ -3734,7 +3760,7 @@ static void ml_updatechunk(buf_T *buf, linenr_T line, int len, int updtype)
     // First line in empty buffer from ml_flush_line() -- reset
     buf->b_ml.ml_usedchunks = 1;
     buf->b_ml.ml_chunksize[0].mlcs_numlines = 1;
-    buf->b_ml.ml_chunksize[0].mlcs_totalsize = (int)strlen(buf->b_ml.ml_line_ptr) + 1;
+    buf->b_ml.ml_chunksize[0].mlcs_totalsize = buf->b_ml.ml_line_len;
     return;
   }
 

src/nvim/memline_defs.h

@@ -56,6 +56,7 @@ typedef struct {
 #define ML_ALLOCATED    0x10    // ml_line_ptr is an allocated copy
   int ml_flags;
 
+  colnr_T ml_line_len;          // length of the cached line + NUL
   linenr_T ml_line_lnum;        // line number of cached line, 0 if not valid
   char *ml_line_ptr;            // pointer to cached line
   size_t ml_line_offset;        // cached byte offset of ml_line_lnum

src/nvim/normal.c

@@ -3223,8 +3223,7 @@ static void nv_colon(cmdarg_T *cap)
     clearop(cap->oap);
   } else if (cap->oap->op_type != OP_NOP
              && (cap->oap->start.lnum > curbuf->b_ml.ml_line_count
-                 || cap->oap->start.col >
+                 || cap->oap->start.col > ml_get_len(cap->oap->start.lnum)
-                 (colnr_T)strlen(ml_get(cap->oap->start.lnum))
                  || did_emsg)) {
     // The start of the operator has become invalid by the Ex command.
     clearopbeep(cap->oap);
@@ -3592,7 +3591,7 @@ bool get_visual_text(cmdarg_T *cap, char **pp, size_t *lenp)
   }
   if (VIsual_mode == 'V') {
     *pp = get_cursor_line_ptr();
-    *lenp = strlen(*pp);
+    *lenp = (size_t)get_cursor_line_len();
   } else {
     if (lt(curwin->w_cursor, VIsual)) {
       *pp = ml_get_pos(&curwin->w_cursor);
@@ -4527,9 +4526,8 @@ static void nv_replace(cmdarg_T *cap)
   }
 
   // Abort if not enough characters to replace.
-  char *ptr = get_cursor_pos_ptr();
+  if ((size_t)get_cursor_pos_len() < (unsigned)cap->count1
-  if (strlen(ptr) < (unsigned)cap->count1
+      || (mb_charlen(get_cursor_pos_ptr()) < cap->count1)) {
-      || (mb_charlen(ptr) < cap->count1)) {
     clearopbeep(cap->oap);
     return;
   }
@@ -5347,7 +5345,7 @@ static void nv_gi_cmd(cmdarg_T *cap)
   if (curbuf->b_last_insert.mark.lnum != 0) {
     curwin->w_cursor = curbuf->b_last_insert.mark;
     check_cursor_lnum(curwin);
-    int i = (int)strlen(get_cursor_line_ptr());
+    int i = (int)get_cursor_line_len();
     if (curwin->w_cursor.col > (colnr_T)i) {
       if (virtual_active()) {
         curwin->w_cursor.coladd += curwin->w_cursor.col - i;
@@ -6036,7 +6034,7 @@ bool unadjust_for_sel(void)
       mark_mb_adjustpos(curbuf, pp);
     } else if (pp->lnum > 1) {
       pp->lnum--;
-      pp->col = (colnr_T)strlen(ml_get(pp->lnum));
+      pp->col = ml_get_len(pp->lnum);
       return true;
     }
   }

src/nvim/textformat.c

@@ -86,8 +86,7 @@ void internal_format(int textwidth, int second_indent, int flags, bool format_on
 
   // When 'ai' is off we don't want a space under the cursor to be
   // deleted.  Replace it with an 'x' temporarily.
-  if (!curbuf->b_p_ai
+  if (!curbuf->b_p_ai && !(State & VREPLACE_FLAG)) {
-      && !(State & VREPLACE_FLAG)) {
     cc = gchar_cursor();
     if (ascii_iswhite(cc)) {
       save_char = (char)cc;

test/old/testdir/test_comments.vim

@@ -237,6 +237,12 @@ func Test_comment_autoformat()
   call feedkeys("aone\ntwo\n", 'xt')
   call assert_equal(['one', 'two', ''], getline(1, '$'))
 
+  set backspace=indent,eol,start
+  %d
+  call feedkeys("aone \n\<BS>", 'xt')
+  call assert_equal(['one'], getline(1, '$'))
+  set backspace&
+
   close!
 endfunc