vim-patch:8.1.1631: displaying signs is inefficient

+ support for neovim's dynamic width signcolumn

Problem:    Displaying signs is inefficient.
Solution:   Avoid making multiple calls to get information about a placed
            sign. (Yegappan Lakshmanan, closes #4586)

src/nvim/option.c

@@ -7607,7 +7607,9 @@ int win_signcol_count(win_T *wp)
     }
   }
 
-  return MAX(minimum, MIN(maximum, needed_signcols));
+  int ret = MAX(minimum, MIN(maximum, needed_signcols));
+  assert(ret <= SIGN_SHOW_MAX);
+  return ret;
 }
 
 /// Get window or buffer local options

src/nvim/screen.c

@@ -2084,6 +2084,8 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
   colnr_T trailcol = MAXCOL;            // start of trailing spaces
   colnr_T leadcol = 0;                  // start of leading spaces
   bool need_showbreak = false;          // overlong line, skip first x chars
+  sign_attrs_T sattrs[SIGN_SHOW_MAX];   // attributes for signs
+  int num_signs;                        // number of signs for line
   int line_attr = 0;                    // attribute for the whole line
   int line_attr_lowprio = 0;            // low-priority attribute for the line
   matchitem_T *cur;                     // points to the match list
@@ -2375,11 +2377,14 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
     wp->w_last_cursorline = wp->w_cursor.lnum;
   }
 
+  memset(sattrs, 0, sizeof(sattrs));
+  num_signs = buf_get_signattrs(wp->w_buffer, lnum, sattrs);
+
   // If this line has a sign with line highlighting set line_attr.
   // TODO(bfredl, vigoux): this should not take priority over decoration!
