git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] sha1write: don't copy full sized buffers

From
Nicolas Pitre <nico@cam.org>
Date
Sep 2, 2008, 14:22 UTC
Message-ID
<1220365342-21308-1-git-send-email-nico@cam.org>

No need to memcpy() source buffer data when we might just process the data in place instead of accumulating it into a separate buffer. This is the case when a whole buffer would have been copied, summed, written out and then discarded right away.

Also move the CRC32 processing within the loop so the data is more likely to remain in the L1 CPU cache between the CRC32 sum, SHA1 sum and the write call.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 csum-file.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/csum-file.c b/csum-file.c
index 2838954..bb70c75 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -11,10 +11,8 @@
 #include "progress.h"
 #include "csum-file.h"
 
-static void sha1flush(struct sha1file *f, unsigned int count)
+static void sha1flush(struct sha1file *f, void *buf, unsigned int count)
 {
-	void *buf = f->buffer;
-
 	for (;;) {
 		int ret = xwrite(f->fd, buf, count);
 		if (ret > 0) {
@@ -39,7 +37,7 @@ int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
 
 	if (offset) {
 		SHA1_Update(&f->ctx, f->buffer, offset);
-		sha1flush(f, offset);
+		sha1flush(f, f->buffer, offset);
 		f->offset = 0;
 	}
 	SHA1_Final(f->buffer, &f->ctx);
@@ -47,7 +45,7 @@ int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
 		hashcpy(result, f->buffer);
 	if (flags & (CSUM_CLOSE | CSUM_FSYNC)) {
 		/* write checksum and close fd */
-		sha1flush(f, 20);
+		sha1flush(f, f->buffer, 20);
 		if (flags & CSUM_FSYNC)
 			fsync_or_die(f->fd, f->name);
 		if (close(f->fd))
@@ -62,21 +60,30 @@ int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
 
 int sha1write(struct sha1file *f, void *buf, unsigned int count)
 {
-	if (f->do_crc)
-		f->crc32 = crc32(f->crc32, buf, count);
 	while (count) {
 		unsigned offset = f->offset;
 		unsigned left = sizeof(f->buffer) - offset;
 		unsigned nr = count > left ? left : count;
+		void *data;
+
+		if (f->do_crc)
+			f->crc32 = crc32(f->crc32, buf, nr);
+
+		if (nr == sizeof(f->buffer)) {
+			/* process full buffer directly without copy */
+			data = buf;
+		} else {
+			memcpy(f->buffer + offset, buf, nr);
+			data = f->buffer;
+		}
 
-		memcpy(f->buffer + offset, buf, nr);
 		count -= nr;
 		offset += nr;
 		buf = (char *) buf + nr;
 		left -= nr;
 		if (!left) {
-			SHA1_Update(&f->ctx, f->buffer, offset);
-			sha1flush(f, offset);
+			SHA1_Update(&f->ctx, data, offset);
+			sha1flush(f, data, offset);
 			offset = 0;
 		}
 		f->offset = offset;
-- 
1.6.0.1.276.g59b81
Next: Nicolas Pitre
Message 1 of 3 in “sha1write: don't copy full sized buffers”
  1. sha1write: don't copy full sized buffersNicolas Pitre, Sep 2, 2008
  2. pack-objects: don't include missing preferred base objectsNicolas Pitre, Sep 2, 2008
  3. t5300: improve SHA1 collision testNicolas Pitre, Sep 2, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.