Skip to content

Commit ef8db63

Browse files
jherlandgitster
jherland
authored and
gitster
committed
Refactor notes code to concatenate multiple notes annotating the same object
Currently, having multiple notes referring to the same commit from various locations in the notes tree is strongly discouraged, since only one of those notes will be parsed and shown. This patch teaches the notes code to _concatenate_ multiple notes that annotate the same commit. Notes are concatenated by creating a new blob object containing the concatenation of the notes in question, and replacing them with the concatenated note in the internal notes tree structure. Getting the concatenation right requires being more proactive in unpacking subtree entries in the internal notes tree structure, so that we don't return a note prematurely (i.e. before having found all other notes that annotate the same object). As such, this patch may incur a small performance penalty. Suggested-by: Sam Vilain <sam@vilain.net> Re-suggested-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 0057c09 commit ef8db63

File tree

1 file changed

+161
-82
lines changed

1 file changed

+161
-82
lines changed

‎notes.c‎

Lines changed: 161 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -59,115 +59,196 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
5959
unsigned int n);
6060

6161
/*
62-
* To find a leaf_node:
62+
* Search the tree until the appropriate location for the given key is found:
6363
* 1. Start at the root node, with n = 0
64-
* 2. Use the nth nibble of the key as an index into a:
65-
* - If a[n] is an int_node, recurse into that node and increment n
66-
* - If a leaf_node with matching key, return leaf_node (assert note entry)
64+
* 2. If a[0] at the current level is a matching subtree entry, unpack that
65+
* subtree entry and remove it; restart search at the current level.
66+
* 3. Use the nth nibble of the key as an index into a:
67+
* - If a[n] is an int_node, recurse from #2 into that node and increment n
6768
* - If a matching subtree entry, unpack that subtree entry (and remove it);
6869
* restart search at the current level.
69-
* - Otherwise, we end up at a NULL pointer, or a non-matching leaf_node.
70-
* Backtrack out of the recursion, one level at a time and check a[0]:
71-
* - If a[0] at the current level is a matching subtree entry, unpack that
72-
* subtree entry (and remove it); restart search at the current level.
70+
* - Otherwise, we have found one of the following:
71+
* - a subtree entry which does not match the key
72+
* - a note entry which may or may not match the key
73+
* - an unused leaf node (NULL)
74+
* In any case, set *tree and *n, and return pointer to the tree location.
7375
*/
74-
static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
75-
const unsigned char *key_sha1)
76+
static void **note_tree_search(struct int_node **tree,
77+
unsigned char *n, const unsigned char *key_sha1)
7678
{
7779
struct leaf_node *l;
78-
unsigned char i = GET_NIBBLE(n, key_sha1);
79-
void *p = tree->a[i];
80+
unsigned char i;
81+
void *p = (*tree)->a[0];
8082

83+
if (GET_PTR_TYPE(p) == PTR_TYPE_SUBTREE) {
84+
l = (struct leaf_node *) CLR_PTR_TYPE(p);
85+
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
86+
/* unpack tree and resume search */
87+
(*tree)->a[0] = NULL;
88+
load_subtree(l, *tree, *n);
89+
free(l);
90+
return note_tree_search(tree, n, key_sha1);
91+
}
92+
}
93+
94+
i = GET_NIBBLE(*n, key_sha1);
95+
p = (*tree)->a[i];
8196
switch(GET_PTR_TYPE(p)) {
8297
case PTR_TYPE_INTERNAL:
83-
l = note_tree_find(CLR_PTR_TYPE(p), n + 1, key_sha1);
84-
if (l)
85-
return l;
86-
break;
87-
case PTR_TYPE_NOTE:
88-
l = (struct leaf_node *) CLR_PTR_TYPE(p);
89-
if (!hashcmp(key_sha1, l->key_sha1))
90-
return l; /* return note object matching given key */
91-
break;
98+
*tree = CLR_PTR_TYPE(p);
99+
(*n)++;
100+
return note_tree_search(tree, n, key_sha1);
92101
case PTR_TYPE_SUBTREE:
93102
l = (struct leaf_node *) CLR_PTR_TYPE(p);
94103
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
95104
/* unpack tree and resume search */
96-
tree->a[i] = NULL;
97-
load_subtree(l, tree, n);
105+
(*tree)->a[i] = NULL;
106+
load_subtree(l, *tree, *n);
98107
free(l);
99-
return note_tree_find(tree, n, key_sha1);
108+
return note_tree_search(tree, n, key_sha1);
100109
}
101-
break;
102-
case PTR_TYPE_NULL:
110+
/* fall through */
103111
default:
104-
assert(!p);
105-
break;
112+
return &((*tree)->a[i]);
106113
}
114+
}
107115