-  v = buf_getsigntype(wp->w_buffer, lnum, SIGN_LINEHL, 0, 1);
+  sign_attrs_T * sattr = sign_get_attr(SIGN_LINEHL, sattrs, 0, 1);
-  if (v != 0) {
+  if (sattr != NULL) {
-    line_attr = sign_get_attr((int)v, SIGN_LINEHL);
+    line_attr = sattr->linehl;
   }
 
   // Highlight the current line in the quickfix window.
@@ -2696,7 +2701,7 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
           int count = win_signcol_count(wp);
           if (count > 0) {
               get_sign_display_info(
-                  false, wp, lnum, row,
+                  false, wp, sattrs, row,
                   startrow, filler_lines, filler_todo, count,
                   &c_extra, &c_final, extra, sizeof(extra),
                   &p_extra, &n_extra,
@@ -2715,10 +2720,10 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
           // in 'lnum', then display the sign instead of the line
           // number.
           if (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u'
-              && buf_findsign_id(wp->w_buffer, lnum, (char_u *)"*") != 0) {
+              && num_signs > 0) {
             int count = win_signcol_count(wp);
             get_sign_display_info(
-                true, wp, lnum, row,
+                true, wp, sattrs, row,
                 startrow, filler_lines, filler_todo, count,
                 &c_extra, &c_final, extra, sizeof(extra),
                 &p_extra, &n_extra,
@@ -2768,11 +2773,10 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
             n_extra = number_width(wp) + 1;
             char_attr = win_hl_attr(wp, HLF_N);
 
-            int num_sign = buf_getsigntype(
+            sign_attrs_T *num_sattr = sign_get_attr(SIGN_NUMHL, sattrs, 0, 1);
-                wp->w_buffer, lnum, SIGN_NUMHL, 0, 1);
+            if (num_sattr != NULL) {
-            if (num_sign != 0) {
               // :sign defined with "numhl" highlight.
-              char_attr = sign_get_attr(num_sign, SIGN_NUMHL);
+              char_attr = num_sattr->numhl;
             } else if ((wp->w_p_cul || wp->w_p_rnu)
                        && lnum == wp->w_cursor.lnum
                        && filler_todo == 0) {
@@ -4451,7 +4455,7 @@ void screen_adjust_grid(ScreenGrid **grid, int *row_off, int *col_off)
 static void get_sign_display_info(
     bool nrcol,
     win_T *wp,
-    linenr_T lnum,
+    sign_attrs_T sattrs[],
     int row,
     int startrow,
     int filler_lines,
@@ -4468,8 +4472,6 @@ static void get_sign_display_info(
     int *sign_idxp
 )
 {
-  int text_sign;
-
   // Draw cells with the sign value or blank.
   *c_extrap = ' ';
   *c_finalp = NUL;
@@ -4481,10 +4483,9 @@ static void get_sign_display_info(
   }
 
   if (row == startrow + filler_lines && filler_todo <= 0) {
-    text_sign = buf_getsigntype(wp->w_buffer, lnum, SIGN_TEXT,
+    sign_attrs_T *sattr = sign_get_attr(SIGN_TEXT, sattrs, *sign_idxp, count);
-                                *sign_idxp, count);
+    if (sattr != NULL) {
-    if (text_sign != 0) {
+      *pp_extra = sattr->text;
-      *pp_extra = sign_get_text(text_sign);
       if (*pp_extra != NULL) {
         *c_extrap = NUL;
         *c_finalp = NUL;
@@ -4517,7 +4518,7 @@ static void get_sign_display_info(
           (*pp_extra)[*n_extrap] = NUL;
         }
       }
-      *char_attrp = sign_get_attr(text_sign, SIGN_TEXT);
+      *char_attrp = sattr->texthl;
     }
   }
 

src/nvim/sign.c

@@ -413,54 +413,101 @@ linenr_T buf_change_sign_type(
     return (linenr_T)0;
 }
 
-/// Gets a sign from a given line.
+/// Return the sign attrs which has the attribute specified by 'type'. Returns
-///
+/// NULL if a sign is not found with the specified attribute.
-/// Return the type number of the sign at line number 'lnum' in buffer 'buf'
-/// which has the attribute specified by 'type'. Returns 0 if a sign is not
-/// found at the line number or it doesn't have the specified attribute.
-/// @param buf Buffer in which to search
-/// @param lnum Line in which to search
 /// @param type Type of sign to look for
+/// @param sattrs Sign attrs to search through
 /// @param idx if there multiple signs, this index will pick the n-th
-//          out of the most `max_signs` sorted ascending by Id.
+///        out of the most `max_signs` sorted ascending by Id.
 /// @param max_signs the number of signs, with priority for the ones
-//         with the highest Ids.
+///        with the highest Ids.
-/// @return Identifier of the matching sign, or 0
+/// @return Attrs of the matching sign, or NULL
-int buf_getsigntype(buf_T *buf, linenr_T lnum, SignType type,
+sign_attrs_T * sign_get_attr(SignType type, sign_attrs_T sattrs[],
-                    int idx, int max_signs)
+                             int idx, int max_signs)
 {
-    signlist_T *sign;  // a sign in a b_signlist
+    sign_attrs_T *matches[SIGN_SHOW_MAX];
-    signlist_T *matches[9];
     int nr_matches = 0;
 
-    FOR_ALL_SIGNS_IN_BUF(buf, sign) {
+    for (int i = 0; i < SIGN_SHOW_MAX; i++) {
-        if (sign->lnum == lnum
+        if (   (type == SIGN_TEXT   && sattrs[i].text   != NULL)
-            && (type == SIGN_ANY
+            || (type == SIGN_LINEHL && sattrs[i].linehl != 0)
-                || (type == SIGN_TEXT
+            || (type == SIGN_NUMHL  && sattrs[i].numhl  != 0)) {
-                    && sign_get_text(sign->typenr) != NULL)
+            matches[nr_matches] = &sattrs[i];
-                || (type == SIGN_LINEHL
-                    && sign_get_attr(sign->typenr, SIGN_LINEHL) != 0)
-                || (type == SIGN_NUMHL
-                    && sign_get_attr(sign->typenr, SIGN_NUMHL) != 0))) {
-            matches[nr_matches] = sign;
             nr_matches++;
-            // signlist is sorted with most important (priority, id), thus we
+            // attr list is sorted with most important (priority, id), thus we
             // may stop as soon as we have max_signs matches
-            if (nr_matches == ARRAY_SIZE(matches) || nr_matches >= max_signs) {
+            if (nr_matches >= max_signs) {
                 break;
             }
         }
     }
 
-    if (nr_matches > 0) {
+    if (nr_matches > idx) {
-        if (idx >= nr_matches) {
+        return matches[nr_matches - idx - 1];
-            return 0;
-        }
-
-        return matches[nr_matches - idx -1]->typenr;
     }
 
-    return 0;
+    return NULL;
+}
+
+/// Lookup a sign by typenr. Returns NULL if sign is not found.
+static sign_T * find_sign_by_typenr(int typenr)
+{
+    sign_T *sp;
+
+    for (sp = first_sign; sp != NULL; sp = sp->sn_next) {
+        if (sp->sn_typenr == typenr) {
+            return sp;
+        }
+    }
+    return NULL;
+}
+
+/// Return the attributes of all the signs placed on line 'lnum' in buffer
+/// 'buf'. Used when refreshing the screen. Returns the number of signs.
+/// @param buf Buffer in which to search
+/// @param lnum Line in which to search
+/// @param sattrs Output array for attrs
+/// @return Number of signs of which attrs were found
+int buf_get_signattrs(buf_T *buf, linenr_T lnum, sign_attrs_T sattrs[])
+{
+    signlist_T *sign;
+    sign_T     *sp;
+
+    int nr_matches = 0;
+
+    FOR_ALL_SIGNS_IN_BUF(buf, sign) {
+        if (sign->lnum > lnum) {
+            // Signs are sorted by line number in the buffer. No need to check
+            // for signs after the specified line number 'lnum'.
+            break;
+        }
+
+        if (sign->lnum == lnum) {
+            sign_attrs_T sattr;
+            memset(&sattr, 0, sizeof(sattr));
+            sattr.typenr = sign->typenr;
+            sp = find_sign_by_typenr(sign->typenr);
+            if (sp != NULL) {
+                sattr.text = sp->sn_text;
+                if (sattr.text != NULL && sp->sn_text_hl != 0) {
+                    sattr.texthl = syn_id2attr(sp->sn_text_hl);
+                }
+                if (sp->sn_line_hl != 0) {
+                    sattr.linehl = syn_id2attr(sp->sn_line_hl);
+                }
+                if (sp->sn_num_hl != 0) {
+                    sattr.numhl = syn_id2attr(sp->sn_num_hl);
+                }
+            }
+
+            sattrs[nr_matches] = sattr;
+            nr_matches++;
+            if (nr_matches == SIGN_SHOW_MAX) {
+                break;
+            }
+        }
+    }
+    return nr_matches;
 }
 
 /// Delete sign 'id' in group 'group' from buffer 'buf'.
@@ -557,6 +604,12 @@ static signlist_T * buf_getsign_at_line(
   signlist_T  *sign;    // a sign in the signlist
 
   FOR_ALL_SIGNS_IN_BUF(buf, sign) {
+    if (sign->lnum > lnum) {
+      // Signs are sorted by line number in the buffer. No need to check
+      // for signs after the specified line number 'lnum'.
+      break;
+    }
+
     if (sign->lnum == lnum && sign_in_group(sign, groupname)) {
       return sign;
     }
@@ -1613,50 +1666,6 @@ static void sign_undefine(sign_T *sp, sign_T *sp_prev)
   xfree(sp);
 }
 
-/// Gets highlighting attribute for sign "typenr" corresponding to "type".
-int sign_get_attr(int typenr, SignType type)
-{
-  sign_T  *sp;
-  int sign_hl = 0;
-
-  for (sp = first_sign; sp != NULL; sp = sp->sn_next) {
-    if (sp->sn_typenr == typenr) {
-      switch (type) {
-        case SIGN_TEXT:
-          sign_hl = sp->sn_text_hl;
-          break;
-        case SIGN_LINEHL:
-          sign_hl = sp->sn_line_hl;
-          break;
-        case SIGN_NUMHL:
-          sign_hl = sp->sn_num_hl;
-          break;
-        default:
-          abort();
-      }
-      if (sign_hl > 0) {
-        return syn_id2attr(sign_hl);
-      }
-      break;
-    }
-  }
-  return 0;
-}
-
-/// Get text mark for sign "typenr".
-/// Returns NULL if there isn't one.
-char_u * sign_get_text(int typenr)
-{
-  sign_T  *sp;
-
-  for (sp = first_sign; sp != NULL; sp = sp->sn_next) {
-    if (sp->sn_typenr == typenr) {
-      return sp->sn_text;
-    }
-  }
-  return NULL;
-}
-
 /// Undefine/free all signs.
 void free_signs(void)
 {

src/nvim/sign_defs.h

@@ -33,16 +33,25 @@ struct signlist
     signlist_T *prev;       // previous entry -- for easy reordering
 };
 
+/// Sign attributes. Used by the screen refresh routines.
+typedef struct sign_attrs_S {
+    int     typenr;
+    char_u *text;
+    int     texthl;
+    int     linehl;
+    int     numhl;
+} sign_attrs_T;
+
+#define SIGN_SHOW_MAX 9
+
 // Default sign priority for highlighting
 #define SIGN_DEF_PRIO 10
 
-// type argument for buf_getsigntype() and sign_get_attr()
+// type argument for sign_get_attr()
 typedef enum {
-  SIGN_ANY,
   SIGN_LINEHL,
-  SIGN_ICON,
-  SIGN_TEXT,
   SIGN_NUMHL,
+  SIGN_TEXT,
 } SignType;