patch 8.1.1631: displaying signs is inefficient

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

Author: brammool

src/proto/sign.pro

@@ -1,6 +1,6 @@
 /* sign.c */
 void init_signs(void);
-int buf_getsigntype(buf_T *buf, linenr_T lnum, int type);
+int buf_get_signattrs(buf_T *buf, linenr_T lnum, sign_attrs_T *sattr);
 linenr_T buf_delsign(buf_T *buf, linenr_T atlnum, int id, char_u *group);
 int buf_findsign(buf_T *buf, int id, char_u *group);
 int buf_findsign_id(buf_T *buf, linenr_T lnum, char_u *groupname);
@@ -13,8 +13,6 @@ int sign_undefine_by_name(char_u *name);
 void ex_sign(exarg_T *eap);
 void get_buffer_signs(buf_T *buf, list_T *l);
 void sign_gui_started(void);
-int sign_get_attr(int typenr, int line);
-char_u *sign_get_text(int typenr);
 void *sign_get_image(int typenr);
 void free_signs(void);
 char_u *get_sign_name(expand_T *xp, int idx);

src/screen.c

@@ -3042,7 +3042,8 @@ text_prop_compare(const void *s1, const void *s2)
 get_sign_display_info(
 	int		nrcol,
 	win_T		*wp,
-	linenr_T	lnum,
+	linenr_T	lnum UNUSED,
+	sign_attrs_T	*sattr,
 	int		wcr_attr,
 	int		row,
 	int		startrow,
@@ -3077,9 +3078,9 @@ get_sign_display_info(
 #endif
        )
     {
-	text_sign = buf_getsigntype(wp->w_buffer, lnum, SIGN_TEXT);
+	text_sign = (sattr->text != NULL) ? sattr->typenr : 0;
 # ifdef FEAT_SIGN_ICONS
-	icon_sign = buf_getsigntype(wp->w_buffer, lnum, SIGN_ICON);
+	icon_sign = (sattr->icon != NULL) ? sattr->typenr : 0;
 	if (gui.in_use && icon_sign != 0)
 	{
 	    // Use the image in this position.
@@ -3093,7 +3094,7 @@ get_sign_display_info(
 	    else
 		*c_extrap = SIGN_BYTE;
 #  ifdef FEAT_NETBEANS_INTG
-	    if (buf_signcount(wp->w_buffer, lnum) > 1)
+	    if (netbeans_active() && (buf_signcount(wp->w_buffer, lnum) > 1))
 	    {
 		if (nrcol)
 		{
@@ -3114,7 +3115,7 @@ get_sign_display_info(
 # endif
 	    if (text_sign != 0)
 	    {
-		*pp_extra = sign_get_text(text_sign);
+		*pp_extra = sattr->text;
 		if (*pp_extra != NULL)
 		{
 		    if (nrcol)
@@ -3127,7 +3128,7 @@ get_sign_display_info(
 		    *c_finalp = NUL;
 		    *n_extrap = (int)STRLEN(*pp_extra);
 		}
-		*char_attrp = sign_get_attr(text_sign, FALSE);
+		*char_attrp = sattr->texthl;
 	    }
     }
 }
@@ -3264,6 +3265,8 @@ win_line(
 #endif
 #if defined(FEAT_SIGNS) || defined(FEAT_QUICKFIX) \
 	|| defined(FEAT_SYN_HL) || defined(FEAT_DIFF)
+    int		sign_present = FALSE;
+    sign_attrs_T sattr;
 # define LINE_ATTR
     int		line_attr = 0;		/* attribute for the whole line */
 #endif
@@ -3585,12 +3588,15 @@ win_line(
     filler_todo = filler_lines;
 #endif
 
+#ifdef FEAT_SIGNS
+    sign_present = buf_get_signattrs(wp->w_buffer, lnum, &sattr);
+#endif
+
 #ifdef LINE_ATTR
 # ifdef FEAT_SIGNS
     /* If this line has a sign with line highlighting set line_attr. */
-    v = buf_getsigntype(wp->w_buffer, lnum, SIGN_LINEHL);
-    if (v != 0)
-	line_attr = sign_get_attr((int)v, TRUE);
+    if (sign_present)
+	line_attr = sattr.linehl;
 # endif
 # if defined(FEAT_QUICKFIX)
     /* Highlight the current line in the quickfix window. */
@@ -3974,8 +3980,8 @@ win_line(
 		/* Show the sign column when there are any signs in this
 		 * buffer or when using Netbeans. */
 		if (signcolumn_on(wp))
-		    get_sign_display_info(FALSE, wp, lnum, wcr_attr, row,
-			    startrow, filler_lines, filler_todo, &c_extra,
+		    get_sign_display_info(FALSE, wp, lnum, &sattr, wcr_attr,
+			    row, startrow, filler_lines, filler_todo, &c_extra,
 			    &c_final, extra, &p_extra, &n_extra, &char_attr);
 	    }
 #endif
@@ -3997,11 +4003,10 @@ win_line(
 		    // 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)
-			get_sign_display_info(TRUE, wp, lnum, wcr_attr, row,
-				startrow, filler_lines, filler_todo, &c_extra,
-				&c_final, extra, &p_extra, &n_extra,
+			    && sign_present)
+			get_sign_display_info(TRUE, wp, lnum, &sattr, wcr_attr,
+				row, startrow, filler_lines, filler_todo,
+				&c_extra, &c_final, extra, &p_extra, &n_extra,
 				&char_attr);
 		    else
 #endif

src/sign.c

@@ -273,6 +273,20 @@ insert_sign_by_lnum_prio(
     insert_sign(buf, prev, sign, id, group, prio, lnum, typenr);
 }
 
+/*
+ * 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;
+}
+
 /*
  * Get the name of a sign by its typenr.
  */
@@ -445,31 +459,44 @@ buf_change_sign_type(
 }
 
 /*
- * 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.
+ * Return the attributes of the first sign placed on line 'lnum' in buffer
+ * 'buf'. Used when refreshing the screen. Returns TRUE if a sign is found on
+ * 'lnum', FALSE otherwise.
  */
     int
-buf_getsigntype(
-    buf_T	*buf,
-    linenr_T	lnum,
-    int		type)	// SIGN_ICON, SIGN_TEXT, SIGN_ANY, SIGN_LINEHL
+buf_get_signattrs(buf_T *buf, linenr_T lnum, sign_attrs_T *sattr)
 {
-    signlist_T	*sign;		// a sign in a b_signlist
+    signlist_T	*sign;
+    sign_T	*sp;
+
+    vim_memset(sattr, 0, sizeof(sign_attrs_T));
 
     FOR_ALL_SIGNS_IN_BUF(buf, sign)
-	if (sign->lnum == lnum
-		&& (type == SIGN_ANY
+    {
+	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)
+	{
+	    sattr->typenr = sign->typenr;
+	    sp = find_sign_by_typenr(sign->typenr);
+	    if (sp == NULL)
+		return FALSE;
+
 # ifdef FEAT_SIGN_ICONS
-		    || (type == SIGN_ICON
-			&& sign_get_image(sign->typenr) != NULL)
+	    sattr->icon = sp->sn_image;
 # endif
-		    || (type == SIGN_TEXT
-			&& sign_get_text(sign->typenr) != NULL)
-		    || (type == SIGN_LINEHL
-			&& sign_get_attr(sign->typenr, TRUE) != 0)))
-	    return sign->typenr;
-    return 0;
+	    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);
+	    return TRUE;
+	}
+    }
+    return FALSE;
 }
 
 /*
@@ -571,8 +598,15 @@ 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;
+    }
 
     return NULL;
 }
@@ -608,8 +642,15 @@ buf_findsigntype_id(
     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->typenr == typenr)
 	    return sign->id;
+    }
 
     return 0;
 }
@@ -626,9 +667,16 @@ buf_signcount(buf_T *buf, linenr_T lnum)
     int		count = 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)
 	    if (sign_get_image(sign->typenr) != NULL)
 		count++;
+    }
 
     return count;
 }
@@ -1792,48 +1840,6 @@ sign_undefine(sign_T *sp, sign_T *sp_prev)
     vim_free(sp);
 }
 
-/*
- * Get highlighting attribute for sign "typenr".
- * If "line" is TRUE: line highl, if FALSE: text highl.
- */
-    int
-sign_get_attr(int typenr, int line)
-{
-    sign_T	*sp;
-
-    for (sp = first_sign; sp != NULL; sp = sp->sn_next)
-	if (sp->sn_typenr == typenr)
-	{
-	    if (line)
-	    {
-		if (sp->sn_line_hl > 0)
-		    return syn_id2attr(sp->sn_line_hl);
-	    }
-	    else
-	    {
-		if (sp->sn_text_hl > 0)
-		    return syn_id2attr(sp->sn_text_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;
-}
-
 # if defined(FEAT_SIGN_ICONS) || defined(PROTO)
     void *
 sign_get_image(

src/structs.h

@@ -759,6 +759,17 @@ 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;
+    void	*icon;
+    char_u	*text;
+    int		texthl;
+    int		linehl;
+} sign_attrs_T;
+
 #if defined(FEAT_SIGNS) || defined(PROTO)
 // Macros to get the sign group structure from the group name
 #define SGN_KEY_OFF	offsetof(signgroup_T, sg_name)
@@ -767,11 +778,6 @@ struct signlist
 // Default sign priority for highlighting
 #define SIGN_DEF_PRIO	10
 
-/* type argument for buf_getsigntype() */
-#define SIGN_ANY	0
-#define SIGN_LINEHL	1
-#define SIGN_ICON	2
-#define SIGN_TEXT	3
 #endif
 
 /*

src/version.c

@@ -777,6 +777,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1631,
 /**/
     1630,
 /**/