ProcRenderAddGlyphs: Avoid allocating a glyph just to find it cached

This is a cleanup without any real savings (yet). Previously, the
implementation would allocate a new glyph, then (often) find it in
the cache, and immediately discard the allocated object. This
re-organization first uses a new FindGlyphByHash function and only
allocates the glyph if nothing is found.

This isn't a real savings yet, since FindGlyphByHash currently still
does a temporary glyph allocation, but this is expected to be replaced
immediately as we switch to an alternate hashing mechanism (SHA1).

3 files changed, 71 insertions, 25 deletions

diff --git a/render/glyph.c b/render/glyph.c
index 53c00b326..1204c3b79 100644
--- a/render/glyph.c
+++ b/render/glyph.c
@@ -490,6 +490,31 @@ HashGlyph (GlyphPtr glyph)
 				 glyph->size - sizeof (xGlyphInfo));
 }
 
+GlyphPtr
+FindGlyphByHash (CARD32	    hash,
+		 xGlyphInfo *gi,
+		 CARD8	    *bits,
+		 int	    format)
+{
+    GlyphRefPtr gr;
+    GlyphPtr template;
+
+    /* XXX: Should handle out-of-memory here */
+    template = AllocateGlyph (gi, format);
+    memcpy ((CARD8 *) (template + 1), bits,
+	    template->size - sizeof (xGlyphInfo));
+
+    gr = FindGlyphRef (&globalGlyphs[format],
+		       hash, TRUE, template);
+
+    xfree (template);
+
+    if (gr->glyph && gr->glyph != DeletedGlyph)
+	return gr->glyph;
+    else
+	return NULL;
+}
+
 #ifdef CHECK_DUPLICATES
 void
 DuplicateRef (GlyphPtr glyph, char *where)
@@ -572,7 +597,7 @@ AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id)
     /* Locate existing matching glyph */
     hash = HashGlyph (glyph);
     gr = FindGlyphRef (&globalGlyphs[glyphSet->fdepth], hash, TRUE, glyph);
-    if (gr->glyph && gr->glyph != DeletedGlyph)
+    if (gr->glyph && gr->glyph != DeletedGlyph && gr->glyph != glyph)
     {
 	PictureScreenPtr ps;
 	int              i;
@@ -588,7 +613,7 @@ AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id)
 	xfree (glyph);
 	glyph = gr->glyph;
     }
-    else
+    else if (gr->glyph != glyph)
     {
 	gr->glyph = glyph;
 	gr->signature = hash;
diff --git a/render/glyphstr.h b/render/glyphstr.h
index b941dabaf..37462f744 100644
--- a/render/glyphstr.h
+++ b/render/glyphstr.h
@@ -129,6 +129,12 @@ FindGlyphHashSet (CARD32 filled);
 GlyphRefPtr
 FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare);
 
+GlyphPtr
+FindGlyphByHash (CARD32	    hash,
+		 xGlyphInfo *gi,
+		 CARD8	    *bits,
+		 int	    format);
+
 CARD32
 HashGlyphInfoAndBits (xGlyphInfo *gi, CARD8 *data, unsigned int size);
 
diff --git a/render/render.c b/render/render.c
index 10a5b805c..831c98417 100644
--- a/render/render.c
+++ b/render/render.c
@@ -1082,6 +1082,8 @@ ProcRenderFreeGlyphSet (ClientPtr client)
 typedef struct _GlyphNew {
     Glyph	id;
     GlyphPtr    glyph;
+    Bool	found;
+    CARD32	hash;
 } GlyphNewRec, *GlyphNewPtr;
 
 static int
@@ -1090,8 +1092,7 @@ ProcRenderAddGlyphs (ClientPtr client)
     GlyphSetPtr     glyphSet;
     REQUEST(xRenderAddGlyphsReq);
     GlyphNewRec	    glyphsLocal[NLOCALGLYPH];
-    GlyphNewPtr	    glyphsBase, glyphs;
-    GlyphPtr	    glyph;
+    GlyphNewPtr	    glyphsBase, glyphs, glyph_new;
     int		    remain, nglyphs;
     CARD32	    *gids;
     xGlyphInfo	    *gi;
@@ -1115,11 +1116,13 @@ ProcRenderAddGlyphs (ClientPtr client)
     if (nglyphs > UINT32_MAX / sizeof(GlyphNewRec))
 	    return BadAlloc;
 
-    if (nglyphs <= NLOCALGLYPH)
+    if (nglyphs <= NLOCALGLYPH) {
+	memset (glyphsLocal, 0, sizeof (glyphsLocal));
 	glyphsBase = glyphsLocal;
+    }
     else
     {
-	glyphsBase = (GlyphNewPtr) Xalloc (nglyphs * sizeof (GlyphNewRec));
+	glyphsBase = (GlyphNewPtr) Xcalloc (nglyphs * sizeof (GlyphNewRec));
 	if (!glyphsBase)
 	    return BadAlloc;
     }
@@ -1134,26 +1137,41 @@ ProcRenderAddGlyphs (ClientPtr client)
     remain -= (sizeof (CARD32) + sizeof (xGlyphInfo)) * nglyphs;
     for (i = 0; i < nglyphs; i++)
     {
-	glyph = AllocateGlyph (&gi[i], glyphSet->fdepth);
-	if (!glyph)
-	{
-	    err = BadAlloc;
-	    goto bail;
-	}
-	
-	glyphs->glyph = glyph;
-	glyphs->id = gids[i];	
-	
-	size = glyph->size - sizeof (xGlyphInfo);
+	glyph_new = &glyphs[i];
+	size = gi[i].height * PixmapBytePad (gi[i].width,
+					     glyphSet->format->depth);
 	if (remain < size)
 	    break;
-	memcpy ((CARD8 *) (glyph + 1), bits, size);
+
+	glyph_new->hash = HashGlyphInfoAndBits (&gi[i], bits, size);
+
+	glyph_new->glyph = FindGlyphByHash (glyph_new->hash,
+					    &gi[i], bits,
+					    glyphSet->fdepth);
+
+	if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph)
+	{
+	    glyph_new->found = TRUE;
+	}
+	else
+	{
+	    glyph_new->found = FALSE;
+	    glyph_new->glyph = AllocateGlyph (&gi[i], glyphSet->fdepth);
+	    if (! glyph_new->glyph)
+	    {
+		err = BadAlloc;
+		goto bail;
+	    }
+
+	    memcpy ((CARD8 *) (glyph_new->glyph + 1), bits, size);
+	}
+
+	glyph_new->id = gids[i];
 	
 	if (size & 3)
 	    size += 4 - (size & 3);
 	bits += size;
 	remain -= size;
-	glyphs++;
     }
     if (remain || i < nglyphs)
     {
@@ -1165,7 +1183,6 @@ ProcRenderAddGlyphs (ClientPtr client)
 	err = BadAlloc;
 	goto bail;
     }
-    glyphs = glyphsBase;
     for (i = 0; i < nglyphs; i++)
 	AddGlyph (glyphSet, glyphs[i].glyph, glyphs[i].id);
 
@@ -1173,11 +1190,9 @@ ProcRenderAddGlyphs (ClientPtr client)
 	Xfree (glyphsBase);
     return client->noClientException;
 bail:
-    while (glyphs != glyphsBase)
-    {
-	--glyphs;
-	xfree (glyphs->glyph);
-    }
+    for (i = 0; i < nglyphs; i++)
+	if (glyphs[i].glyph && ! glyphs[i].found)
+	    xfree (glyphs[i].glyph);
     if (glyphsBase != glyphsLocal)
 	Xfree (glyphsBase);
     return err;