{"thread":{"id":"9833","subject":"[PATCH] diff-delta.c: pack the index structure","startedAt":"2007-09-08T21:17:44Z","lastAt":"2007-09-08T21:25:55Z","messageCount":2,"participants":["David Kastrup"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"53009","messageId":"851wd8ua9z.fsf@lola.goethe.zz","threadId":"9833","inReplyTo":null,"subject":"[PATCH] diff-delta.c: pack the index structure","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-08T21:17:44Z","receivedAt":"2007-09-08T21:17:44Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"In normal use cases, the performance wins are not overly impressive:\nwe get something like 5-10% due to the slightly better locality of\nmemory accesses using the packed structure.\n\nHowever, since the data structure for index entries saves 33% of\nmemory on 32-bit platforms and 40% on 64-bit platforms, the behavior\nwhen memory gets limited should be nicer.\n\nThis is a rather well-contained change.  One obvious improvement would\nbe sorting the elements in one bucket according to their hash, then\nusing binary probing to find the elements with the right hash value.\n\nAs it stands, the output should be strictly the same as previously\nunless one uses the option for limiting the amount of used memory, in\nwhich case the created packs might be better.\n\nSigned-off-by: David Kastrup <dak@gnu.org>\nAcked-by: Nicolas Pitre <nico@cam.org>\n---\n diff-delta.c |   74 +++++++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 58 insertions(+), 16 deletions(-)\n\ndiff --git a/diff-delta.c b/diff-delta.c\nindex 0dde2f2..d355e5e 100644\n--- a/diff-delta.c\n+++ b/diff-delta.c\n@@ -115,9 +115,13 @@ static const unsigned int U[256] = {\n struct index_entry {\n \tconst unsigned char *ptr;\n \tunsigned int val;\n-\tstruct index_entry *next;\n };\n \n+struct unpacked_index_entry {\n+\tstruct index_entry entry;\n+\tstruct unpacked_index_entry *next;\n+};\t\n+\n struct delta_index {\n \tunsigned long memsize;\n \tconst void *src_buf;\n@@ -131,7 +135,8 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)\n \tunsigned int i, hsize, hmask, entries, prev_val, *hash_count;\n \tconst unsigned char *data, *buffer = buf;\n \tstruct delta_index *index;\n-\tstruct index_entry *entry, **hash;\n+\tstruct unpacked_index_entry *entry, **hash;\n+\tstruct index_entry *packed_entry, **packed_hash;\n \tvoid *mem;\n \tunsigned long memsize;\n \n@@ -148,28 +153,21 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)\n \thmask = hsize - 1;\n \n \t/* allocate lookup index */\n-\tmemsize = sizeof(*index) +\n-\t\t  sizeof(*hash) * hsize +\n+\tmemsize = sizeof(*hash) * hsize +\n \t\t  sizeof(*entry) * entries;\n \tmem = malloc(memsize);\n \tif (!mem)\n \t\treturn NULL;\n-\tindex = mem;\n-\tmem = index + 1;\n \thash = mem;\n \tmem = hash + hsize;\n \tentry = mem;\n \n-\tindex->memsize = memsize;\n-\tindex->src_buf = buf;\n-\tindex->src_size = bufsize;\n-\tindex->hash_mask = hmask;\n \tmemset(hash, 0, hsize * sizeof(*hash));\n \n \t/* allocate an array to count hash entries */\n \thash_count = calloc(hsize, sizeof(*hash_count));\n \tif (!hash_count) {\n-\t\tfree(index);\n+\t\tfree(hash);\n \t\treturn NULL;\n \t}\n \n@@ -183,12 +181,13 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)\n \t\t\tval = ((val << 8) | data[i]) ^ T[val >> RABIN_SHIFT];\n \t\tif (val == prev_val) {\n \t\t\t/* keep the lowest of consecutive identical blocks */\n-\t\t\tentry[-1].ptr = data + RABIN_WINDOW;\n+\t\t\tentry[-1].entry.ptr = data + RABIN_WINDOW;\n+\t\t\t--entries;\n \t\t} else {\n \t\t\tprev_val = val;\n \t\t\ti = val & hmask;\n-\t\t\tentry->ptr = data + RABIN_WINDOW;\n-\t\t\tentry->val = val;\n+\t\t\tentry->entry.ptr = data + RABIN_WINDOW;\n+\t\t\tentry->entry.val = val;\n \t\t\tentry->next = hash[i];\n \t\t\thash[i] = entry++;\n \t\t\thash_count[i]++;\n@@ -212,16 +211,59 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)\n \t\t\tcontinue;\n \t\tentry = hash[i];\n \t\tdo {\n-\t\t\tstruct index_entry *keep = entry;\n+\t\t\tstruct unpacked_index_entry *keep = entry;\n \t\t\tint skip = hash_count[i] / HASH_LIMIT;\n \t\t\tdo {\n+\t\t\t\t--entries;\n \t\t\t\tentry = entry->next;\n \t\t\t} while(--skip && entry);\n+\t\t\t++entries;\n \t\t\tkeep->next = entry;\n \t\t} while(entry);\n \t}\n \tfree(hash_count);\n \n+\t/* Now create the packed index in array form rather than\n+\t * linked lists */\n+\n+\tmemsize = sizeof(*index)\n+\t\t+ sizeof(*packed_hash) * (hsize+1)\n+\t\t+ sizeof(*packed_entry) * entries;\n+\n+\tmem = malloc(memsize);\n+\n+\tif (!mem) {\n+\t\tfree(hash);\n+\t\treturn NULL;\n+\t}\n+\n+\tindex = mem;\n+\tindex->memsize = memsize;\n+\tindex->src_buf = buf;\n+\tindex->src_size = bufsize;\n+\tindex->hash_mask = hmask;\n+\n+\tmem = index + 1;\n+\tpacked_hash = mem;\n+\tmem = packed_hash + (hsize+1);\n+\tpacked_entry = mem;\n+\n+\t/* Coalesce all entries belonging to one linked list into\n+\t * consecutive array entries */\n+\n+\tfor (i = 0; i < hsize; i++) {\n+\t\tpacked_hash[i] = packed_entry;\n+\t\tfor (entry = hash[i]; entry; entry = entry->next)\n+\t\t\t*packed_entry++ = entry->entry;\n+\t}\n+\n+\t/* Sentinel value to indicate the length of the last hash\n+\t * bucket */\n+\n+\tpacked_hash[hsize] = packed_entry;\n+\tassert(packed_entry - (struct index_entry *)mem == entries);\n+\tfree(hash);\n+\n \treturn index;\n }\n \n@@ -302,7 +344,7 @@ create_delta(const struct delta_index *index,\n \t\t\tval ^= U[data[-RABIN_WINDOW]];\n \t\t\tval = ((val << 8) | *data) ^ T[val >> RABIN_SHIFT];\n \t\t\ti = val & index->hash_mask;\n-\t\t\tfor (entry = index->hash[i]; entry; entry = entry->next) {\n+\t\t\tfor (entry = index->hash[i]; entry < index->hash[i+1]; entry++) {\n \t\t\t\tconst unsigned char *ref = entry->ptr;\n \t\t\t\tconst unsigned char *src = data;\n \t\t\t\tunsigned int ref_size = ref_top - ref;\n-- \n1.5.3.GIT\n"},{"id":"53010","messageId":"85r6l8svbw.fsf@lola.goethe.zz","threadId":"9833","inReplyTo":"851wd8ua9z.fsf@lola.goethe.zz","subject":"[PATCH] diff-delta.c: Rationalize culling of hash buckets","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-08T21:25:55Z","receivedAt":"2007-09-08T21:25:55Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"The previous hash bucket culling resulted in a somewhat unpredictable\nnumber of hash bucket entries in the order of magnitude of HASH_LIMIT.\n\nReplace this with a Bresenham-like algorithm leaving us with exactly\nHASH_LIMIT entries by uniform culling.\n\nSigned-off-by: David Kastrup <dak@gnu.org>\n---\nThis is to be applied on top of the patch for packing the index\nstructure.  Which is the reason it is posted as a followup to the\nrespective article.  In contrast to that patch, this patch has only\nbeen tested/reviewed by myself as to yet.\n\n diff-delta.c |   41 +++++++++++++++++++++++++++++++----------\n 1 files changed, 31 insertions(+), 10 deletions(-)\n\ndiff --git a/diff-delta.c b/diff-delta.c\nindex d355e5e..1a8d6fb 100644\n--- a/diff-delta.c\n+++ b/diff-delta.c\n@@ -207,19 +207,40 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)\n \t * the reference buffer.\n \t */\n \tfor (i = 0; i < hsize; i++) {\n-\t\tif (hash_count[i] < HASH_LIMIT)\n+\t\tint acc;\n+\n+\t\tif (hash_count[i] <= HASH_LIMIT)\n \t\t\tcontinue;\n+\n+\t\tentries -= hash_count[i] - HASH_LIMIT;\n+\t\t/* We leave exactly HASH_LIMIT entries in the bucket */\n+\n \t\tentry = hash[i];\n+\t\tacc = 0;\n \t\tdo {\n-\t\t\tstruct unpacked_index_entry *keep = entry;\n-\t\t\tint skip = hash_count[i] / HASH_LIMIT;\n-\t\t\tdo {\n-\t\t\t\t--entries;\n-\t\t\t\tentry = entry->next;\n-\t\t\t} while(--skip && entry);\n-\t\t\t++entries;\n-\t\t\tkeep->next = entry;\n-\t\t} while(entry);\n+\t\t\tacc += hash_count[i] - HASH_LIMIT;\n+\t\t\tif (acc > 0) {\n+\t\t\t\tstruct unpacked_index_entry *keep = entry;\n+\t\t\t\tdo {\n+\t\t\t\t\tentry = entry->next;\n+\t\t\t\t\tacc -= HASH_LIMIT;\n+\t\t\t\t} while (acc > 0);\n+\t\t\t\tkeep->next = entry->next;\n+\t\t\t}\n+\t\t\tentry = entry->next;\n+\t\t} while (entry);\n+\n+\t\t/* Assume that this loop is gone through exactly\n+\t\t * HASH_LIMIT times and is entered and left with\n+\t\t * acc==0.  So the first statement in the loop\n+\t\t * contributes (hash_count[i]-HASH_LIMIT)*HASH_LIMIT\n+\t\t * to the accumulator, and the inner loop consequently\n+\t\t * is run (hash_count[i]-HASH_LIMIT) times, removing\n+\t\t * one element from the list each time.  Since acc\n+\t\t * balances out to 0 at the final run, the inner loop\n+\t\t * body can't be left with entry==NULL.  So we indeed\n+\t\t * encounter entry==NULL in the outer loop only.\n+\t\t */\n \t}\n \tfree(hash_count);\n \n-- \n1.5.3.GIT\n"}]}