patch 8.1.0632: using sign group names is inefficient

Problem:    Using sign group names is inefficient.
Solution:   Store group names in a hash table and use a reference to them.
            Also remove unnecessary use of ":exe" from the tests.  (Yegappan
            Lakshmanan, closes #3715)

src/buffer.c

@@ -5865,6 +5865,73 @@ find_win_for_buf(
 #endif
 
 #if defined(FEAT_SIGNS) || defined(PROTO)
+static hashtab_T	sg_table;	// sign group (signgroup_T) hashtable
+
+/*
+ * A new sign in group 'groupname' is added. If the group is not present,
+ * create it. Otherwise reference the group.
+ */
+    static signgroup_T *
+sign_group_ref(char_u *groupname)
+{
+    static int		initialized = FALSE;
+    hash_T		hash;
+    hashitem_T		*hi;
+    signgroup_T		*group;
+
+    if (!initialized)
+    {
+	initialized = TRUE;
+	hash_init(&sg_table);
+    }
+
+    hash = hash_hash(groupname);
+    hi = hash_lookup(&sg_table, groupname, hash);
+    if (HASHITEM_EMPTY(hi))
+    {
+	// new group
+	group = (signgroup_T *)alloc(
+		(unsigned)(sizeof(signgroup_T) + STRLEN(groupname)));
+	if (group == NULL)
+	    return NULL;
+	STRCPY(group->sg_name, groupname);
+	group->refcount = 1;
+	hash_add_item(&sg_table, hi, group->sg_name, hash);
+    }
+    else
+    {
+	// existing group
+	group = HI2SG(hi);
+	group->refcount++;
+    }
+
+    return group;
+}
+
+/*
+ * A sign in group 'groupname' is removed. If all the signs in this group are
+ * removed, then remove the group.
+ */
+    static void
+sign_group_unref(char_u *groupname)
+{
+    hashitem_T		*hi;
+    signgroup_T		*group;
+
+    hi = hash_find(&sg_table, groupname);
+    if (!HASHITEM_EMPTY(hi))
+    {
+	group = HI2SG(hi);
+	group->refcount--;
+	if (group->refcount == 0)
+	{
+	    // All the signs in this group are removed
+	    hash_remove(&sg_table, hi);
+	    vim_free(group);
+	}
+    }
+}
+
 /*
  * Insert a new sign into the signlist for buffer 'buf' between the 'prev' and
  * 'next' signs.
@@ -5890,7 +5957,14 @@ insert_sign(
 	newsign->lnum = lnum;
 	newsign->typenr = typenr;
 	if (group != NULL)
-	    newsign->group = vim_strsave(group);
+	{
+	    newsign->group = sign_group_ref(group);
+	    if (newsign->group == NULL)
+	    {
+		vim_free(newsign);
+		return;
+	    }
+	}
 	else
 	    newsign->group = NULL;
 	newsign->priority = prio;
@@ -5959,7 +6033,7 @@ sign_in_group(signlist_T *sign, char_u *group)
     return ((group != NULL && STRCMP(group, "*") == 0) ||
 	    (group == NULL && sign->group == NULL) ||
 	    (group != NULL && sign->group != NULL &&
-					STRCMP(group, sign->group) == 0));
+				STRCMP(group, sign->group->sg_name) == 0));
 }
 
 /*
@@ -5974,7 +6048,7 @@ sign_get_info(signlist_T *sign)
 	return NULL;
     dict_add_number(d, "id", sign->id);
     dict_add_string(d, "group", (sign->group == NULL) ?
-						(char_u *)"" : sign->group);
+					(char_u *)"" : sign->group->sg_name);
     dict_add_number(d, "lnum", sign->lnum);
     dict_add_string(d, "name", sign_typenr2name(sign->typenr));
     dict_add_number(d, "priority", sign->priority);
@@ -5989,7 +6063,7 @@ sign_get_info(signlist_T *sign)
 buf_addsign(
     buf_T	*buf,		// buffer to store sign in
     int		id,		// sign ID
-    char_u	*group,		// sign group
+    char_u	*groupname,	// sign group
     int		prio,		// sign priority
     linenr_T	lnum,		// line number which gets the mark
     int		typenr)		// typenr of sign we are adding
@@ -6001,22 +6075,22 @@ buf_addsign(
     FOR_ALL_SIGNS_IN_BUF(buf)
     {
 	if (lnum == sign->lnum && id == sign->id &&
-		sign_in_group(sign, group))
+		sign_in_group(sign, groupname))
 	{
 	    // Update an existing sign
 	    sign->typenr = typenr;
 	    return;
 	}
 	else if (lnum < sign->lnum)
 	{
-	    insert_sign_by_lnum_prio(buf, prev, id, group, prio, lnum, typenr);
+	    insert_sign_by_lnum_prio(buf, prev, id, groupname, prio,
+								lnum, typenr);
 	    return;
 	}
 	prev = sign;
     }
 
-    insert_sign_by_lnum_prio(buf, prev, id, group, prio, lnum, typenr);
-
+    insert_sign_by_lnum_prio(buf, prev, id, groupname, prio, lnum, typenr);
     return;
 }
 
@@ -6106,7 +6180,8 @@ buf_delsign(
 	    if (next != NULL)
 		next->prev = sign->prev;
 	    lnum = sign->lnum;
-	    vim_free(sign->group);
+	    if (sign->group != NULL)
+		sign_group_unref(sign->group->sg_name);
 	    vim_free(sign);
 	    // Check whether only one sign needs to be deleted
 	    if (group == NULL || (*group != '*' && id != 0))
@@ -6269,7 +6344,8 @@ buf_delete_signs(buf_T *buf, char_u *group)
 	    *lastp = next;
 	    if (next != NULL)
 		next->prev = sign->prev;
-	    vim_free(sign->group);
+	    if (sign->group != NULL)
+		sign_group_unref(sign->group->sg_name);
 	    vim_free(sign);
 	}
 	else
@@ -6317,10 +6393,13 @@ sign_list_placed(buf_T *rbuf, char_u *sign_group)
 	}
 	FOR_ALL_SIGNS_IN_BUF(buf)
 	{
+	    if (got_int)
+		break;
 	    if (!sign_in_group(sign, sign_group))
 		continue;
 	    if (sign->group != NULL)
-		vim_snprintf(group, BUFSIZ, "  group=%s", sign->group);
+		vim_snprintf(group, BUFSIZ, "  group=%s",
+							sign->group->sg_name);
 	    else
 		group[0] = '\0';
 	    vim_snprintf(lbuf, BUFSIZ, _("    line=%ld  id=%d%s  name=%s "

src/ex_cmds.c

@@ -8180,7 +8180,7 @@ ex_sign(exarg_T *eap)
 	{
 	    EMSG2(_("E158: Invalid buffer name: %s"), arg);
 	}
-	else if (id <= 0 && !(idx == SIGNCMD_UNPLACE && id == -2))
+	else if (id <= 0 && idx == SIGNCMD_PLACE)
 	{
 	    if ((group == NULL) && (lnum >= 0 || sign_name != NULL))
 		EMSG(_(e_invarg));

src/structs.h

@@ -733,14 +733,25 @@ typedef struct proptype_S
 
 
 #if defined(FEAT_SIGNS) || defined(PROTO)
+// Sign group
+typedef struct signgroup_S
+{
+    short_u	refcount;		// number of signs in this group
+    char_u	sg_name[1];		// sign group name
+} signgroup_T;
+
+// Macros to get the sign group structure from the group name
+#define SGN_KEY_OFF	offsetof(signgroup_T, sg_name)
+#define HI2SG(hi)	((signgroup_T *)((hi)->hi_key - SGN_KEY_OFF))
+
 typedef struct signlist signlist_T;
 
 struct signlist
 {
     int		id;		/* unique identifier for each placed sign */
     linenr_T	lnum;		/* line number which has this sign */
     int		typenr;		/* typenr of sign */
-    char_u	*group;		/* sign group */
+    signgroup_T	*group;		/* sign group */
     int		priority;	/* priority for highlighting */
     signlist_T	*next;		/* next signlist entry */
     signlist_T  *prev;		/* previous entry -- for easy reordering */