108-
/*
109-
* Did not find key at this (or any lower) level.
110-
* Check if there's a matching subtree entry in tree->a[0].
111-
* If so, unpack tree and resume search.
112-
*/
113-
p = tree->a[0];
114-
if (GET_PTR_TYPE(p) != PTR_TYPE_SUBTREE)
115-
return NULL;
116-
l = (struct leaf_node *) CLR_PTR_TYPE(p);
117-
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
118-
/* unpack tree and resume search */
119-
tree->a[0] = NULL;
120-
load_subtree(l, tree, n);
121-
free(l);
122-
return note_tree_find(tree, n, key_sha1);
116+
/*
117+
* To find a leaf_node:
118+
* Search to the tree location appropriate for the given key:
119+
* If a note entry with matching key, return the note entry, else return NULL.
120+
*/
121+
static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
122+
const unsigned char *key_sha1)
123+
{
124+
void **p = note_tree_search(&tree, &n, key_sha1);
125+
if (GET_PTR_TYPE(*p) == PTR_TYPE_NOTE) {
126+
struct leaf_node *l = (struct leaf_node *) CLR_PTR_TYPE(*p);
127+
if (!hashcmp(key_sha1, l->key_sha1))
128+
return l;
123129
}
124130
return NULL;
125131
}
126132

