From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
Yet another iteration of the 'git notes' feature. Rebased on top of 'next':
- Patches 1-9 are unchanged from (patches 1-7, 11-12 of) the last iteration.
- Patch 10 teaches the notes code to free its data structures on request.
- Patch 11 introduces the 16-tree notes lookup code that handles SHA1-based
fanout schemes. This is pretty much unchanged from patch 8 in the previous
iteration.
- Patch 12 adds selftests that verify correct parsing of notes trees with
various SHA1-based fanouts.
- Patch 13 introduces a flexible parser for a variety of date-based and
SHA1-based fanout schemes. This is the interesting part, as far as this
iteration is concerned.
- Patch 14 adds selftests that verify correct parsing of notes trees with
various date-based fanouts.
Note that the series does not yet include code for _writing_ notes into a
suitably structured notes tree. That will be done in a later iteration.
I have some performance numbers that I will send in a separate email.
Have fun! :)
...Johan
Johan Herland (9):
Teach "-m <msg>" and "-F <file>" to "git notes edit"
fast-import: Add support for importing commit notes
t3302-notes-index-expensive: Speed up create_repo()
Add flags to get_commit_notes() to control the format of the note string
Teach notes code to free its internal data structures on request.
Teach the notes lookup code to parse notes trees with various fanout schemes
Selftests verifying semantics when loading notes trees with various fanouts
Allow flexible organization of notes trees, using both commit date and SHA1
Add test cases for date-based fanouts
Johannes Schindelin (5):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes
Add '%N'-format for pretty-printing commit notes
.gitignore | 1 +
Documentation/config.txt | 13 +
Documentation/git-fast-import.txt | 45 +++-
Documentation/git-notes.txt | 60 ++++
Documentation/pretty-formats.txt | 1 +
Makefile | 3 +
cache.h | 4 +
command-list.txt | 1 +
commit.c | 1 +
config.c | 5 +
environment.c | 1 +
fast-import.c | 88 +++++-
git-notes.sh | 121 +++++++
notes.c | 673 +++++++++++++++++++++++++++++++++++++
notes.h | 12 +
pretty.c | 10 +
t/t3301-notes.sh | 150 ++++++++
t/t3302-notes-index-expensive.sh | 118 +++++++
t/t3303-notes-subtrees.sh | 201 +++++++++++
t/t9300-fast-import.sh | 166 +++++++++
20 files changed, 1664 insertions(+), 10 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100644 notes.c
create mode 100644 notes.h
create mode 100755 t/t3301-notes.sh
create mode 100755 t/t3302-notes-index-expensive.sh
create mode 100755 t/t3303-notes-subtrees.sh
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
From: Johannes Schindelin <redacted>
Commit notes are blobs which are shown together with the commit
message. These blobs are taken from the notes ref, which you can
configure by the config variable core.notesRef, which in turn can
be overridden by the environment variable GIT_NOTES_REF.
The notes ref is a branch which contains "files" whose names are
the names of the corresponding commits (i.e. the SHA-1).
The rationale for putting this information into a ref is this: we
want to be able to fetch and possibly union-merge the notes,
maybe even look at the date when a note was introduced, and we
want to store them efficiently together with the other objects.
This patch has been improved by the following contributions:
- Thomas Rast: fix core.notesRef documentation
- Tor Arne Vestbø: fix printing of multi-line notes
- Alex Riesen: Using char array instead of char pointer costs less BSS
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Thomas Rast <redacted>
Signed-off-by: Tor Arne Vestbø <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
Documentation/config.txt | 13 +++++++++
Makefile | 2 +
cache.h | 4 +++
commit.c | 1 +
config.c | 5 +++
environment.c | 1 +
notes.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 7 +++++
pretty.c | 5 +++
9 files changed, 106 insertions(+), 0 deletions(-)
create mode 100644 notes.c
create mode 100644 notes.h
@@ -443,6 +443,19 @@ On some file system/operating system combinations, this is unreliable. Set this config setting to 'rename' there; However, This will remove the check that makes sure that existing object files will not get overwritten.+core.notesRef::+ When showing commit messages, also show notes which are stored in+ the given ref. This ref is expected to contain files named+ after the full SHA-1 of the commit they annotate.+++If such a file exists in the given ref, the referenced blob is read, and+appended to the commit message, separated by a "Notes:" line. If the+given ref itself does not exist, it is not an error, but means that no+notes should be printed.+++This setting defaults to "refs/notes/commits", and can be overridden by+the `GIT_NOTES_REF` environment variable.+ add.ignore-errors:: Tells 'git-add' to continue adding files when some files cannot be added due to indexing errors. Equivalent to the '--ignore-errors'
@@ -0,0 +1,68 @@+#include"cache.h"+#include"commit.h"+#include"notes.h"+#include"refs.h"+#include"utf8.h"+#include"strbuf.h"++staticintinitialized;++voidget_commit_notes(conststructcommit*commit,structstrbuf*sb,+constchar*output_encoding)+{+staticconstcharutf8[]="utf-8";+structstrbufname=STRBUF_INIT;+unsignedcharsha1[20];+char*msg,*msg_p;+unsignedlonglinelen,msglen;+enumobject_typetype;++if(!initialized){+constchar*env=getenv(GIT_NOTES_REF_ENVIRONMENT);+if(env)+notes_ref_name=getenv(GIT_NOTES_REF_ENVIRONMENT);+elseif(!notes_ref_name)+notes_ref_name=GIT_NOTES_DEFAULT_REF;+if(notes_ref_name&&read_ref(notes_ref_name,sha1))+notes_ref_name=NULL;+initialized=1;+}++if(!notes_ref_name)+return;++strbuf_addf(&name,"%s:%s",notes_ref_name,+sha1_to_hex(commit->object.sha1));+if(get_sha1(name.buf,sha1))+return;++if(!(msg=read_sha1_file(sha1,&type,&msglen))||!msglen||+type!=OBJ_BLOB)+return;++if(output_encoding&&*output_encoding&&+strcmp(utf8,output_encoding)){+char*reencoded=reencode_string(msg,output_encoding,utf8);+if(reencoded){+free(msg);+msg=reencoded;+msglen=strlen(msg);+}+}++/* we will end the annotation by a newline anyway */+if(msglen&&msg[msglen-1]=='\n')+msglen--;++strbuf_addstr(sb,"\nNotes:\n");++for(msg_p=msg;msg_p<msg+msglen;msg_p+=linelen+1){+linelen=strchrnul(msg_p,'\n')-msg_p;++strbuf_addstr(sb," ");+strbuf_add(sb,msg_p,linelen);+strbuf_addch(sb,'\n');+}++free(msg);+}
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
From: Johannes Schindelin <redacted>
The script 'git notes' allows you to edit and show commit notes, by
calling either
git notes show <commit>
or
git notes edit <commit>
This patch has been improved by the following contributions:
- Tor Arne Vestbø: fix printing of multi-line notes
- Michael J Gruber: test and handle empty notes gracefully
- Thomas Rast:
- only clean up message file when editing
- use GIT_EDITOR and core.editor over VISUAL/EDITOR
- t3301: fix confusing quoting in test for valid notes ref
- t3301: use test_must_fail instead of !
- refuse to edit notes outside refs/notes/
- Junio C Hamano: tests: fix "export var=val"
- Christian Couder: documentation: fix 'linkgit' macro in "git-notes.txt"
- Johan Herland: minor cleanup and bugfixing in git-notes.sh (v2)
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Tor Arne Vestbø <redacted>
Signed-off-by: Michael J Gruber <redacted>
Signed-off-by: Thomas Rast <redacted>
Signed-off-by: Christian Couder <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
.gitignore | 1 +
Documentation/git-notes.txt | 46 +++++++++++++++++
Makefile | 1 +
command-list.txt | 1 +
git-notes.sh | 73 +++++++++++++++++++++++++++
t/t3301-notes.sh | 114 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 236 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100755 t/t3301-notes.sh
@@ -0,0 +1,46 @@+git-notes(1)+============++NAME+----+git-notes - Add/inspect commit notes++SYNOPSIS+--------+[verse]+'git-notes' (edit | show) [commit]++DESCRIPTION+-----------+This command allows you to add notes to commit messages, without+changing the commit. To discern these notes from the message stored+in the commit object, the notes are indented like the message, after+an unindented line saying "Notes:".++To disable commit notes, you have to set the config variable+core.notesRef to the empty string. Alternatively, you can set it+to a different ref, something like "refs/notes/bugzilla". This setting+can be overridden by the environment variable "GIT_NOTES_REF".+++SUBCOMMANDS+-----------++edit::+ Edit the notes for a given commit (defaults to HEAD).++show::+ Show the notes for a given commit (defaults to HEAD).+++Author+------+Written by Johannes Schindelin <johannes.schindelin@gmx.de>++Documentation+-------------+Documentation by Johannes Schindelin++GIT+---+Part of the linkgit:git[7] suite
@@ -0,0 +1,73 @@+#!/bin/sh++USAGE="(edit | show) [commit]"+.git-sh-setup++test-n"$3"&&usage++test-z"$1"&&usage+ACTION="$1";shift++test-z"$GIT_NOTES_REF"&&GIT_NOTES_REF="$(gitconfigcore.notesref)"+test-z"$GIT_NOTES_REF"&&GIT_NOTES_REF="refs/notes/commits"++COMMIT=$(gitrev-parse--verify--defaultHEAD"$@")||+die"Invalid commit: $@"++case"$ACTION"in+edit)+if["${GIT_NOTES_REF#refs/notes/}"="$GIT_NOTES_REF"];then+die"Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)"+fi++MSG_FILE="$GIT_DIR/new-notes-$COMMIT"+GIT_INDEX_FILE="$MSG_FILE.idx"+exportGIT_INDEX_FILE++trap'+test-f"$MSG_FILE"&&rm"$MSG_FILE"+test-f"$GIT_INDEX_FILE"&&rm"$GIT_INDEX_FILE"+'0++GIT_NOTES_REF=gitlog-1$COMMIT|sed"s/^/#/">"$MSG_FILE"++CURRENT_HEAD=$(gitshow-ref"$GIT_NOTES_REF"|cut-f1-d' ')+if[-z"$CURRENT_HEAD"];then+PARENT=+else+PARENT="-p $CURRENT_HEAD"+gitread-tree"$GIT_NOTES_REF"||die"Could not read index"+gitcat-fileblob:$COMMIT>>"$MSG_FILE"2>/dev/null+fi++core_editor="$(gitconfigcore.editor)"+${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}}"$MSG_FILE"++grep-v^#<"$MSG_FILE"|gitstripspace>"$MSG_FILE".processed+mv"$MSG_FILE".processed"$MSG_FILE"+if[-s"$MSG_FILE"];then+BLOB=$(githash-object-w"$MSG_FILE")||+die"Could not write into object database"+gitupdate-index--add--cacheinfo0644$BLOB$COMMIT||+die"Could not write index"+else+test-z"$CURRENT_HEAD"&&+die"Will not initialise with empty tree"+gitupdate-index--force-remove$COMMIT||+die"Could not update index"+fi++TREE=$(gitwrite-tree)||die"Could not write tree"+NEW_HEAD=$(echoAnnotate$COMMIT|gitcommit-tree$TREE$PARENT)||+die"Could not annotate"+gitupdate-ref-m"Annotate $COMMIT"\+"$GIT_NOTES_REF"$NEW_HEAD$CURRENT_HEAD+;;+show)+gitrev-parse-q--verify"$GIT_NOTES_REF":$COMMIT>/dev/null||+die"No note for commit $COMMIT."+gitshow"$GIT_NOTES_REF":$COMMIT+;;+*)+usage+esac
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
From: Johannes Schindelin <redacted>
git-notes have the potential of being pretty expensive, so test with
a lot of commits. A lot. So to make things cheaper, you have to
opt-in explicitely, by setting the environment variable
GIT_NOTES_TIMING_TESTS.
This patch has been improved by the following contributions:
- Junio C Hamano: tests: fix "export var=val"
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
t/t3302-notes-index-expensive.sh | 98 ++++++++++++++++++++++++++++++++++++++
1 files changed, 98 insertions(+), 0 deletions(-)
create mode 100755 t/t3302-notes-index-expensive.sh
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
The "-m" and "-F" options are already the established method
(in both git-commit and git-tag) to specify a commit/tag message
without invoking the editor. This patch teaches "git notes edit"
to respect the same options for specifying a notes message without
invoking the editor.
Multiple "-m" and/or "-F" options are concatenated as separate
paragraphs.
The patch also updates the "git notes" documentation and adds
selftests for the new functionality. Unfortunately, the added
selftests include a couple of lines with trailing whitespace
(without these the test will fail). This may cause git to warn
about "whitespace errors".
This patch has been improved by the following contributions:
- Thomas Rast: fix trailing whitespace in t3301
Signed-off-by: Johan Herland <redacted>
---
Documentation/git-notes.txt | 16 ++++++++++-
git-notes.sh | 64 +++++++++++++++++++++++++++++++++++++-----
t/t3301-notes.sh | 36 ++++++++++++++++++++++++
3 files changed, 107 insertions(+), 9 deletions(-)
@@ -33,6 +33,20 @@ show:: Show the notes for a given commit (defaults to HEAD).+OPTIONS+-------+-m <msg>::+ Use the given note message (instead of prompting).+ If multiple `-m` (or `-F`) options are given, their+ values are concatenated as separate paragraphs.++-F <file>::+ Take the note message from the given file. Use '-' to+ read the note message from the standard input.+ If multiple `-F` (or `-m`) options are given, their+ values are concatenated as separate paragraphs.++ Author ------ Written by Johannes Schindelin <johannes.schindelin@gmx.de>
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
Creating repos with 10/100/1000/10000 commits and notes takes a lot of time.
However, using git-fast-import to do the job is a lot more efficient than
using plumbing commands to do the same.
This patch decreases the overall run-time of this test on my machine from
~3 to ~1 minutes.
Signed-off-by: Johan Herland <redacted>
Acked-by: Johannes Schindelin <redacted>
---
t/t3302-notes-index-expensive.sh | 74 ++++++++++++++++++++++++--------------
1 files changed, 47 insertions(+), 27 deletions(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
Introduce a 'notemodify' subcommand of the 'commit' command. This subcommand
is similar to 'filemodify', except that no mode is supplied (all notes have
mode 0644), and the path is set to the hex SHA1 of the given "comittish".
This enables fast import of note objects along with their associated commits,
since the notes can now be named using the mark references of their
corresponding commits.
The patch also includes a test case of the added functionality.
Signed-off-by: Johan Herland <redacted>
Acked-by: Shawn O. Pearce <redacted>
---
Documentation/git-fast-import.txt | 45 +++++++++--
fast-import.c | 88 +++++++++++++++++++-
t/t9300-fast-import.sh | 166 +++++++++++++++++++++++++++++++++++++
3 files changed, 289 insertions(+), 10 deletions(-)
@@ -348,14 +348,13 @@ commit message use a 0 length data. Commit messages are free-form and are not interpreted by Git. Currently they must be encoded in UTF-8, as fast-import does not permit other encodings to be specified.-Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`-and `filedeleteall` commands+Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`,+`filedeleteall` and `notemodify` commands may be included to update the contents of the branch prior to creating the commit. These commands may be supplied in any order. However it is recommended that a `filedeleteall` command precede-all `filemodify`, `filecopy` and `filerename` commands in the same-commit, as `filedeleteall`-wipes the branch clean (see below).+all `filemodify`, `filecopy`, `filerename` and `notemodify` commands in+the same commit, as `filedeleteall` wipes the branch clean (see below). The `LF` after the command is optional (it used to be required).
@@ -604,6 +603,40 @@ more memory per active branch (less than 1 MiB for even most large projects); so frontends that can easily obtain only the affected paths for a commit are encouraged to do so.+`notemodify`+^^^^^^^^^^^^+Included in a `commit` command to add a new note (annotating a given+commit) or change the content of an existing note. This command has+two different means of specifying the content of the note.++External data format::+ The data content for the note was already supplied by a prior+ `blob` command. The frontend just needs to connect it to the+ commit that is to be annotated.+++....+ 'N' SP <dataref> SP <committish> LF+....+++Here `<dataref>` can be either a mark reference (`:<idnum>`)+set by a prior `blob` command, or a full 40-byte SHA-1 of an+existing Git blob object.++Inline data format::+ The data content for the note has not been supplied yet.+ The frontend wants to supply it as part of this modify+ command.+++....+ 'N' SP 'inline' SP <committish> LF+ data+....+++See below for a detailed description of the `data` command.++In both formats `<committish>` is any of the commit specification+expressions also accepted by `from` (see above).+ `mark` ~~~~~~ Arranges for fast-import to save a reference to the current object, allowing
@@ -22,8 +22,8 @@ Format of STDIN stream:('author'spnamesp'<'email'>'spwhenlf)?'committer'spnamesp'<'email'>'spwhenlfcommit_msg-('from'sp(ref_str|hexsha1|sha1exp_str|idnum)lf)?-('merge'sp(ref_str|hexsha1|sha1exp_str|idnum)lf)*+('from'spcommittishlf)?+('merge'spcommittishlf)*file_change*lf?;commit_msg::=data;
@@ -41,15 +41,18 @@ Format of STDIN stream:file_obm::='M'spmodesp(hexsha1|idnum)sppath_strlf;file_inm::='M'spmodesp'inline'sppath_strlfdata;+note_obm::='N'sp(hexsha1|idnum)spcommittishlf;+note_inm::='N'sp'inline'spcommittishlf+data;new_tag::='tag'sptag_strlf-'from'sp(ref_str|hexsha1|sha1exp_str|idnum)lf+'from'spcommittishlf('tagger'spnamesp'<'email'>'spwhenlf)?tag_msg;tag_msg::=data;reset_branch::='reset'spref_strlf-('from'sp(ref_str|hexsha1|sha1exp_str|idnum)lf)?+('from'spcommittishlf)?lf?;checkpoint::='checkpoint'lf
@@ -88,6 +91,7 @@ Format of STDIN stream:# stream formatting is: \, " and LF. Otherwise these values# are UTF8.#+committish::=(ref_str|hexsha1|sha1exp_str|idnum);ref_str::=ref;sha1exp_str::=sha1exp;tag_str::=tag;
@@ -2053,6 +2057,80 @@ static void file_change_cr(struct branch *b, int rename)leaf.tree);}+staticvoidnote_change_n(structbranch*b)+{+constchar*p=command_buf.buf+2;+staticstructstrbufuq=STRBUF_INIT;+structobject_entry*oe=oe;+structbranch*s;+unsignedcharsha1[20],commit_sha1[20];+uint16_tinline_data=0;++/* <dataref> or 'inline' */+if(*p==':'){+char*x;+oe=find_mark(strtoumax(p+1,&x,10));+hashcpy(sha1,oe->sha1);+p=x;+}elseif(!prefixcmp(p,"inline")){+inline_data=1;+p+=6;+}else{+if(get_sha1_hex(p,sha1))+die("Invalid SHA1: %s",command_buf.buf);+oe=find_object(sha1);+p+=40;+}+if(*p++!=' ')+die("Missing space after SHA1: %s",command_buf.buf);++/* <committish> */+s=lookup_branch(p);+if(s){+hashcpy(commit_sha1,s->sha1);+}elseif(*p==':'){+uintmax_tcommit_mark=strtoumax(p+1,NULL,10);+structobject_entry*commit_oe=find_mark(commit_mark);+if(commit_oe->type!=OBJ_COMMIT)+die("Mark :%"PRIuMAX" not a commit",commit_mark);+hashcpy(commit_sha1,commit_oe->sha1);+}elseif(!get_sha1(p,commit_sha1)){+unsignedlongsize;+char*buf=read_object_with_reference(commit_sha1,+commit_type,&size,commit_sha1);+if(!buf||size<46)+die("Not a valid commit: %s",p);+free(buf);+}else+die("Invalid ref name or SHA1 expression: %s",p);++if(inline_data){+staticstructstrbufbuf=STRBUF_INIT;++if(p!=uq.buf){+strbuf_addstr(&uq,p);+p=uq.buf;+}+read_next_command();+parse_data(&buf);+store_object(OBJ_BLOB,&buf,&last_blob,sha1,0);+}elseif(oe){+if(oe->type!=OBJ_BLOB)+die("Not a blob (actually a %s): %s",+typename(oe->type),command_buf.buf);+}else{+enumobject_typetype=sha1_object_info(sha1,NULL);+if(type<0)+die("Blob not found: %s",command_buf.buf);+if(type!=OBJ_BLOB)+die("Not a blob (actually a %s): %s",+typename(type),command_buf.buf);+}++tree_content_set(&b->branch_tree,sha1_to_hex(commit_sha1),sha1,+S_IFREG|0644,NULL);+}+staticvoidfile_change_deleteall(structbranch*b){release_tree_content_recursive(b->branch_tree.tree);
@@ -1089,6 +1089,172 @@ test_expect_success 'P: fail on blob mark in gitlink' 'test_must_failgitfast-import<input'###+### series Q (notes)+###++note1_data="Note for the first commit"+note2_data="Note for the second commit"+note3_data="Note for the third commit"++test_tick+cat>input<<INPUT_END+blob+mark:2+data<<EOF+$file2_data+EOF++commitrefs/heads/notes-test+mark:3+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+first(:3)+COMMIT++M644:2file2++blob+mark:4+data$file4_len+$file4_data+commitrefs/heads/notes-test+mark:5+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+second(:5)+COMMIT++M644:4file4++commitrefs/heads/notes-test+mark:6+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+third(:6)+COMMIT++M644inlinefile5+data<<EOF+$file5_data+EOF++M755inlinefile6+data<<EOF+$file6_data+EOF++blob+mark:7+data<<EOF+$note1_data+EOF++blob+mark:8+data<<EOF+$note2_data+EOF++commitrefs/notes/foobar+mark:9+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+notes(:9)+COMMIT++N:7:3+N:8:5+Ninline:6+data<<EOF+$note3_data+EOF++INPUT_END+test_expect_success\+'Q: commit notes'\+'gitfast-import<input&&+gitwhatchangednotes-test'+test_expect_success\+'Q: verify pack'\+'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'++commit1=$(gitrev-parsenotes-test~2)+commit2=$(gitrev-parsenotes-test^)+commit3=$(gitrev-parsenotes-test)++cat>expect<<EOF+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++first(:3)+EOF+test_expect_success\+'Q: verify first commit'\+'gitcat-filecommitnotes-test~2|sed1d>actual&&+test_cmpexpectactual'++cat>expect<<EOF+parent$commit1+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++second(:5)+EOF+test_expect_success\+'Q: verify second commit'\+'gitcat-filecommitnotes-test^|sed1d>actual&&+test_cmpexpectactual'++cat>expect<<EOF+parent$commit2+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++third(:6)+EOF+test_expect_success\+'Q: verify third commit'\+'gitcat-filecommitnotes-test|sed1d>actual&&+test_cmpexpectactual'++cat>expect<<EOF+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++notes(:9)+EOF+test_expect_success\+'Q: verify notes commit'\+'gitcat-filecommitrefs/notes/foobar|sed1d>actual&&+test_cmpexpectactual'++cat>expect.unsorted<<EOF+100644blob$commit1+100644blob$commit2+100644blob$commit3+EOF+catexpect.unsorted|sort>expect+test_expect_success\+'Q: verify notes tree'\+'gitcat-file-prefs/notes/foobar^{tree}|sed"s/ [0-9a-f]* / /">actual&&+test_cmpexpectactual'++echo"$note1_data">expect+test_expect_success\+'Q: verify note for first commit'\+'git cat-file blob refs/notes/foobar:$commit1 >actual && test_cmp expect actual'++echo"$note2_data">expect+test_expect_success\+'Q: verify note for second commit'\+'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'++echo"$note3_data">expect+test_expect_success\+'Q: verify note for third commit'\+'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'++###### series R (feature and option)###
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
This patch adds the following flags to get_commit_notes() for adjusting the
format of the produced note string:
- NOTES_SHOW_HEADER: Print "Notes:" line before the notes contents
- NOTES_INDENT: Indent notes contents by 4 spaces
Suggested-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
---
notes.c | 8 +++++---
notes.h | 5 ++++-
pretty.c | 3 ++-
3 files changed, 11 insertions(+), 5 deletions(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
From: Johannes Schindelin <redacted>
To avoid looking up each and every commit in the notes ref's tree
object, which is very expensive, speed things up by slurping the tree
object's contents into a hash_map.
The idea for the hashmap singleton is from David Reiss, initial
benchmarking by Jeff King.
Note: the implementation allows for arbitrary entries in the notes
tree object, ignoring those that do not reference a valid object. This
allows you to annotate arbitrary branches, or objects.
This patch has been improved by the following contributions:
- Junio C Hamano: fixed an obvious error in initialize_hash_map()
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
notes.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 102 insertions(+), 10 deletions(-)
@@ -4,15 +4,112 @@#include"refs.h"#include"utf8.h"#include"strbuf.h"+#include"tree-walk.h"++structentry{+unsignedcharcommit_sha1[20];+unsignedcharnotes_sha1[20];+};++structhash_map{+structentry*entries;+off_tcount,size;+};staticintinitialized;+staticstructhash_maphash_map;++staticinthash_index(structhash_map*map,constunsignedchar*sha1)+{+inti=((*(unsignedint*)sha1)%map->size);++for(;;){+unsignedchar*current=map->entries[i].commit_sha1;++if(!hashcmp(sha1,current))+returni;++if(is_null_sha1(current))+return-1-i;++if(++i==map->size)+i=0;+}+}++staticvoidadd_entry(constunsignedchar*commit_sha1,+constunsignedchar*notes_sha1)+{+intindex;++if(hash_map.count+1>hash_map.size>>1){+inti,old_size=hash_map.size;+structentry*old=hash_map.entries;++hash_map.size=old_size?old_size<<1:64;+hash_map.entries=(structentry*)+xcalloc(sizeof(structentry),hash_map.size);++for(i=0;i<old_size;i++)+if(!is_null_sha1(old[i].commit_sha1)){+index=-1-hash_index(&hash_map,+old[i].commit_sha1);+memcpy(hash_map.entries+index,old+i,+sizeof(structentry));+}+free(old);+}++index=hash_index(&hash_map,commit_sha1);+if(index<0){+index=-1-index;+hash_map.count++;+}++hashcpy(hash_map.entries[index].commit_sha1,commit_sha1);+hashcpy(hash_map.entries[index].notes_sha1,notes_sha1);+}++staticvoidinitialize_hash_map(constchar*notes_ref_name)+{+unsignedcharsha1[20],commit_sha1[20];+unsignedmode;+structtree_descdesc;+structname_entryentry;+void*buf;++if(!notes_ref_name||read_ref(notes_ref_name,commit_sha1)||+get_tree_entry(commit_sha1,"",sha1,&mode))+return;++buf=fill_tree_descriptor(&desc,sha1);+if(!buf)+die("Could not read %s for notes-index",sha1_to_hex(sha1));++while(tree_entry(&desc,&entry))+if(!get_sha1(entry.path,commit_sha1))+add_entry(commit_sha1,entry.sha1);+free(buf);+}++staticunsignedchar*lookup_notes(constunsignedchar*commit_sha1)+{+intindex;++if(!hash_map.size)+returnNULL;++index=hash_index(&hash_map,commit_sha1);+if(index<0)+returnNULL;+returnhash_map.entries[index].notes_sha1;+}voidget_commit_notes(conststructcommit*commit,structstrbuf*sb,constchar*output_encoding){staticconstcharutf8[]="utf-8";-structstrbufname=STRBUF_INIT;-unsignedcharsha1[20];+unsignedchar*sha1;char*msg,*msg_p;unsignedlonglinelen,msglen;enumobject_typetype;
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
Add selftests verifying:
- that we are able to parse notes trees with various fanout schemes
- that notes trees with conflicting fanout schemes are parsed as expected
Signed-off-by: Johan Herland <redacted>
---
t/t3303-notes-subtrees.sh | 137 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 137 insertions(+), 0 deletions(-)
create mode 100755 t/t3303-notes-subtrees.sh
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
The semantics used when parsing notes trees (with regards to fanout subtrees)
follow Dscho's proposal fairly closely:
- No concatenation/merging of notes is performed. If there are several notes
objects referencing a given commit, only one of those objects are used.
- If a notes object for a given commit is present in the "root" notes tree,
no subtrees are consulted; the object in the root tree is used directly.
- If there are more than one subtree that prefix-matches the given commit,
only the subtree with the longest matching prefix is consulted. This
means that if the given commit is e.g. "deadbeef", and the notes tree have
subtrees "de" and "dead", then the following paths in the notes tree are
searched: "deadbeef", "dead/beef". Note that "de/adbeef" is NOT searched.
- Fanout directories (subtrees) must references a whole number of bytes
from the SHA1 sum they subdivide. E.g. subtrees "dead" and "de" are
acceptable; "d" and "dea" are not.
- Multiple levels of fanout are allowed. All the above rules apply
recursively. E.g. "de/adbeef" is preferred over "de/adbe/ef", etc.
This patch changes the in-memory datastructure for holding parsed notes:
Instead of holding all note (and subtree) entries in a hash table, a
simple 16-tree structure is used instead. The tree structure consists of
16-arrays as internal nodes, and note/subtree entries as leaf nodes. The
tree is traversed by indexing subsequent nibbles of the search key until
a leaf node is encountered. If a subtree entry is encountered while
searching for a note, the subtree is unpacked into the 16-tree structure,
and the search continues into that subtree.
The new algorithm performs significantly better in the cases where only
a fraction of the notes need to be looked up (this is assumed to be the
common case for notes lookup). The new code even performs marginally
better in the worst case (where _all_ the notes are looked up).
In addition to this, comes the massive performance win associated with
organizing the notes tree according to some fanout scheme. Even a simple
2/38 fanout scheme is dramatically quicker to traverse (going from tens of
seconds to sub-second runtimes).
As for memory usage, the new code is marginally better than the old code in
the worst case, but in the case of looking up only some notes from a notes
tree with proper fanout, the new code uses only a small fraction of the
memory needed to hold the entire notes tree.
However, there is one casualty of this patch. The old notes lookup code was
able to parse notes that were associated with non-SHA1s (e.g. refs). The new
code requires the referenced object to be named by a SHA1 sum. Still, this
is not considered a major setback, since the notes infrastructure was not
originally intended to annotate objects outside the Git object database.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 248 insertions(+), 69 deletions(-)
@@ -6,103 +6,282 @@#include"strbuf.h"#include"tree-walk.h"-structentry{-unsignedcharcommit_sha1[20];-unsignedcharnotes_sha1[20];+/*+*Useanon-balancingsimple16-treestructurewithstructint_nodeas+*internalnodes,andstructleaf_nodeasleafnodes.Eachint_nodehasa+*16-arrayofpointerstoitschildren.+*Thebottom2bitsofeachpointerisusedtoidentifythepointertype+*-ptr&3==0-NULLpointer,assert(ptr==NULL)+*-ptr&3==1-pointertonextinternalnode-casttostructint_node*+*-ptr&3==2-pointertonoteentry-casttostructleaf_node*+*-ptr&3==3-pointertosubtreeentry-casttostructleaf_node*+*+*Therootnodeisastaticallyallocatedstructint_node.+*/+structint_node{+void*a[16];};-structhash_map{-structentry*entries;-off_tcount,size;+/*+*Leafnodescomeintwovariants,noteentriesandsubtreeentries,+*distinguishedbytheLSboftheleafnodepointer(seeabove).+*Asanoteentry,thekeyistheSHA1ofthereferencedcommit,andthe+*valueistheSHA1ofthenoteobject.+*Asasubtreeentry,thekeyistheprefixSHA1(w/trailingNULs)ofthe+*referencedcommit,usingthelastbyteofthekeytostorethelengthof+*theprefix.ThevalueistheSHA1ofthetreeobjectcontainingthenotes+*subtree.+*/+structleaf_node{+unsignedcharkey_sha1[20];+unsignedcharval_sha1[20];};-staticintinitialized;-staticstructhash_maphash_map;+#define PTR_TYPE_NULL 0+#define PTR_TYPE_INTERNAL 1+#define PTR_TYPE_NOTE 2+#define PTR_TYPE_SUBTREE 3-staticinthash_index(structhash_map*map,constunsignedchar*sha1)-{-inti=((*(unsignedint*)sha1)%map->size);+#define GET_PTR_TYPE(ptr) ((uintptr_t) (ptr) & 3)+#define CLR_PTR_TYPE(ptr) ((void *) ((uintptr_t) (ptr) & ~3))+#define SET_PTR_TYPE(ptr, type) ((void *) ((uintptr_t) (ptr) | (type)))-for(;;){-unsignedchar*current=map->entries[i].commit_sha1;+#define GET_NIBBLE(n, sha1) (((sha1[n >> 1]) >> ((n & 0x01) << 2)) & 0x0f)-if(!hashcmp(sha1,current))-returni;+#define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \+(memcmp(key_sha1,subtree_sha1,subtree_sha1[19]))-if(is_null_sha1(current))-return-1-i;+staticstructint_noderoot_node;-if(++i==map->size)-i=0;+staticintinitialized;++staticvoidload_subtree(structleaf_node*subtree,structint_node*node,+unsignedintn);++/*+*Tofindaleaf_node:+*1.Startattherootnode,withn=0+*2.Usethenthnibbleofthekeyasanindexintoa:+*-Ifa[n]isanint_node,recurseintothatnodeandincrementn+*-Ifaleaf_nodewithmatchingkey,returnleaf_node(assertnoteentry)+*-Ifamatchingsubtreeentry,unpackthatsubtreeentry(andremoveit);+*restartsearchatthecurrentlevel.+*-Otherwise,weendupataNULLpointer,oranon-matchingleaf_node.+*Backtrackoutoftherecursion,onelevelatatimeandchecka[0]:+*-Ifa[0]atthecurrentlevelisamatchingsubtreeentry,unpackthat+*subtreeentry(andremoveit);restartsearchatthecurrentlevel.+*/+staticstructleaf_node*note_tree_find(structint_node*tree,unsignedcharn,+constunsignedchar*key_sha1)+{+structleaf_node*l;+unsignedchari=GET_NIBBLE(n,key_sha1);+void*p=tree->a[i];++switch(GET_PTR_TYPE(p)){+casePTR_TYPE_INTERNAL:+l=note_tree_find(CLR_PTR_TYPE(p),n+1,key_sha1);+if(l)+returnl;+break;+casePTR_TYPE_NOTE:+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!hashcmp(key_sha1,l->key_sha1))+returnl;/* return note object matching given key */+break;+casePTR_TYPE_SUBTREE:+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){+/* unpack tree and resume search */+tree->a[i]=NULL;+load_subtree(l,tree,n);+free(l);+returnnote_tree_find(tree,n,key_sha1);+}+break;+casePTR_TYPE_NULL:+default:+assert(!p);+break;}++/*+*Didnotfindkeyatthis(oranylower)level.+*Checkifthere'samatchingsubtreeentryintree->a[0].+*Ifso,unpacktreeandresumesearch.+*/+p=tree->a[0];+if(GET_PTR_TYPE(p)!=PTR_TYPE_SUBTREE)+returnNULL;+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){+/* unpack tree and resume search */+tree->a[0]=NULL;+load_subtree(l,tree,n);+free(l);+returnnote_tree_find(tree,n,key_sha1);+}+returnNULL;}-staticvoidadd_entry(constunsignedchar*commit_sha1,-constunsignedchar*notes_sha1)+/*+*Toinsertaleaf_node:+*1.Startattherootnode,withn=0+*2.Usethenthnibbleofthekeyasanindexintoa:+*-Ifa[n]isNULL,storethetweakedpointerdirectlyintoa[n]+*-Ifa[n]isanint_node,recurseintothatnodeandincrementn+*-Ifa[n]isaleaf_node:+*1.Checkifthey'reequal,andhandlethat(abort?overwrite?)+*2.Createanewint_node,andstorebothleaf_nodesthere+*3.Storethenewint_nodeintoa[n].+*/+staticintnote_tree_insert(structint_node*tree,unsignedcharn,+conststructleaf_node*entry,unsignedchartype){-intindex;--if(hash_map.count+1>hash_map.size>>1){-inti,old_size=hash_map.size;-structentry*old=hash_map.entries;--hash_map.size=old_size?old_size<<1:64;-hash_map.entries=(structentry*)-xcalloc(sizeof(structentry),hash_map.size);--for(i=0;i<old_size;i++)-if(!is_null_sha1(old[i].commit_sha1)){-index=-1-hash_index(&hash_map,-old[i].commit_sha1);-memcpy(hash_map.entries+index,old+i,-sizeof(structentry));-}-free(old);+structint_node*new_node;+conststructleaf_node*l;+intret;+unsignedchari=GET_NIBBLE(n,entry->key_sha1);+void*p=tree->a[i];+assert(GET_PTR_TYPE(entry)==PTR_TYPE_NULL);+switch(GET_PTR_TYPE(p)){+casePTR_TYPE_NULL:+assert(!p);+tree->a[i]=SET_PTR_TYPE(entry,type);+return0;+casePTR_TYPE_INTERNAL:+returnnote_tree_insert(CLR_PTR_TYPE(p),n+1,entry,type);+default:+assert(GET_PTR_TYPE(p)==PTR_TYPE_NOTE||+GET_PTR_TYPE(p)==PTR_TYPE_SUBTREE);+l=(conststructleaf_node*)CLR_PTR_TYPE(p);+if(!hashcmp(entry->key_sha1,l->key_sha1))+return-1;/* abort insert on matching key */+new_node=(structint_node*)+xcalloc(sizeof(structint_node),1);+ret=note_tree_insert(new_node,n+1,+CLR_PTR_TYPE(p),GET_PTR_TYPE(p));+if(ret){+free(new_node);+return-1;+}+tree->a[i]=SET_PTR_TYPE(new_node,PTR_TYPE_INTERNAL);+returnnote_tree_insert(new_node,n+1,entry,type);}+}-index=hash_index(&hash_map,commit_sha1);-if(index<0){-index=-1-index;-hash_map.count++;+/* Free the entire notes data contained in the given tree */+staticvoidnote_tree_free(structint_node*tree)+{+unsignedinti;+for(i=0;i<16;i++){+void*p=tree->a[i];+switch(GET_PTR_TYPE(p)){+casePTR_TYPE_INTERNAL:+note_tree_free(CLR_PTR_TYPE(p));+/* fall through */+casePTR_TYPE_NOTE:+casePTR_TYPE_SUBTREE:+free(CLR_PTR_TYPE(p));+}}+}-hashcpy(hash_map.entries[index].commit_sha1,commit_sha1);-hashcpy(hash_map.entries[index].notes_sha1,notes_sha1);+/*+*ConvertapartialSHA1hexstringtothecorrespondingpartialSHA1value.+*-hex-PartialSHA1segmentinASCIIhexformat+*-hex_len-Lengthofabovesegment.Mustbemultipleof2between0and40+*-sha1-PartialSHA1valueiswrittenhere+*-sha1_len-Max#bytestostoreinsha1,Mustbe>=hex_len/2,and<20+*Returns-1onerror(invalidargumentsorinvalidSHA1(notinhexformat).+*Otherwise,returnsnumberofbyteswrittentosha1(i.e.hex_len/2).+*Padssha1withNULsuptosha1_len(notincludedinreturnedlength).+*/+staticintget_sha1_hex_segment(constchar*hex,unsignedinthex_len,+unsignedchar*sha1,unsignedintsha1_len)+{+unsignedinti,len=hex_len>>1;+if(hex_len%2!=0||len>sha1_len)+return-1;+for(i=0;i<len;i++){+unsignedintval=(hexval(hex[0])<<4)|hexval(hex[1]);+if(val&~0xff)+return-1;+*sha1++=val;+hex+=2;+}+for(;i<sha1_len;i++)+*sha1++=0;+returnlen;}-staticvoidinitialize_hash_map(constchar*notes_ref_name)+staticvoidload_subtree(structleaf_node*subtree,structint_node*node,+unsignedintn){-unsignedcharsha1[20],commit_sha1[20];-unsignedmode;+unsignedcharcommit_sha1[20];+unsignedintprefix_len;+intstatus;+void*buf;structtree_descdesc;structname_entryentry;-void*buf;++buf=fill_tree_descriptor(&desc,subtree->val_sha1);+if(!buf)+die("Could not read %s for notes-index",+sha1_to_hex(subtree->val_sha1));++prefix_len=subtree->key_sha1[19];+assert(prefix_len*2>=n);+memcpy(commit_sha1,subtree->key_sha1,prefix_len);+while(tree_entry(&desc,&entry)){+intlen=get_sha1_hex_segment(entry.path,strlen(entry.path),+commit_sha1+prefix_len,20-prefix_len);+if(len<0)+continue;/* entry.path is not a SHA1 sum. Skip */+len+=prefix_len;++/*+*IfcommitSHA1iscomplete(len==20),assumenoteobject+*IfcommitSHA1isincomplete(len<20),assumenotesubtree+*/+if(len<=20){+unsignedchartype=PTR_TYPE_NOTE;+structleaf_node*l=(structleaf_node*)+xcalloc(sizeof(structleaf_node),1);+hashcpy(l->key_sha1,commit_sha1);+hashcpy(l->val_sha1,entry.sha1);+if(len<20){+l->key_sha1[19]=(unsignedchar)len;+type=PTR_TYPE_SUBTREE;+}+status=note_tree_insert(node,n,l,type);+assert(!status);+}+}+free(buf);+}++staticvoidinitialize_notes(constchar*notes_ref_name)+{+unsignedcharsha1[20],commit_sha1[20];+unsignedmode;+structleaf_noderoot_tree;if(!notes_ref_name||read_ref(notes_ref_name,commit_sha1)||get_tree_entry(commit_sha1,"",sha1,&mode))return;-buf=fill_tree_descriptor(&desc,sha1);-if(!buf)-die("Could not read %s for notes-index",sha1_to_hex(sha1));--while(tree_entry(&desc,&entry))-if(!get_sha1(entry.path,commit_sha1))-add_entry(commit_sha1,entry.sha1);-free(buf);+hashclr(root_tree.key_sha1);+hashcpy(root_tree.val_sha1,sha1);+load_subtree(&root_tree,&root_node,0);}staticunsignedchar*lookup_notes(constunsignedchar*commit_sha1){-intindex;--if(!hash_map.size)-returnNULL;--index=hash_index(&hash_map,commit_sha1);-if(index<0)-returnNULL;-returnhash_map.entries[index].notes_sha1;+structleaf_node*found=note_tree_find(&root_node,0,commit_sha1);+if(found)+returnfound->val_sha1;+returnNULL;}voidget_commit_notes(conststructcommit*commit,structstrbuf*sb,
@@ -134,4 +134,68 @@ test_expect_success 'verify notes in 4/36-fanout overriding 2/38-fanout' 'verify test_expect_success'test notes in 2/38-fanout overriding 2/2/36-fanout''test_preferred "s|^..|&/|" "s|^\(..\)\(..\)|\1/\2/|"' test_expect_success'verify notes in 2/38-fanout overriding 2/2/36-fanout''verify_notes'+test_date_based(){+(+start_note_commit&&+nr=$number_of_commits&&+gitlog--format="%H %ct"refs/heads/master|+whilereadsha1date_t;do+date=$(date-u-d"@$date_t"+"$1")+note_path="$date/$(echo"$sha1"|sed"$2")"+cat<<INPUT_END&&+M100644inline$note_path+data<<EOF+noteforcommit#$nr+EOF++INPUT_END++nr=$(($nr-1))+done+)|+gitfast-import--quiet+}++test_expect_success'test notes in y/40-fanout''test_date_based "y%Y" ""'+test_expect_success'verify notes in y/40-fanout''verify_notes'++test_expect_success'test notes in y/2/38-fanout''test_date_based "y%Y" "s|^..|&/|"'+test_expect_success'verify notes in y/2/38-fanout''verify_notes'++test_expect_success'test notes in ym/40-fanout''test_date_based "y%Ym%m" ""'+test_expect_success'verify notes in ym/40-fanout''verify_notes'++test_expect_success'test notes in ym/2/38-fanout''test_date_based "y%Ym%m" "s|^..|&/|"'+test_expect_success'verify notes in ym/2/38-fanout''verify_notes'++test_expect_success'test notes in ymd/40-fanout''test_date_based "y%Ym%md%d" ""'+test_expect_success'verify notes in ymd/40-fanout''verify_notes'++test_expect_success'test notes in ymd/2/38-fanout''test_date_based "y%Ym%md%d" "s|^..|&/|"'+test_expect_success'verify notes in ymd/2/38-fanout''verify_notes'++test_expect_success'test notes in y/m/40-fanout''test_date_based "y%Y/m%m" ""'+test_expect_success'verify notes in y/m/40-fanout''verify_notes'++test_expect_success'test notes in y/m/2/38-fanout''test_date_based "y%Y/m%m" "s|^..|&/|"'+test_expect_success'verify notes in y/m/2/38-fanout''verify_notes'++test_expect_success'test notes in y/md/40-fanout''test_date_based "y%Y/m%md%d" ""'+test_expect_success'verify notes in y/md/40-fanout''verify_notes'++test_expect_success'test notes in y/md/2/38-fanout''test_date_based "y%Y/m%md%d" "s|^..|&/|"'+test_expect_success'verify notes in y/md/2/38-fanout''verify_notes'++test_expect_success'test notes in ym/d/40-fanout''test_date_based "y%Ym%m/d%d" ""'+test_expect_success'verify notes in ym/d/40-fanout''verify_notes'++test_expect_success'test notes in ym/d/2/38-fanout''test_date_based "y%Ym%m/d%d" "s|^..|&/|"'+test_expect_success'verify notes in ym/d/2/38-fanout''verify_notes'++test_expect_success'test notes in y/m/d/40-fanout''test_date_based "y%Y/m%m/d%d" ""'+test_expect_success'verify notes in y/m/d/40-fanout''verify_notes'++test_expect_success'test notes in y/m/d/2/38-fanout''test_date_based "y%Y/m%m/d%d" "s|^..|&/|"'+test_expect_success'verify notes in y/m/d/2/38-fanout''verify_notes'+ test_done
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
This is a major expansion of the notes lookup code to allow for variations
in the notes tree organization. The variations allowed include mixing fanout
schemes based on the commit dates of the annotated commits (aka. date-based
fanout) with fanout schemes based on the SHA1 of the annotated commits (aka.
SHA1-based fanout).
Using date-based fanout in the notes tree structure enables considerable
speedup in the notes lookup process, since notes are almost always looked up
sequentially in the (reverse) chronological order of their associated commits.
Furthermore, organizing notes in a way that allow (near) sequential lookup,
enables us to decrease memory consumption both by lazily loading parts of the
notes tree structure on-demand, and freeing parts of the notes structure that
are unlikely to be used again soon.
The new flexible organization of the notes tree changes the rules for valid
note tree entries. The new rules are as follows:
1. Note objects are named by the SHA1 of the commit they annotate, possibly
split across several SHA1-based fanout levels (this is the same as is
implemented earlier in this series).
2. Note entries are located within zero or more date-based fanout levels.
3. Date-based fanout schemes may use the year, month and day values of the
associated commit's timestamp. The values must be prefixed by 'y', 'm'
and 'd' (respectively) in the notes tree.
4. The date-based components can be combined in one fanout level, or split
across multiple fanout levels. Individual components may not be split
across multiple fanout levels.
5. The year/month/date values must be specified in that order, and month or
date values may not occur without the preceding year or month value.
6. All entries of a tree object in the notes tree structure must follow the
same scheme used at that level.
Thus, the following example note entries are all valid locations for a note
annotating commit 123456789abcdef0123456789abcdef0123456789 at 2009-09-01:
- 123456789abcdef0123456789abcdef0123456789
- 12/3456789abcdef0123456789abcdef0123456789
- 1234/56789abcdef0123456789abcdef0123456789
- 12/34/56789abcdef0123456789abcdef0123456789
- 1234/5678/9abcdef0123456789abcdef0123456789
- 1234/56/78/9abcdef0123456789abcdef0123456789
- y2009/123456789abcdef0123456789abcdef0123456789
- y2009/m09/12/3456789abcdef0123456789abcdef0123456789
- y2009/m09/d01/123456789abcdef0123456789abcdef0123456789
- y2009m09/12/34/56789abcdef0123456789abcdef0123456789
- y2009m09/d01/1234/567/89abcdef0123456789abcdef0123456789
- y2009/m09d01/12/34/56/78/9abcdef0123456789abcdef0123456789
- y2009m09d01/123456789abcdef0123456789abcdef0123456789
Conversely, the following example note entries are all invalid:
- 1/23456789abcdef0123456789abcdef0123456789 (violates #1)
- 123/456789abcdef0123456789abcdef0123456789 (violates #1)
- 12/345/6789abcdef0123456789abcdef0123456789 (violates #1)
- y2009123456789abcdef0123456789abcdef0123456789 (violates #2)
- 2009/09/01/123456789abcdef0123456789abcdef0123456789 (violates #3)
- y20/09/m09/12/3456789abcdef0123456789abcdef0123456789 (violates #4)
- y20/09m09/d01/123456789abcdef0123456789abcdef0123456789 (violates #4)
- y2009m/09/12/34/56789abcdef0123456789abcdef0123456789 (violates #4)
- y2009/d01/1234/5678/9abcdef0123456789abcdef0123456789 (violates #5)
- m09/y2009/d01/12/34/56/78/9abcdef0123456789abcdef0123456789 (violates #5)
From rule #6, we see that the following example notes tree is valid:
- y2009m09/0123456789abcdef0123456789abcdef012345678
- y2009m09/123456789abcdef0123456789abcdef0123456789
- y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
- y2008m01/d31/34/56789abcdef0123456789abcdef0123456789ab
- y2008m01/d16/4567/89abcdef0123456789abcdef0123456789abc
- y2008m01/d16/5678/9abcdef0123456789abcdef0123456789abcd
Conversely the following structure is invalid (violates rule #6):
- y2009m09/0123456789abcdef0123456789abcdef012345678
- y2009m09/12/3456789abcdef0123456789abcdef0123456789
- y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
- y2008m01/34/56789abcdef0123456789abcdef0123456789ab
- y2008m01/d16/45/6789abcdef0123456789abcdef0123456789abc
- y2008/m01d16/5678/9abcdef0123456789abcdef0123456789abcd
The flexibility added by this patch adds considerable complexity to the notes
tree parser, but the runtime and memory usage is not significantly affected
(except for the effects introduced by the chosen notes tree structure).
Internally, the 16-tree data structure introduced in earlier patches is still
used to hold the SHA1-based fanout levels and the note entries themselves.
However, this patch adds a hierarchical date-based linked-list structure
around the 16-tree structure that mirrors the fanout scheme used in the
actual notes tree.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 403 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 364 insertions(+), 39 deletions(-)
@@ -17,9 +81,45 @@*-ptr&3==3-pointertosubtreeentry-casttostructleaf_node***Therootnodeisastaticallyallocatedstructint_node.+*+*Inordertoallowdate-basedfanoutschemesinadditiontotheoriginal+*SHA1-basedfanoutschemes,weneedtooverloadthisstructure,asfollows:+*Ifthefirstpointerinthe16-arrayis~0(i.e.0xffffffffon32-bit+*systemsand0xffffffffffffffffon64-bitsystems),thentheint_nodeisNOT+*tobeinterpretedasa16-arrayofchildnodepointers.Rather,theint_node+*nowrepresentsaperiod-basednodewiththefollowingproperties:+*-Thenodehasapointertoa"child"nodeoftypestructint_node,whichis+*EITHERa"regular"int_nodeobjectrepresentingtherootnodeofa16-tree+*structureholdingnotesassociatedwithcommitswithtimestampswithin+*thattimeperiod,ORanotherperiod-basedint_noderepresentingsome+*subdivisionofthetimeperiod.+*-Thenodealsohasapointertoa"previous"period-basedint_node,which+*representstheprevioustimeperiodforwhichthereexistnoteobjects.+*-Thenodehasapointertoa"parent"node,whichistheperiod-based+*int_nodethathasthisint_nodeasoneofitschildren.Thisisneeded+*whentraversingthedate-basedint_nodeslookingforaperiodmatchingthe+*givencommit.Fortop-levelobjects,thisissettoNULL.+*-ThenodestorestheSHA1sumofthetreeobjectthatrepresentsitschild+*(withinthenotestreestructure).Thus,wekeepareferencetothechild+*structurethatwithoutnecessarilyallocatingthechildnode(and+*underlyingstructure).+*-Finally,thenodehasaperiodstring,whichindicatesthetimeperiodof+*thenotescontainedwithin,typicallyoftheform"YYYY","YYYY-MM"or+*"YYYY-MM-DD",dependingonthegranularityofthecorresponding+*period-basedentriesinthenotestreestructure.*/structint_node{-void*a[16];+union{+void*a[16];+struct{+void*magic;/* ~0 "enables" this part of the union */+structint_node*child;+structint_node*prev;+structint_node*parent;+unsignedchartree_sha1[20];+charperiod[11];/* Enough to hold "YYYY-MM-DD" */+};+};};/*
@@ -94,7 +200,8 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){/* unpack tree and resume search */tree->a[i]=NULL;-load_subtree(l,tree,n);+load_subtree(l->val_sha1,l->key_sha1,l->key_sha1[19],+tree,NULL,(int)n);free(l);returnnote_tree_find(tree,n,key_sha1);}
@@ -117,7 +224,8 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){/* unpack tree and resume search */tree->a[0]=NULL;-load_subtree(l,tree,n);+load_subtree(l->val_sha1,l->key_sha1,l->key_sha1[19],tree,+NULL,(int)n);free(l);returnnote_tree_find(tree,n,key_sha1);}
@@ -173,16 +281,28 @@ static int note_tree_insert(struct int_node *tree, unsigned char n,/* Free the entire notes data contained in the given tree */staticvoidnote_tree_free(structint_node*tree){-unsignedinti;-for(i=0;i<16;i++){-void*p=tree->a[i];-switch(GET_PTR_TYPE(p)){-casePTR_TYPE_INTERNAL:-note_tree_free(CLR_PTR_TYPE(p));-/* fall through */-casePTR_TYPE_NOTE:-casePTR_TYPE_SUBTREE:-free(CLR_PTR_TYPE(p));+if(tree->magic==(void*)~0){+if(tree->prev){+note_tree_free(tree->prev);+free(tree->prev);+}+if(tree->child){+note_tree_free(tree->child);+free(tree->child);+}+}+else{+unsignedinti;+for(i=0;i<16;i++){+void*p=tree->a[i];+switch(GET_PTR_TYPE(p)){+casePTR_TYPE_INTERNAL:+note_tree_free(CLR_PTR_TYPE(p));+/* fall through */+casePTR_TYPE_NOTE:+casePTR_TYPE_SUBTREE:+free(CLR_PTR_TYPE(p));+}}}}
@@ -215,29 +335,139 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,returnlen;}-staticvoidload_subtree(structleaf_node*subtree,structint_node*node,-unsignedintn)+/*+*Parseyear/month/datestrings,andgeneratethecorrespondingperiodstring+*forthegivenpathentry:+*-prefixmustfollowoneoftheseforms:"","YYYY","YYYY-MM"+*-pathshouldfollowoneoftheseforms:"yYYYY","yYYYYmMM","yYYYYmMMdDD",+*"mMMdDD","mMM"or"dDD"+*Theresultingstring(whichfollowstheform"YYYY","YYYY-MM"or+*"YYYY-MM-DD")isreturnedasastaticstring.Ifpathisnotvalidinthe+*given(prefix)context,NULLisreturned.+*/+staticconstchar*parse_period(constchar*prefix,unsignedintprefix_len,+constchar*path,unsignedintpath_len)+{+staticcharresult[11];+charexpect_type;/* y/m/d for year/month/day-based fanout */+unsignedintexpect_len,value;+char*endptr,*target=result;++switch(prefix_len){+case0:+/* No prefix, expect year-based fanout in path */+expect_type='y';+expect_len=4;+break;+case4:+/* Year in prefix, expect month-based fanout in path */+expect_type='m';+expect_len=2;+break;+case7:+/* "YYYY-MM" in prefix, expect day-based fanout in path */+expect_type='d';+expect_len=2;+break;+default:+die("Date-based notes tree loading invoked with invalid "+"prefix '%.*s'",prefix_len,prefix);+}++if(path[0]!=expect_type){+warning("Unexpected entry path in date-based notes tree: '%s' "+"(skipping)",path);+returnNULL;+}+value=(unsignedint)strtoul(path+1,&endptr,10);+switch(expect_type){+case'y':+if(value<1969||value>=3000){+warning("Invalid year value in date-based notes tree:"+" '%s' (skipping)",path);+returnNULL;+}+break;+case'm':+if(value<1||value>12){+warning("Invalid month value in date-based notes tree:"+" '%s' (skipping)",path);+returnNULL;+}+break;+case'd':+if(value<1||value>31){+warning("Invalid day value in date-based notes tree:"+" '%s' (skipping)",path);+returnNULL;+}+break;+}++if(prefix==result){+target=result+prefix_len;+prefix=NULL;+prefix_len=0;+}+prefix_len=snprintf(target,11,"%.*s%s%0*u",prefix_len,prefix,+expect_len==2?"-":"",expect_len,value);+prefix_len+=target-result;+assert(prefix_len<11);++if(*endptr)/* there are more components in this path */+returnparse_period(result,prefix_len,endptr,+path_len-(endptr-path));+returnresult;+}++staticvoidload_date_subtree(structtree_desc*tree_desc,+constchar*prefix,unsignedintprefix_len,+structint_node*node,structint_node*parent)+{+structname_entryentry;+structint_node*cur_node=NULL;+structint_node*new_node;++while(tree_entry(tree_desc,&entry)){+constchar*period=parse_period(+prefix,prefix_len,entry.path,strlen(entry.path));+if(!period)+continue;+if(tree_desc->size)/* this is not the last tree entry */+new_node=(structint_node*)+xmalloc(sizeof(structint_node));+else/* this is the last entry, store directly into node */+new_node=node;++new_node->magic=(void*)~0;+new_node->child=NULL;+new_node->prev=cur_node;+new_node->parent=parent;+hashcpy(new_node->tree_sha1,entry.sha1);+strcpy(new_node->period,period);+cur_node=new_node;+}+assert(!cur_node||cur_node==node);+}++staticvoidload_sha1_subtree(structtree_desc*tree_desc,+constunsignedchar*prefix,unsignedintprefix_len,+structint_node*node,unsignedcharn){unsignedcharcommit_sha1[20];-unsignedintprefix_len;intstatus;-void*buf;-structtree_descdesc;structname_entryentry;-buf=fill_tree_descriptor(&desc,subtree->val_sha1);-if(!buf)-die("Could not read %s for notes-index",-sha1_to_hex(subtree->val_sha1));--prefix_len=subtree->key_sha1[19];assert(prefix_len*2>=n);-memcpy(commit_sha1,subtree->key_sha1,prefix_len);-while(tree_entry(&desc,&entry)){+memcpy(commit_sha1,prefix,prefix_len);+while(tree_entry(tree_desc,&entry)){intlen=get_sha1_hex_segment(entry.path,strlen(entry.path),commit_sha1+prefix_len,20-prefix_len);-if(len<0)+if(len<0){+warning("Invalid value in notes tree: '%s' (skipping)",+entry.path);continue;/* entry.path is not a SHA1 sum. Skip */+}len+=prefix_len;/*
@@ -258,6 +488,42 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,assert(!status);}}+}++staticvoidload_subtree(constunsignedchar*sha1,+constunsignedchar*prefix,unsignedintprefix_len,+structint_node*node,structint_node*parent,intn)+{+void*buf;+structtree_descdesc;++buf=fill_tree_descriptor(&desc,sha1);+if(!buf)+die("Could not read notes subtree at %s",sha1_to_hex(sha1));+/*+*Afterfill_tree_descriptor(),wecanpeekatthefirsttreeentry+*indesc.entry.+*/+switch(desc.entry.path[0]){+case'd':+if(strlen(desc.entry.path)!=3)+break;+/* fall-through */+case'm':+case'y':+/* path cannot be a SHA1 fragment */+load_date_subtree(&desc,(constchar*)prefix,prefix_len,+node,parent);+free(buf);+return;+}+if(n<0){+/* Arriving from a date-based subtree; reset prefix */+n=0;+prefix=NULL;+prefix_len=0;+}+load_sha1_subtree(&desc,prefix,prefix_len,node,n);free(buf);}
@@ -265,23 +531,81 @@ static void initialize_notes(const char *notes_ref_name){unsignedcharsha1[20],commit_sha1[20];unsignedmode;-structleaf_noderoot_tree;if(!notes_ref_name||read_ref(notes_ref_name,commit_sha1)||get_tree_entry(commit_sha1,"",sha1,&mode))return;-hashclr(root_tree.key_sha1);-hashcpy(root_tree.val_sha1,sha1);-load_subtree(&root_tree,&root_node,0);+load_subtree(sha1,NULL,0,&root_node,NULL,0);+cur_node=&root_node;}-staticunsignedchar*lookup_notes(constunsignedchar*commit_sha1)+staticunsignedchar*lookup_notes(conststructcommit*commit){-structleaf_node*found=note_tree_find(&root_node,0,commit_sha1);-if(found)-returnfound->val_sha1;-returnNULL;+structint_node*node=cur_node,*seen_node=cur_node;+structleaf_node*found;+constchar*short_date;++if(!node)+returnNULL;++/* Convert commit->date to YYYY-MM-DD format */+short_date=show_date(commit->date,0,DATE_SHORT);++while(node->magic==(void*)~0){/* date-based node */+intcmp=SUBTREE_DATE_PREFIXCMP(short_date,node->period);+if(cmp==0){+/* Search inside child node */+if(!node->child){+/* Must unpack child node first */+node->child=(structint_node*)+xcalloc(sizeof(structint_node),1);+load_subtree(node->tree_sha1,+(constunsignedchar*)node->period,+strlen(node->period),node->child,+node,-1);+}+seen_node=node;+node=node->child;+}+elseif(cmp>0){+/* Search in past node */+if(node->prev)+node=node->prev;+else+node=node->parent;+}+else{+/* Search in future node */+if(!node->parent){+/* Restart from root_node */+seen_node=node;+node=&root_node;+}+else+node=node->parent;+}+if(!node||node==seen_node){+/* We've been here before, give up search */+returnNULL;+}+}+while(cur_node&&+SUBTREE_DATE_PREFIXCMP(cur_node->period,seen_node->period)<0)+{+/*+*We'reabouttomovecur_nodebackwardsinhistory.Weare+*unlikelytoneedthiscur_nodeinthefuture,sofree()it.+*/+note_tree_free(cur_node->child);+cur_node->child=NULL;+cur_node=cur_node->parent;+}+cur_node=seen_node;++/* Drill down further with SHA1-based lookup */+found=note_tree_find(node,0,commit->object.sha1);+returnfound?found->val_sha1:NULL;}voidget_commit_notes(conststructcommit*commit,structstrbuf*sb,
@@ -123,6 +123,7 @@ The placeholders are: - '%s': subject - '%f': sanitized subject line, suitable for a filename - '%b': body+- '%N': commit notes - '%Cred': switch color to red - '%Cgreen': switch color to green - '%Cblue': switch color to blue
@@ -702,6 +702,10 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,case'd':format_decoration(sb,commit);return1;+case'N':+get_commit_notes(commit,sb,git_log_output_encoding?+git_log_output_encoding:git_commit_encoding,0);+return1;}/* For the rest we have to parse the commit header. */
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
There's no need to be rude to memory-concious callers...
Signed-off-by: Johan Herland <redacted>
---
notes.c | 7 +++++++
notes.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:22
On Tuesday 08 September 2009, Johan Herland wrote:
I have some performance numbers that I will send in a separate email.
Ok, here we go:
Test scenario:
Linux kernel repo with 157118 commits, 1 note per commit, with notes
organized into various fanout schemes.
Hardware is Intel Core 2 Quad with 4GB RAM.
The tests were done on the following algorithms:
- "before": This is the state of the notes code after applying patches 1-9.
It uses the original notes-in-hash-map implementation, and does not grok
any fanout scheme.
- "16tree": This is the state of the notes code after applying patch 10.
It uses the 16-tree data structure that parses the SHA-1 based fanout
schemes.
- "flexible": This is the state of the notes code after applying the entire
patch series. This code parses a variety of date- and SHA1-based fanout
schemes.
Furthermore, the following notes tree structures were tested:
- "no-notes": Testing without any notes at all. This is only present as a
baseline, and to verify that the notes code does not negatively affect
performance when not in use.
- "no-fanout": All notes stored directly inside the root notes tree object.
- "2_38": All notes stored in a SHA1-based 2/38 fanout scheme.
- "2_2_36": All notes stored in a SHA1-based 2/2/36 fanout scheme.
- "ym": Notes are organized within "yYYYYmMM"-named subtrees, where "YYYY"
and "MM" are the year and month (respectively) from the annotated commit's
commit date.
- "ym_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme within
the "yYYYYmMM"-named subtrees.
- "ymd": Notes are organized within "yYYYYmMMdDD"-named subtrees.
- "ymd_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme within
the "yYYYYmMMdDD"-named subtrees.
- "y_m": Notes are organized within two-level "yYYYY/mMM" subtrees.
- "y_m_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme within
the "yYYYY/mMM"-named subtrees.
- "y_m_d": Notes are organized within three-level "yYYYY/mMM/dDD" subtrees.
- "y_m_d_2_38": Same as above, but with a 2/38 SHA1-based fanout scheme
within the "yYYYY/mMM/dDD"-named subtrees.
Here are the runtime numbers, the first column shows the runtime for 100
repetitions of "git log -n10" (which we assume to be a common use case),
and the second column shows the runtime from a single run of
"git log --all" (which is somewhat closer to a worst case).
Algorithm / Notes tree git log -n10 (x100) git log --all
------------------------------------------------------------
before / no-notes 4.78s 63.90s
before / no-fanout 56.85s 65.69s
16tree / no-notes 4.77s 64.18s
16tree / no-fanout 30.35s 65.39s
16tree / 2_38 5.57s 65.42s
16tree / 2_2_36 5.19s 65.76s
flexible / no-notes 4.78s 63.91s
flexible / no-fanout 30.34s 65.57s
flexible / 2_38 5.57s 65.46s
flexible / 2_2_36 5.18s 65.72s
flexible / ym 5.13s 65.66s
flexible / ym_2_38 5.08s 65.63s
flexible / ymd 5.30s 65.45s
flexible / ymd_2_38 5.29s 65.90s
flexible / y_m 5.11s 65.72s
flexible / y_m_2_38 5.08s 65.67s
flexible / y_m_d 5.06s 65.50s
flexible / y_m_d_2_38 5.07s 65.79s
Finally, I have also looked at the memory consumption of the various
algorithms and fanout schemes:
The memory usage was measured by calculating the #bytes dynamically
allocated for the notes data structure, and printing the current
usage every time get_commit_notes() was called during a complete run
of "git log --all".
The results are attached as two gnuplot graphs, one with regular
axes, and one with logarithmic axes.
Have fun! :)
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
Yet another iteration of the 'git notes' feature. Rebased on top of
'next':
- Patches 1-9 are unchanged from (patches 1-7, 11-12 of) the last
iteration.
- Patch 10 teaches the notes code to free its data structures on
request.
- Patch 11 introduces the 16-tree notes lookup code that handles
SHA1-based
fanout schemes. This is pretty much unchanged from patch 8 in the
previous
iteration.
- Patch 12 adds selftests that verify correct parsing of notes trees
with
various SHA1-based fanouts.
- Patch 13 introduces a flexible parser for a variety of date-based
and
SHA1-based fanout schemes. This is the interesting part, as far as
this
iteration is concerned.
- Patch 14 adds selftests that verify correct parsing of notes trees
with
various date-based fanouts.
Note that the series does not yet include code for _writing_ notes
into a
suitably structured notes tree. That will be done in a later
iteration.
I have some performance numbers that I will send in a separate email.
Hi Johan,
I've been following this series with some interest, and am curious
why notes need to be stored in a separate data structure from regular
objects. Note that I'm not questioning the design (and certainly would
not want to, this late in the process), rather I'd like to learn
about the reasons.
I've wondered about this as well in the context of refs, reflog and
git config. In a completely unified model, every change to the
repository (except for the index, pack indices and working directory)
would be a commit of the .git/ directory (again excluding indices).
One of the advantages (besides allowing configuration management
of the repository itself in addition to its contents) would be that
no locking is ever required.
This would be just an implementation detail without necessarily
affecting the user interface other than direct inspection/modification
of the .git directory, which is a similar to the move to packed refs.
Again, I'm not proposing to change anything, just wondering about
design rationale.
-Geert
From: Michael J Gruber <hidden> Date: 2016-06-15 22:47:23
Geert Bosch venit, vidit, dixit 10.09.2009 16:00:
On Sep 7, 2009, at 22:26, Johan Herland wrote:
...
Hi Johan,
I've been following this series with some interest, and am curious
why notes need to be stored in a separate data structure from regular
objects. Note that I'm not questioning the design (and certainly would
It's not separate, that's the point. They're stored as objects in trees,
just like anything else. The discussion about the structure is about how
to organize the tree structure, not actual subdirectories under .git/.
not want to, this late in the process), rather I'd like to learn
about the reasons.
I've wondered about this as well in the context of refs, reflog and
git config. In a completely unified model, every change to the
repository (except for the index, pack indices and working directory)
would be a commit of the .git/ directory (again excluding indices).
One of the advantages (besides allowing configuration management
of the repository itself in addition to its contents) would be that
no locking is ever required.
...and one of the disadvantages that you're not in control of your
config any more, if you pull from upstream. config and reflog are
something inherently private and local. The reflog does not even make
sense other than in a local (per repo) context.
For the config, one may think up a solution where parts of config are
shared (by storing them as objects and referencing them) and git asks
you before changing anything on pull/fetch. In a sense git submodule
does that already.
Cheers,
Michael
On Sep 10, 2009, at 10:09, Michael J Gruber wrote:
It's not separate, that's the point. They're stored as objects in
trees,
just like anything else. The discussion about the structure is about
how
to organize the tree structure, not actual subdirectories under .git/.
Ok, I have been pondering this back and forth, and I'm not sure what to
think. It seems allowing (not mandating) date-based fanout gives a slight
runtime advantage if used correctly, but I'm not sure the slight runtime
improvement is worth the added code complexity and worse maintainability.
I'm starting to lean against SHA1-based fanout being "good enough".
But when we look at the memory consumption, it's clear that SHA1-based
fanout loses out (because you cannot throw away subtrees without fear that
they will be needed again soon). Then again, memory consumption has not been
the major focus of the git project, and 14 MB (for holding all ~157000 notes
in the kernel repo example) is not excessive for an average desktop
computer.
Shawn, do you have any additional defence for the date-based fanout? Are
there untested reasonable scenarios that would show the benefits of date-
based fanout? How does the plan for notes usage in your code-review thingy
compare to my test scenario?
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
From: Shawn O. Pearce <hidden> Date: 2016-06-15 22:47:23
Johan Herland [off-list ref] wrote:
Shawn, do you have any additional defence for the date-based fanout?
No.
The only defense I have for it is "it sounds like a nice theory
given access patterns", and the note about memory usage you made,
but which I clipped to keep this email shorter. :-)
It was only a theory I tossed out there in a back-seat-driver
sort of way. Your results show my hunch was correct, it may help.
But they also say it may not help enough to justify the complexity,
so I now agree with you that SHA-1 fan out may be good enough.
Are
there untested reasonable scenarios that would show the benefits of date-
based fanout?
I don't think there are, your tests were pretty good at covering
things.
How does the plan for notes usage in your code-review thingy
compare to my test scenario?
I think your tests may still have been too low in volume, 115k notes
isn't a lot. Based on the distributions I was looking at before,
I could be seeing a growth of >100k notes/year. Ask me again in
5 years if 115k notes is a lot. :-)
But we all know that SHA-1 distributes data quite well, so the SHA-1
fan-out may just need to change from 2_38 to 2_2_2_34 (or something)
to handle that larger volume.
--
Shawn.
From: Johan Herland <hidden> Date: 2016-06-15 22:47:23
On Saturday 12 September 2009, Shawn O. Pearce wrote:
Johan Herland [off-list ref] wrote:
quoted
Shawn, do you have any additional defence for the date-based fanout?
No.
The only defense I have for it is "it sounds like a nice theory
given access patterns", and the note about memory usage you made,
but which I clipped to keep this email shorter. :-)
It was only a theory I tossed out there in a back-seat-driver
sort of way. Your results show my hunch was correct, it may help.
But they also say it may not help enough to justify the complexity,
so I now agree with you that SHA-1 fan out may be good enough.
Ok, so I guess we can drop the flexible part of notes code. Junio: Feel free
to drop the two last patches from the jh/notes series.
quoted
How does the plan for notes usage in your code-review thingy
compare to my test scenario?
I think your tests may still have been too low in volume, 115k notes
isn't a lot. Based on the distributions I was looking at before,
I could be seeing a growth of >100k notes/year. Ask me again in
5 years if 115k notes is a lot. :-)
But we all know that SHA-1 distributes data quite well, so the SHA-1
fan-out may just need to change from 2_38 to 2_2_2_34 (or something)
to handle that larger volume.
Yes, I expect that the optimal number of entries per tree level is ~256, so
if we add an upper threshold at ~300 (where we start using another fanout
level), and a lower threshold at ~200 (where we consolidate subtrees and put
all into this level), the (still-to-be-written) writing part of the notes
code should automatically adjust the notes tree to the optimal layout.
With those assumptions, and a growth of 100k notes/year, a 2/2/36 fanout
should last you ~150 years, and a 2/2/2/34 fanout should be enough for the
next ~40,000 years... ;)
Have fun! :)
...Johan
--
Johan Herland, [off-list ref]
www.herland.net