{"thread":{"id":"28426","subject":"[PATCH 0/8] fast-import: cache oe more often","startedAt":"2011-09-19T01:27:29Z","lastAt":"2011-09-20T14:39:41Z","messageCount":13,"participants":["Dmitry Ivankov","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"175763","messageId":"1316395657-6991-1-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":null,"subject":"[PATCH 0/8] fast-import: cache oe more often","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:29Z","receivedAt":"2011-09-19T01:27:29Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-import keeps a struct object_entry for each object written to\nit's pack. This is to keep type, pack-coordinates and delta_depth.\nstruct object_entry is also used to cache this metadata for objects\nthat exist outside fast-import's pack ('old' objects).\nstruct object_entry has a small fixed size and thus it should be\nreasonable to cache any 'old' object metadata retrieval to save the\ndisk i/o.\n\nAlso it is a step toward making fast-import identify objects via\nstruct object_entry rather than sha1. One pointer takes less than\n20 bytes, it'll be later possible to have references to objects\nthat don't yet have sha1 computed (fast-import with threads future).\n\nDmitry Ivankov (8):\n fast-import: cache oe in file_change_m\n fast-import: cache oe in parse_new_tag\n fast-import: cache oe in note_change_n\n fast-import: extract common sha1_file access functions\n fast-import: tiny optimization in read_marks\n fast-import: cache oe in load_tree\n fast-import: cache oe in cat_blob\n fast-import: cache objects while dereferencing\n\n fast-import.c | 177 +++++++++++++++++++++++++++++++--------------------------\n 1 files changed, 96 insertions(+), 81 deletions(-)\n\n-- \n1.7.3.4\n"},{"id":"175765","messageId":"1316395657-6991-2-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 1/8] fast-import: cache oe in file_change_m","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:30Z","receivedAt":"2011-09-19T01:27:30Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"file_change_m checks object type for objects specified by sha1. It does\nso via sha1_object_info but doesn't cache this information in struct\nobject_entry.\n\nMake this call to sha1_object_info cached in struct object_entry.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 22 ++++++++++++++--------\n 1 files changed, 14 insertions(+), 8 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 742e7da..42f9b17 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2297,15 +2297,21 @@ static void file_change_m(struct branch *b)\n \t} else {\n \t\tenum object_type expected = S_ISDIR(mode) ?\n \t\t\t\t\t\tOBJ_TREE: OBJ_BLOB;\n-\t\tenum object_type type = oe ? oe->type :\n-\t\t\t\t\tsha1_object_info(sha1, NULL);\n-\t\tif (type < 0)\n-\t\t\tdie(\"%s not found: %s\",\n-\t\t\t\t\tS_ISDIR(mode) ? \"Tree\" : \"Blob\",\n-\t\t\t\t\tcommand_buf.buf);\n-\t\tif (type != expected)\n+\t\tif (!oe)\n+\t\t\toe = insert_object(sha1);\n+\t\tif (!oe->idx.offset) {\n+\t\t\tenum object_type type = sha1_object_info(oe->idx.sha1, NULL);\n+\t\t\tif (type < 0)\n+\t\t\t\tdie(\"%s not found: %s\",\n+\t\t\t\t\t\tS_ISDIR(mode) ? \"Tree\" : \"Blob\",\n+\t\t\t\t\t\tcommand_buf.buf);\n+\t\t\toe->type = type;\n+\t\t\toe->pack_id = MAX_PACK_ID;\n+\t\t\toe->idx.offset = 1; /* nonzero */\n+\t\t}\n+\t\tif (oe->type != expected)\n \t\t\tdie(\"Not a %s (actually a %s): %s\",\n-\t\t\t\ttypename(expected), typename(type),\n+\t\t\t\ttypename(expected), typename(oe->type),\n \t\t\t\tcommand_buf.buf);\n \t}\n \n-- \n1.7.3.4\n"},{"id":"175764","messageId":"1316395657-6991-3-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 2/8] fast-import: cache oe in parse_new_tag","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:31Z","receivedAt":"2011-09-19T01:27:31Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"parse_new_tag uses sha1_object_info to find out the type of an object\ngiven by a sha1 expression. But doesn't cache it in struct object_entry.\n\nMake this call to sha1_object_info cached in struct object_entry.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 7 +++++--\n 1 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 42f9b17..2b049f7 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2732,11 +2732,14 @@ static void parse_new_tag(void)\n \t\ttype = oe->type;\n \t\thashcpy(sha1, oe->idx.sha1);\n \t} else if (!get_sha1(from, sha1)) {\n-\t\tstruct object_entry *oe = find_object(sha1);\n-\t\tif (!oe) {\n+\t\tstruct object_entry *oe = insert_object(sha1);\n+\t\tif (!oe->idx.offset) {\n \t\t\ttype = sha1_object_info(sha1, NULL);\n \t\t\tif (type < 0)\n \t\t\t\tdie(\"Not a valid object: %s\", from);\n+\t\t\toe->type = type;\n+\t\t\toe->pack_id = MAX_PACK_ID;\n+\t\t\toe->idx.offset = 1; /* nonzero */\n \t\t} else\n \t\t\ttype = oe->type;\n \t} else\n-- \n1.7.3.4\n"},{"id":"175766","messageId":"1316395657-6991-4-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 3/8] fast-import: cache oe in note_change_n","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:32Z","receivedAt":"2011-09-19T01:27:32Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"note_change_n checks the type of annotating data object to be Blob.\nFor objects given by sha1 it does so via sha1_object_info and does not\ncache the result in struct object_entry.\n\nMake this call to sha1_object_info cached in struct object_entry. Also\nmake note_change_n operate on oe rather than on sha1 - no functional\nchange, just a purification.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 30 +++++++++++++++++-------------\n 1 files changed, 17 insertions(+), 13 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 2b049f7..47c1e69 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2394,9 +2394,9 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)\n {\n \tconst char *p = command_buf.buf + 2;\n \tstatic struct strbuf uq = STRBUF_INIT;\n-\tstruct object_entry *oe = oe;\n+\tstruct object_entry *oe = NULL;\n \tstruct branch *s;\n-\tunsigned char sha1[20], commit_sha1[20];\n+\tunsigned char commit_sha1[20];\n \tchar path[60];\n \tuint16_t inline_data = 0;\n \tunsigned char new_fanout;\n@@ -2405,15 +2405,16 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)\n \tif (*p == ':') {\n \t\tchar *x;\n \t\toe = find_mark(strtoumax(p + 1, &x, 10));\n-\t\thashcpy(sha1, oe->idx.sha1);\n \t\tp = x;\n \t} else if (!prefixcmp(p, \"inline\")) {\n \t\tinline_data = 1;\n \t\tp += 6;\n \t} else {\n+\t\tunsigned char sha1[20];\n \t\tif (get_sha1_hex(p, sha1))\n \t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n-\t\toe = find_object(sha1);\n+\t\tif (!is_null_sha1(sha1))\n+\t\t\toe = insert_object(sha1);\n \t\tp += 40;\n \t}\n \tif (*p++ != ' ')\n@@ -2440,36 +2441,39 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)\n \t\tdie(\"Invalid ref name or SHA1 expression: %s\", p);\n \n \tif (inline_data) {\n+\t\tunsigned char sha1[20];\n \t\tif (p != uq.buf) {\n \t\t\tstrbuf_addstr(&uq, p);\n \t\t\tp = uq.buf;\n \t\t}\n \t\tread_next_command();\n \t\tparse_and_store_blob(&last_blob, sha1, 0);\n+\t\toe = find_object(sha1);\n \t} else if (oe) {\n+\t\tif (!oe->idx.offset) {\n+\t\t\tenum object_type type = sha1_object_info(oe->idx.sha1, NULL);\n+\t\t\tif (type < 0)\n+\t\t\t\tdie(\"Blob not found: %s\", command_buf.buf);\n+\t\t\toe->type = type;\n+\t\t\toe->pack_id = MAX_PACK_ID;\n+\t\t\toe->idx.offset = 1; /* nonzero */\n+\t\t}\n \t\tif (oe->type != OBJ_BLOB)\n \t\t\tdie(\"Not a blob (actually a %s): %s\",\n \t\t\t\ttypename(oe->type), command_buf.buf);\n-\t} else if (!is_null_sha1(sha1)) {\n-\t\tenum object_type type = sha1_object_info(sha1, NULL);\n-\t\tif (type < 0)\n-\t\t\tdie(\"Blob not found: %s\", command_buf.buf);\n-\t\tif (type != OBJ_BLOB)\n-\t\t\tdie(\"Not a blob (actually a %s): %s\",\n-\t\t\t typename(type), command_buf.buf);\n \t}\n \n \tconstruct_path_with_fanout(sha1_to_hex(commit_sha1), old_fanout, path);\n \tif (tree_content_remove(&b->branch_tree, path, NULL))\n \t\tb->num_notes--;\n \n-\tif (is_null_sha1(sha1))\n+\tif (!oe)\n \t\treturn; /* nothing to insert */\n \n \tb->num_notes++;\n \tnew_fanout = convert_num_notes_to_fanout(b->num_notes);\n \tconstruct_path_with_fanout(sha1_to_hex(commit_sha1), new_fanout, path);\n-\ttree_content_set(&b->branch_tree, path, sha1, S_IFREG | 0644, NULL);\n+\ttree_content_set(&b->branch_tree, path, oe->idx.sha1, S_IFREG | 0644, NULL);\n }\n \n static void file_change_deleteall(struct branch *b)\n-- \n1.7.3.4\n"},{"id":"175769","messageId":"1316395657-6991-5-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 4/8] fast-import: extract common sha1_file access functions","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:33Z","receivedAt":"2011-09-19T01:27:33Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-import asks sha1_object_info and find_sha1_pack to initialize\nstruct object_entry in several codepoints.\n\nExtract common functions doing this initialization.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 82 +++++++++++++++++++++++---------------------------------\n 1 files changed, 34 insertions(+), 48 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 47c1e69..3c2a067 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -589,6 +589,28 @@ static struct object_entry *insert_object(unsigned char *sha1)\n \treturn e;\n }\n \n+static void resolve_sha1_object(struct object_entry *oe)\n+{\n+\tenum object_type type = sha1_object_info(oe->idx.sha1, NULL);\n+\tif (type < 0)\n+\t\tdie(\"object not found: %s\", sha1_to_hex(oe->idx.sha1));\n+\toe->type = type;\n+\toe->pack_id = MAX_PACK_ID;\n+\toe->idx.offset = 1; /* nonzero */\n+}\n+\n+static int try_resolve_sha1_pack_object(struct object_entry *e,\n+\t\t\t\t\t\tenum object_type type)\n+{\n+\tif (!find_sha1_pack(e->idx.sha1, packed_git))\n+\t\treturn 0;\n+\n+\te->type = type;\n+\te->pack_id = MAX_PACK_ID;\n+\te->idx.offset = 1; /* just not zero! */\n+\treturn 1;\n+}\n+\n static unsigned int hc_str(const char *s, size_t len)\n {\n \tunsigned int r = 0;\n@@ -1042,10 +1064,7 @@ static int store_object(\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n-\t} else if (find_sha1_pack(sha1, packed_git)) {\n-\t\te->type = type;\n-\t\te->pack_id = MAX_PACK_ID;\n-\t\te->idx.offset = 1; /* just not zero! */\n+\t} else if (try_resolve_sha1_pack_object(e, type)) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n \t}\n@@ -1252,10 +1271,7 @@ static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n \t\ttruncate_pack(offset, &pack_file_ctx);\n \n-\t} else if (find_sha1_pack(sha1, packed_git)) {\n-\t\te->type = OBJ_BLOB;\n-\t\te->pack_id = MAX_PACK_ID;\n-\t\te->idx.offset = 1; /* just not zero! */\n+\t} else if (try_resolve_sha1_pack_object(e, OBJ_BLOB)) {\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n \t\ttruncate_pack(offset, &pack_file_ctx);\n \n@@ -1838,13 +1854,8 @@ static void read_marks(void)\n \t\t\tdie(\"corrupt mark line: %s\", line);\n \t\te = find_object(sha1);\n \t\tif (!e) {\n-\t\t\tenum object_type type = sha1_object_info(sha1, NULL);\n-\t\t\tif (type < 0)\n-\t\t\t\tdie(\"object not found: %s\", sha1_to_hex(sha1));\n \t\t\te = insert_object(sha1);\n-\t\t\te->type = type;\n-\t\t\te->pack_id = MAX_PACK_ID;\n-\t\t\te->idx.offset = 1; /* just not zero! */\n+\t\t\tresolve_sha1_object(e);\n \t\t}\n \t\tinsert_mark(mark, e);\n \t}\n@@ -2299,16 +2310,8 @@ static void file_change_m(struct branch *b)\n \t\t\t\t\t\tOBJ_TREE: OBJ_BLOB;\n \t\tif (!oe)\n \t\t\toe = insert_object(sha1);\n-\t\tif (!oe->idx.offset) {\n-\t\t\tenum object_type type = sha1_object_info(oe->idx.sha1, NULL);\n-\t\t\tif (type < 0)\n-\t\t\t\tdie(\"%s not found: %s\",\n-\t\t\t\t\t\tS_ISDIR(mode) ? \"Tree\" : \"Blob\",\n-\t\t\t\t\t\tcommand_buf.buf);\n-\t\t\toe->type = type;\n-\t\t\toe->pack_id = MAX_PACK_ID;\n-\t\t\toe->idx.offset = 1; /* nonzero */\n-\t\t}\n+\t\tif (!oe->idx.offset)\n+\t\t\tresolve_sha1_object(oe);\n \t\tif (oe->type != expected)\n \t\t\tdie(\"Not a %s (actually a %s): %s\",\n \t\t\t\ttypename(expected), typename(oe->type),\n@@ -2450,14 +2453,8 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)\n \t\tparse_and_store_blob(&last_blob, sha1, 0);\n \t\toe = find_object(sha1);\n \t} else if (oe) {\n-\t\tif (!oe->idx.offset) {\n-\t\t\tenum object_type type = sha1_object_info(oe->idx.sha1, NULL);\n-\t\t\tif (type < 0)\n-\t\t\t\tdie(\"Blob not found: %s\", command_buf.buf);\n-\t\t\toe->type = type;\n-\t\t\toe->pack_id = MAX_PACK_ID;\n-\t\t\toe->idx.offset = 1; /* nonzero */\n-\t\t}\n+\t\tif (!oe->idx.offset)\n+\t\t\tresolve_sha1_object(oe);\n \t\tif (oe->type != OBJ_BLOB)\n \t\t\tdie(\"Not a blob (actually a %s): %s\",\n \t\t\t\ttypename(oe->type), command_buf.buf);\n@@ -2737,15 +2734,10 @@ static void parse_new_tag(void)\n \t\thashcpy(sha1, oe->idx.sha1);\n \t} else if (!get_sha1(from, sha1)) {\n \t\tstruct object_entry *oe = insert_object(sha1);\n-\t\tif (!oe->idx.offset) {\n-\t\t\ttype = sha1_object_info(sha1, NULL);\n-\t\t\tif (type < 0)\n-\t\t\t\tdie(\"Not a valid object: %s\", from);\n-\t\t\toe->type = type;\n-\t\t\toe->pack_id = MAX_PACK_ID;\n-\t\t\toe->idx.offset = 1; /* nonzero */\n-\t\t} else\n-\t\t\ttype = oe->type;\n+\t\tif (!oe->idx.offset)\n+\t\t\tresolve_sha1_object(oe);\n+\n+\t\ttype = oe->type;\n \t} else\n \t\tdie(\"Invalid ref name or SHA1 expression: %s\", from);\n \tread_next_command();\n@@ -2892,14 +2884,8 @@ static struct object_entry *dereference(struct object_entry *oe,\n \tunsigned long size;\n \tchar *buf = NULL;\n \tif (!oe) {\n-\t\tenum object_type type = sha1_object_info(sha1, NULL);\n-\t\tif (type < 0)\n-\t\t\tdie(\"object not found: %s\", sha1_to_hex(sha1));\n-\t\t/* cache it! */\n \t\toe = insert_object(sha1);\n-\t\toe->type = type;\n-\t\toe->pack_id = MAX_PACK_ID;\n-\t\toe->idx.offset = 1;\n+\t\tresolve_sha1_object(oe);\n \t}\n \tswitch (oe->type) {\n \tcase OBJ_TREE:\t/* easy case. */\n-- \n1.7.3.4\n"},{"id":"175771","messageId":"1316395657-6991-6-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 5/8] fast-import: tiny optimization in read_marks","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:34Z","receivedAt":"2011-09-19T01:27:34Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"read_marks calls find_object and then insert_object if nothing is found.\n\nReduce it to just insert_object and a check if it was found or inserted.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 6 ++----\n 1 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 3c2a067..dd3dcd5 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1852,11 +1852,9 @@ static void read_marks(void)\n \t\tif (!mark || end == line + 1\n \t\t\t|| *end != ' ' || get_sha1(end + 1, sha1))\n \t\t\tdie(\"corrupt mark line: %s\", line);\n-\t\te = find_object(sha1);\n-\t\tif (!e) {\n-\t\t\te = insert_object(sha1);\n+\t\te = insert_object(sha1);\n+\t\tif (!e->idx.offset)\n \t\t\tresolve_sha1_object(e);\n-\t\t}\n \t\tinsert_mark(mark, e);\n \t}\n \tfclose(f);\n-- \n1.7.3.4\n"},{"id":"175767","messageId":"1316395657-6991-7-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 6/8] fast-import: cache oe in load_tree","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:35Z","receivedAt":"2011-09-19T01:27:35Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"load_tree reads a tree object given it's sha1. If there was no\nstruct object_entry allocated for this sha1, load_tree doesn't\nallocate it and thus doesn't cache it's struct object_entry.\n\nMake this read_sha1_file cached in struct object_entry.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 20 +++++++++++++++++---\n 1 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex dd3dcd5..1c0716b 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -599,6 +599,17 @@ static void resolve_sha1_object(struct object_entry *oe)\n \toe->idx.offset = 1; /* nonzero */\n }\n \n+static void *resolve_sha1_object_read(struct object_entry *oe, enum object_type *type, unsigned long *size)\n+{\n+\tvoid *ret = read_sha1_file(oe->idx.sha1, type, size);\n+\tif (!ret)\n+\t\treturn ret;\n+\toe->type = *type;\n+\toe->pack_id = MAX_PACK_ID;\n+\toe->idx.offset = 1; /* nonzero */\n+\treturn ret;\n+}\n+\n static int try_resolve_sha1_pack_object(struct object_entry *e,\n \t\t\t\t\t\tenum object_type type)\n {\n@@ -1363,8 +1374,8 @@ static void load_tree(struct tree_entry *root)\n \tif (is_null_sha1(sha1))\n \t\treturn;\n \n-\tmyoe = find_object(sha1);\n-\tif (myoe && myoe->pack_id != MAX_PACK_ID) {\n+\tmyoe = insert_object(sha1);\n+\tif (myoe->idx.offset && myoe->pack_id != MAX_PACK_ID) {\n \t\tif (myoe->type != OBJ_TREE)\n \t\t\tdie(\"Not a tree: %s\", sha1_to_hex(sha1));\n \t\tt->delta_depth = myoe->depth;\n@@ -1373,7 +1384,10 @@ static void load_tree(struct tree_entry *root)\n \t\t\tdie(\"Can't load tree %s\", sha1_to_hex(sha1));\n \t} else {\n \t\tenum object_type type;\n-\t\tbuf = read_sha1_file(sha1, &type, &size);\n+\t\tif (!myoe->idx.offset)\n+\t\t\tbuf = resolve_sha1_object_read(myoe, &type, &size);\n+\t\telse\n+\t\t\tbuf = read_sha1_file(sha1, &type, &size);\n \t\tif (!buf || type != OBJ_TREE)\n \t\t\tdie(\"Can't load tree %s\", sha1_to_hex(sha1));\n \t}\n-- \n1.7.3.4\n"},{"id":"175768","messageId":"1316395657-6991-8-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 7/8] fast-import: cache oe in cat_blob","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:36Z","receivedAt":"2011-09-19T01:27:36Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"cat_blob read_sha1_file's the blob object and doesn't cache\nit in struct object_entry.\n\nMake this call to read_sha1_file cached in struct object_entry.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 25 +++++++++++++------------\n 1 files changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 1c0716b..3c4c998 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2816,16 +2816,18 @@ static void cat_blob_write(const char *buf, unsigned long size)\n \t\tdie_errno(\"Write to frontend failed\");\n }\n \n-static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n+static void cat_blob(struct object_entry *oe)\n {\n \tstruct strbuf line = STRBUF_INIT;\n \tunsigned long size;\n \tenum object_type type = 0;\n \tchar *buf;\n \n-\tif (!oe || oe->pack_id == MAX_PACK_ID) {\n-\t\tbuf = read_sha1_file(sha1, &type, &size);\n-\t} else {\n+\tif (!oe->idx.offset)\n+\t\tbuf = resolve_sha1_object_read(oe, &type, &size);\n+\telse if (oe->pack_id == MAX_PACK_ID)\n+\t\tbuf = read_sha1_file(oe->idx.sha1, &type, &size);\n+\telse {\n \t\ttype = oe->type;\n \t\tbuf = gfi_unpack_entry(oe, &size);\n \t}\n@@ -2835,25 +2837,25 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n \t */\n \tif (type <= 0) {\n \t\tstrbuf_reset(&line);\n-\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n+\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(oe->idx.sha1));\n \t\tcat_blob_write(line.buf, line.len);\n \t\tstrbuf_release(&line);\n \t\tfree(buf);\n \t\treturn;\n \t}\n \tif (!buf)\n-\t\tdie(\"Can't read object %s\", sha1_to_hex(sha1));\n+\t\tdie(\"Can't read object %s\", sha1_to_hex(oe->idx.sha1));\n \tif (type != OBJ_BLOB)\n \t\tdie(\"Object %s is a %s but a blob was expected.\",\n-\t\t sha1_to_hex(sha1), typename(type));\n+\t\t sha1_to_hex(oe->idx.sha1), typename(type));\n \tstrbuf_reset(&line);\n-\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n+\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(oe->idx.sha1),\n \t\t\t\t\t\ttypename(type), size);\n \tcat_blob_write(line.buf, line.len);\n \tstrbuf_release(&line);\n \tcat_blob_write(buf, size);\n \tcat_blob_write(\"\\n\", 1);\n-\tif (oe && oe->pack_id == pack_id) {\n+\tif (oe->pack_id == pack_id) {\n \t\tlast_blob.offset = oe->idx.offset;\n \t\tstrbuf_attach(&last_blob.data, buf, size, size);\n \t\tlast_blob.depth = oe->depth;\n@@ -2878,16 +2880,15 @@ static void parse_cat_blob(void)\n \t\t\tdie(\"Unknown mark: %s\", command_buf.buf);\n \t\tif (*x)\n \t\t\tdie(\"Garbage after mark: %s\", command_buf.buf);\n-\t\thashcpy(sha1, oe->idx.sha1);\n \t} else {\n \t\tif (get_sha1_hex(p, sha1))\n \t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n \t\tif (p[40])\n \t\t\tdie(\"Garbage after SHA1: %s\", command_buf.buf);\n-\t\toe = find_object(sha1);\n+\t\toe = insert_object(sha1);\n \t}\n \n-\tcat_blob(oe, sha1);\n+\tcat_blob(oe);\n }\n \n static struct object_entry *dereference(struct object_entry *oe,\n-- \n1.7.3.4\n"},{"id":"175770","messageId":"1316395657-6991-9-git-send-email-divanorama@gmail.com","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 8/8] fast-import: cache objects while dereferencing","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-19T01:27:37Z","receivedAt":"2011-09-19T01:27:37Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"dereference() reads objects with read_sha1_file, and reads types\nof objects with sha1_object_info. But doesn't cache the result in\nstruct object_entry.\n\nMake these calls to read_sha1_file and sha1_object_info cached in\nstruct object_entry.\n\nSigned-off-by: Dmitry Ivankov \n---\n fast-import.c | 31 +++++++++++++++++--------------\n 1 files changed, 17 insertions(+), 14 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 3c4c998..43158c8 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2891,15 +2891,11 @@ static void parse_cat_blob(void)\n \tcat_blob(oe);\n }\n \n-static struct object_entry *dereference(struct object_entry *oe,\n-\t\t\t\t\tunsigned char sha1[20])\n+static struct object_entry *dereference(struct object_entry *oe)\n {\n+\tunsigned char next_sha1[20];\n \tunsigned long size;\n \tchar *buf = NULL;\n-\tif (!oe) {\n-\t\toe = insert_object(sha1);\n-\t\tresolve_sha1_object(oe);\n-\t}\n \tswitch (oe->type) {\n \tcase OBJ_TREE:\t/* easy case. */\n \t\treturn oe;\n@@ -2914,26 +2910,31 @@ static struct object_entry *dereference(struct object_entry *oe,\n \t\tbuf = gfi_unpack_entry(oe, &size);\n \t} else {\n \t\tenum object_type unused;\n-\t\tbuf = read_sha1_file(sha1, &unused, &size);\n+\t\tbuf = read_sha1_file(oe->idx.sha1, &unused, &size);\n \t}\n \tif (!buf)\n-\t\tdie(\"Can't load object %s\", sha1_to_hex(sha1));\n+\t\tdie(\"Can't load object %s\", sha1_to_hex(oe->idx.sha1));\n \n \t/* Peel one layer. */\n \tswitch (oe->type) {\n \tcase OBJ_TAG:\n \t\tif (size < 40 + strlen(\"object \") ||\n-\t\t get_sha1_hex(buf + strlen(\"object \"), sha1))\n+\t\t get_sha1_hex(buf + strlen(\"object \"), next_sha1))\n \t\t\tdie(\"Invalid SHA1 in tag: %s\", command_buf.buf);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n \t\tif (size < 40 + strlen(\"tree \") ||\n-\t\t get_sha1_hex(buf + strlen(\"tree \"), sha1))\n+\t\t get_sha1_hex(buf + strlen(\"tree \"), next_sha1))\n \t\t\tdie(\"Invalid SHA1 in commit: %s\", command_buf.buf);\n \t}\n \n \tfree(buf);\n-\treturn find_object(sha1);\n+\n+\toe = insert_object(next_sha1);\n+\tif (!oe->idx.offset)\n+\t\tresolve_sha1_object(oe);\n+\n+\treturn oe;\n }\n \n static struct object_entry *parse_treeish_dataref(const char **p)\n@@ -2953,12 +2954,14 @@ static struct object_entry *parse_treeish_dataref(const char **p)\n \t} else {\t/* */\n \t\tif (get_sha1_hex(*p, sha1))\n \t\t\tdie(\"Invalid SHA1: %s\", command_buf.buf);\n-\t\te = find_object(sha1);\n+\t\te = insert_object(sha1);\n+\t\tif (!e->idx.offset)\n+\t\t\tresolve_sha1_object(e);\n \t\t*p += 40;\n \t}\n \n-\twhile (!e || e->type != OBJ_TREE)\n-\t\te = dereference(e, sha1);\n+\twhile (e->type != OBJ_TREE)\n+\t\te = dereference(e);\n \treturn e;\n }\n \n-- \n1.7.3.4\n"},{"id":"175833","messageId":"7vy5xj7tf5.fsf@alter.siamese.dyndns.org","threadId":"28426","inReplyTo":"1316395657-6991-1-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH 0/8] fast-import: cache oe more often","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-20T04:02:22Z","receivedAt":"2011-09-20T04:02:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Ivankov writes:\n\n> fast-import keeps a struct object_entry for each object written to\n> it's pack. This is to keep type, pack-coordinates and delta_depth.\n> struct object_entry is also used to cache this metadata for objects\n> that exist outside fast-import's pack ('old' objects).\n> struct object_entry has a small fixed size and thus it should be\n> reasonable to cache any 'old' object metadata retrieval to save the\n> disk i/o.\n>\n> Also it is a step toward making fast-import identify objects via\n> struct object_entry rather than sha1. One pointer takes less than\n> 20 bytes, it'll be later possible to have references to objects\n> that don't yet have sha1 computed (fast-import with threads future).\n\nI gave the series a cursory look, and the patches all looked like a good\nand straight forward rewrites. Provided if it is indeed a good idea\noverall to stuff more objects in-core, that is.\n\nHopefully people more involved in fast-import can review and ack after the\npre-release feature freeze.\n"},{"id":"175834","messageId":"20110920042655.GH6343@elie","threadId":"28426","inReplyTo":"7vy5xj7tf5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/8] fast-import: cache oe more often","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-09-20T04:26:56Z","receivedAt":"2011-09-20T04:26:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> I gave the series a cursory look, and the patches all looked like a good\n> and straight forward rewrites. Provided if it is indeed a good idea\n> overall to stuff more objects in-core, that is.\n\nRight, that's exactly the question I had. When and why is it a good\nidea to stuff more objects in-core (or when might it be a bad idea,\nfor that matter)?\n"},{"id":"175837","messageId":"CA+gfSn-nh4BhCPf6m8+EN0zo=BuhxRNLcLBx7ynRWPA=GxfDyg@mail.gmail.com","threadId":"28426","inReplyTo":"20110920042655.GH6343@elie","subject":"Re: [PATCH 0/8] fast-import: cache oe more often","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-20T07:17:36Z","receivedAt":"2011-09-20T07:17:36Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"On Tue, Sep 20, 2011 at 10:26 AM, Jonathan Nieder wrote:\n> Junio C Hamano wrote:\n>\n>> I gave the series a cursory look, and the patches all looked like a good\n>> and straight forward rewrites.  Provided if it is indeed a good idea\n>> overall to stuff more objects in-core, that is.\n>\n> Right, that's exactly the question I had.  When and why is it a good\n> idea to stuff more objects in-core (or when might it be a bad idea,\n> for that matter)?\n\nThe next step would be to replace sha1 with struct object_entry* in fast-import.\nSo it'll be in struct tree_entry (twice, for each of versions[2]),\nbranch, tag, hash_list (used to store merge from lists), last_object.\nThen some fields will be deleted as they can be accessed from\nobject_entry:\nlast_object->depth\nlast_object->offset\ntree_content->delta_depth\nbranch,tag->pack_id\n\nAnd it all even slightly decreased memory consumption (checked some\ntime ago, but think it's still true). Probably because of tree nodes\nhaving NULL instead of null_sha1 and then ptr instead of sha1; and\nmaybe because for huge imports each object is from our pack so for a\nsha1 there always is object_entry anyway.\n\nIn short, if there is nothing bad with this patchset, it'll be\nabsolutely natural one after switch to oe instead of sha1, but it's\nput before to split the big series. And of course this part may have a\nsmall speedup of it's own. If it's not too good to be accepted on it's\nown, I'll just include it into future series depending on it.\n"},{"id":"175869","messageId":"20110920143941.GE7517@elie","threadId":"28426","inReplyTo":"CA+gfSn-nh4BhCPf6m8+EN0zo=BuhxRNLcLBx7ynRWPA=GxfDyg@mail.gmail.com","subject":"Re: [PATCH 0/8] fast-import: cache oe more often","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-09-20T14:39:41Z","receivedAt":"2011-09-20T14:39:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Dmitry Ivankov wrote:\n\n> The next step would be to replace sha1 with struct object_entry* in fast-import.\n> So it'll be in struct tree_entry (twice, for each of versions[2]),\n> branch, tag, hash_list (used to store merge from lists), last_object.\n> Then some fields will be deleted as they can be accessed from\n> object_entry:\n> last_object->depth\n> last_object->offset\n> tree_content->delta_depth\n> branch,tag->pack_id\n> \n> And it all even slightly decreased memory consumption (checked some\n> time ago, but think it's still true).\n\nYes, that sounds interesting, so:\n\n[...]\n> In short, if there is nothing bad with this patchset, it'll be\n> absolutely natural one after switch to oe instead of sha1, but it's\n> put before to split the big series. And of course this part may have a\n> small speedup of it's own. If it's not too good to be accepted on it's\n> own, I'll just include it into future series depending on it.\n\nIt would be indeed be more natural to review a single series that\ncombines this preparation with the change it prepares for. (And the\nchange descriptions should explain on their own why they are\nindividually justified or what project they are contributing towards.)\n\nMy question was actually about this last point you made in the\nsecond-to-last sentence: have you measured the speedup produced by the\npatches you already sent? I didn't think carefully about it, but my\nfirst thought was that it might slow things down as the internal hash\ntables (which still seem to be fixed-size in mainline git) start to\nfill up.\n"}]}