133+
/* Create a new blob object by concatenating the two given blob objects */
134+
static int concatenate_notes(unsigned char *cur_sha1,
135+
const unsigned char *new_sha1)
136+
{
137+
char *cur_msg, *new_msg, *buf;
138+
unsigned long cur_len, new_len, buf_len;
139+
enum object_type cur_type, new_type;
140+
int ret;
141+
142+
/* read in both note blob objects */
143+
new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
144+
if (!new_msg || !new_len || new_type != OBJ_BLOB) {
145+
free(new_msg);
146+
return 0;
147+
}
148+
cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
149+
if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
150+
free(cur_msg);
151+
free(new_msg);
152+
hashcpy(cur_sha1, new_sha1);
153+
return 0;
154+
}
155+
156+
/* we will separate the notes by a newline anyway */
157+
if (cur_msg[cur_len - 1] == '\n')
158+
cur_len--;
159+
160+
/* concatenate cur_msg and new_msg into buf */
161+
buf_len = cur_len + 1 + new_len;
162+
buf = (char *) xmalloc(buf_len);
163+
memcpy(buf, cur_msg, cur_len);
164+
buf[cur_len] = '\n';
165+
memcpy(buf + cur_len + 1, new_msg, new_len);
166+
167+
free(cur_msg);
168+
free(new_msg);
169+
170+
/* create a new blob object from buf */
171+
ret = write_sha1_file(buf, buf_len, "blob", cur_sha1);
172+
free(buf);
173+
return ret;
174+
}
175+
127176
/*
128177
* To insert a leaf_node:
129-
* 1. Start at the root node, with n = 0
130-
* 2. Use the nth nibble of the key as an index into a:
131-
* - If a[n] is NULL, store the tweaked pointer directly into a[n]
132-
* - If a[n] is an int_node, recurse into that node and increment n
133-
* - If a[n] is a leaf_node:
134-
* 1. Check if they're equal, and handle that (abort? overwrite?)
135-
* 2. Create a new int_node, and store both leaf_nodes there
136-
* 3. Store the new int_node into a[n].
178+
* Search to the tree location appropriate for the given leaf_node's key:
179+
* - If location is unused (NULL), store the tweaked pointer directly there
180+
* - If location holds a note entry that matches the note-to-be-inserted, then
181+
* concatenate the two notes.
182+
* - If location holds a note entry that matches the subtree-to-be-inserted,
183+
* then unpack the subtree-to-be-inserted into the location.
184+
* - If location holds a matching subtree entry, unpack the subtree at that
185+
* location, and restart the insert operation from that level.
186+
* - Else, create a new int_node, holding both the node-at-location and the
187+
* node-to-be-inserted, and store the new int_node into the location.
137188
*/
138-
static int note_tree_insert(struct int_node *tree, unsigned char n,
139-
const struct leaf_node *entry, unsigned char type)
189+
static void note_tree_insert(struct int_node *tree, unsigned char n,
190+
struct leaf_node *entry, unsigned char type)
140191
{
141192
struct int_node *new_node;
142-
const struct leaf_node *l;
143-
int ret;
144-
unsigned char i = GET_NIBBLE(n, entry->key_sha1);
145-
void *p = tree->a[i];
146-
assert(GET_PTR_TYPE(entry) == PTR_TYPE_NULL);
147-
switch(GET_PTR_TYPE(p)) {
193+
struct leaf_node *l;
194+
void **p = note_tree_search(&tree, &n, entry->key_sha1);
195+
196+
assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
197+
l = (struct leaf_node *) CLR_PTR_TYPE(*p);
198+
switch(GET_PTR_TYPE(*p)) {
148199
case PTR_TYPE_NULL:
149-
assert(!p);
150-
tree->a[i] = SET_PTR_TYPE(entry, type);
151-
return 0;
152-
case PTR_TYPE_INTERNAL:
153-
return note_tree_insert(CLR_PTR_TYPE(p), n + 1, entry, type);
154-
default:
155-
assert(GET_PTR_TYPE(p) == PTR_TYPE_NOTE ||
156-
GET_PTR_TYPE(p) == PTR_TYPE_SUBTREE);
157-
l = (const struct leaf_node *) CLR_PTR_TYPE(p);
158-
if (!hashcmp(entry->key_sha1, l->key_sha1))
159-
return -1; /* abort insert on matching key */
160-
new_node = (struct int_node *)
161-
xcalloc(sizeof(struct int_node), 1);
162-
ret = note_tree_insert(new_node, n + 1,
163-
CLR_PTR_TYPE(p), GET_PTR_TYPE(p));
164-
if (ret) {
165-
free(new_node);
166-
return -1;
200+
assert(!*p);
201+
*p = SET_PTR_TYPE(entry, type);
202+
return;
203+
case PTR_TYPE_NOTE:
204+
switch (type) {
205+
case PTR_TYPE_NOTE:
206+
if (!hashcmp(l->key_sha1, entry->key_sha1)) {
207+
/* skip concatenation if l == entry */
208+
if (!hashcmp(l->val_sha1, entry->val_sha1))
209+
return;
210+
211+
if (concatenate_notes(l->val_sha1,
212+
entry->val_sha1))
213+
die("failed to concatenate note %s "
214+
"into note %s for commit %s",
215+
sha1_to_hex(entry->val_sha1),
216+
sha1_to_hex(l->val_sha1),
217+
sha1_to_hex(l->key_sha1));
218+
free(entry);
219+
return;
220+
}
221+
break;
222+
case PTR_TYPE_SUBTREE:
223+
if (!SUBTREE_SHA1_PREFIXCMP(l->key_sha1,
224+
entry->key_sha1)) {
225+
/* unpack 'entry' */
226+
load_subtree(entry, tree, n);
227+
free(entry);
228+
return;
229+
}
230+
break;
231+
}
232+
break;
233+
case PTR_TYPE_SUBTREE:
234+
if (!SUBTREE_SHA1_PREFIXCMP(entry->key_sha1, l->key_sha1)) {
235+
/* unpack 'l' and restart insert */
236+
*p = NULL;
237+
load_subtree(l, tree, n);
238+
free(l);
239+
note_tree_insert(tree, n, entry, type);
240+
return;
167241
}
168-
tree->a[i] = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
169-
return note_tree_insert(new_node, n + 1, entry, type);
242+
break;
170243
}
244+
245+
/* non-matching leaf_node */
246+
assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
247+
GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
248+
new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
249+
note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p));
250+
*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
251+
note_tree_insert(new_node, n + 1, entry, type);
171252
}
172253

173254
/* Free the entire notes data contained in the given tree */
@@ -220,7 +301,6 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
220301
{
221302
unsigned char commit_sha1[20];
222303
unsigned int prefix_len;
223-
int status;
224304
void *buf;
225305
struct tree_desc desc;
226306
struct name_entry entry;
@@ -254,8 +334,7 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
254334
l->key_sha1[19] = (unsigned char) len;
255335
type = PTR_TYPE_SUBTREE;
256336
}
257-
status = note_tree_insert(node, n, l, type);
258-
assert(!status);
337+
note_tree_insert(node, n, l, type);
259338
}
260339
}
261340
free(buf);

0 commit comments

Comments
 (0)