{"thread":{"id":"45744","subject":"[PATCH] split-index: add and use unshare_split_index()","startedAt":"2017-04-19T09:33:34Z","lastAt":"2017-04-19T09:33:34Z","messageCount":1,"participants":["Nguyễn Thái Ngọc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"317207","messageId":"20170419093314.4454-1-pclouds@gmail.com","threadId":"45744","inReplyTo":null,"subject":"[PATCH] split-index: add and use unshare_split_index()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-19T09:33:14Z","receivedAt":"2017-04-19T09:33:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When split-index is being used, we have two cache_entry arrays in\nindex_state->cache[] and index_state->split_index->base->cache[].\n\nindex_state->cache[] may share the same entries with base->cache[] so\nwe can quickly determine what entries are shared. This makes memory\nmanagement tricky, we can't free base->cache[] until we know\nindex_state->cache[] does not point to any of those entries.\n\nunshare_split_index() is added for this purpose, to find shared\nentries and either duplicate them in index_state->cache[], or discard\nthem. Either way it should be safe to free base->cache[] after\nunshare_split_index().\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Sorry for the delay. Somehow I put this commit on a branch named\n \"custom-decorate\" and couldn't find it in \"git branch\" listing\n for obvious reason. Luckily \"git log --patch --all\" came and saved\n the day.\n\n The idea is well explained in the commit message, I hope. I'll have\n to look at it again (it's been six months since it's written) to see\n if it still makes sense.\n\n Anyway Christian, feel free to take over ;-)\n\n read-cache.c  | 10 ++--------\n split-index.c | 57 ++++++++++++++++++++++++++++++++++++++++++++-------------\n split-index.h |  1 +\n 3 files changed, 47 insertions(+), 21 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex e447751823..ae2d02a687 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1734,15 +1734,9 @@ int discard_index(struct index_state *istate)\n {\n \tint i;\n \n-\tfor (i = 0; i < istate->cache_nr; i++) {\n-\t\tif (istate->cache[i]->index &&\n-\t\t    istate->split_index &&\n-\t\t    istate->split_index->base &&\n-\t\t    istate->cache[i]->index <= istate->split_index->base->cache_nr &&\n-\t\t    istate->cache[i] == istate->split_index->base->cache[istate->cache[i]->index - 1])\n-\t\t\tcontinue;\n+\tunshare_split_index(istate, 1);\n+\tfor (i = 0; i < istate->cache_nr; i++)\n \t\tfree(istate->cache[i]);\n-\t}\n \tresolve_undo_clear_index(istate);\n \tistate->cache_nr = 0;\n \tistate->cache_changed = 0;\ndiff --git a/split-index.c b/split-index.c\nindex f519e60f87..49bd197f71 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -73,10 +73,17 @@ void move_cache_to_base_index(struct index_state *istate)\n \tint i;\n \n \t/*\n-\t * do not delete old si->base, its index entries may be shared\n-\t * with istate->cache[]. Accept a bit of leaking here because\n-\t * this code is only used by short-lived update-index.\n+\t * If \"si\" is shared with another index_state (e.g. by\n+\t * unpack-trees code), we will need to duplicate split_index\n+\t * struct. It's not happening now though, luckily.\n \t */\n+\tassert(si->refcount <= 1);\n+\n+\tunshare_split_index(istate, 0);\n+\tif (si->base) {\n+\t\tdiscard_index(si->base);\n+\t\tfree(si->base);\n+\t}\n \tsi->base = xcalloc(1, sizeof(*si->base));\n \tsi->base->version = istate->version;\n \t/* zero timestamp disables racy test in ce_write_index() */\n@@ -275,11 +282,41 @@ void finish_writing_split_index(struct index_state *istate)\n \tistate->cache_nr = si->saved_cache_nr;\n }\n \n+void unshare_split_index(struct index_state *istate, int discard)\n+{\n+\tstruct split_index *si = istate->split_index;\n+\tint i;\n+\n+\tif (!si || !si->base)\n+\t\treturn;\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tstruct cache_entry *new = NULL;\n+\n+\t\tif (!ce->index ||\n+\t\t    ce->index > si->base->cache_nr ||\n+\t\t    ce != si->base->cache[ce->index - 1])\n+\t\t\tcontinue;\n+\n+\t\tif (!discard) {\n+\t\t\tint len = ce_namelen(ce);\n+\t\t\tnew = xcalloc(1, cache_entry_size(len));\n+\t\t\tcopy_cache_entry(new, ce);\n+\t\t\tmemcpy(new->name, ce->name, len);\n+\t\t\tnew->index = 0;\n+\t\t}\n+\t\tistate->cache[i] = new;\n+\t}\n+}\n+\n+\n void discard_split_index(struct index_state *istate)\n {\n \tstruct split_index *si = istate->split_index;\n \tif (!si)\n \t\treturn;\n+\tunshare_split_index(istate, 0);\n \tistate->split_index = NULL;\n \tsi->refcount--;\n \tif (si->refcount)\n@@ -328,14 +365,8 @@ void add_split_index(struct index_state *istate)\n \n void remove_split_index(struct index_state *istate)\n {\n-\tif (istate->split_index) {\n-\t\t/*\n-\t\t * can't discard_split_index(&the_index); because that\n-\t\t * will destroy split_index->base->cache[], which may\n-\t\t * be shared with the_index.cache[]. So yeah we're\n-\t\t * leaking a bit here.\n-\t\t */\n-\t\tistate->split_index = NULL;\n-\t\tistate->cache_changed |= SOMETHING_CHANGED;\n-\t}\n+\tif (!istate->split_index)\n+\t\treturn;\n+\tdiscard_split_index(istate);\n+\tistate->cache_changed |= SOMETHING_CHANGED;\n }\ndiff --git a/split-index.h b/split-index.h\nindex df91c1bda8..65c0f09b2b 100644\n--- a/split-index.h\n+++ b/split-index.h\n@@ -33,5 +33,6 @@ void finish_writing_split_index(struct index_state *istate);\n void discard_split_index(struct index_state *istate);\n void add_split_index(struct index_state *istate);\n void remove_split_index(struct index_state *istate);\n+void unshare_split_index(struct index_state *istate, int discard);\n \n #endif\n-- \n2.11.0.157.gd943d85\n\n"}]}