The last post was five months ago [1]. It's probably time for a resend
in case I'm hit by a bus. Numbers are in 17/19, saving about 40% time
on "git status". Details are in 02/19 and 06/19. Still on the table:
- index-helper series [2] probably helps save about 10-15% in total
- watchman support to lower the numbers even more, but not for Windows
I still need to see how watchman can be made on top of this.
[1] http://thread.gmane.org/gmane.comp.version-control.git/248306
[2] http://thread.gmane.org/gmane.comp.version-control.git/254314/focus=254318
--
2.1.0.rc0.78.gc0d8480
This is not used anywhere yet. But the goal is to compare quickly if a
.gitignore file has changed when we have the SHA-1 of both old (cached
somewhere) and new (from index or a tree) versions.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
dir.h | 5 +++++
2 files changed, 48 insertions(+), 7 deletions(-)
@@ -547,7 +559,7 @@ int add_excludes_from_file_to_list(const char *fname,if(0<=fd)close(fd);if(!check_index||-(buf=read_skip_worktree_file_from_index(fname,&size))==NULL)+(buf=read_skip_worktree_file_from_index(fname,&size,ss))==NULL)return-1;if(size==0){free(buf);
@@ -560,6 +572,10 @@ int add_excludes_from_file_to_list(const char *fname,}else{size=xsize_t(st.st_size);if(size==0){+if(ss){+fill_stat_data(&ss->stat,&st);+hashcpy(ss->sha1,EMPTY_BLOB_SHA1_BIN);+}close(fd);return0;}
@@ -571,6 +587,19 @@ int add_excludes_from_file_to_list(const char *fname,}buf[size++]='\n';close(fd);+if(ss){+intpos;+if(ss_valid&&!match_stat_data(&ss->stat,&st))+;/* no content change, ss->sha1 still good */+elseif(check_index&&+(pos=cache_name_pos(fname,strlen(fname)))>=0&&+!ce_stage(active_cache[pos])&&+ce_uptodate(active_cache[pos]))+hashcpy(ss->sha1,active_cache[pos]->sha1);+else+hash_sha1_file(buf,size,"blob",ss->sha1);+fill_stat_data(&ss->stat,&st);+}}el->filebuf=buf;
@@ -589,6 +618,13 @@ int add_excludes_from_file_to_list(const char *fname,return0;}+intadd_excludes_from_file_to_list(constchar*fname,constchar*base,+intbaselen,structexclude_list*el,+intcheck_index)+{+returnadd_excludes(fname,base,baselen,el,check_index,NULL,0);+}+structexclude_list*add_exclude_list(structdir_struct*dir,intgroup_type,constchar*src){
The idea is if we can capture all input and (non-rescursive) output of
read_directory_recursive(), and can verify later that all the input is
the same, then the second r_d_r() should produce the same output as in
the first run.
The requirement for this to work is stat info of a directory MUST
change if an entry is added to or removed from that directory (and
should not change often otherwise). If your OS and filesytem do not
meet this requirement, untracked cache is not for you. Most file
systems on *nix should be fine. On Windows, NTFS is fine while FAT may
be not [1] even though FAT on Linux seems to be fine.
The list of input of r_d_r() is in the big comment block in dir.h. In
short, the output of a directory (not counting subdirs) mainly depends
on stat info of the directory in question, all .gitignore leading to
it and the check_only flag when r_d_r() is called recursively. This
patch records all this info (and the output) as r_d_r() runs.
Two hash_sha1_file() are required for $GIT_DIR/info/exclude and
core.excludesfile unless their stat data matches. hash_sha1_file() is
only needed when .gitignore files in the worktree are modified,
otherwise their SHA-1 in index is used (see the previous patch).
We could store stat data for .gitignore files so we don't have to
rehash them if their content is different from index, but I think
.gitignore files are rarely modified, so not worth extra cache data
(and hashing penalty read-cache.c:verify_hdr(), as we will be storing
this as an index extension).
The implication is, if you change .gitignore, you better add it to the
index soon or you lose all the benefit of untracked cache because a
modified .gitignore invalidates all subdirs recursively. This is
especially bad for .gitignore at root.
This cached output is about untracked files only, not ignored files
because the number of tracked files is usually small, so small cache
overhead, while the number of ignored files could go really high
(e.g. *.o files mixing with source code).
[1] "Description of NTFS date and time stamps for files and folders"
http://support.microsoft.com/kb/299648
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
dir.h | 60 +++++++++++++++++++++++++++
2 files changed, 189 insertions(+), 21 deletions(-)
@@ -642,14 +689,20 @@ struct exclude_list *add_exclude_list(struct dir_struct *dir,/**Usedtosetupcore.excludesfileand.git/info/excludelists.*/-voidadd_excludes_from_file(structdir_struct*dir,constchar*fname)+staticvoidadd_excludes_from_file_1(structdir_struct*dir,constchar*fname,+structsha1_stat*ss,intss_valid){structexclude_list*el;el=add_exclude_list(dir,EXC_FILE,fname);-if(add_excludes_from_file_to_list(fname,"",0,el,0)<0)+if(add_excludes(fname,"",0,el,0,ss,ss_valid)<0)die("cannot use %s as an exclude file",fname);}+voidadd_excludes_from_file(structdir_struct*dir,constchar*fname)+{+add_excludes_from_file_1(dir,fname,NULL,0);+}+intmatch_basename(constchar*basename,intbasenamelen,constchar*pattern,intprefix,intpatternlen,intflags)
@@ -861,9 +915,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)/* Read from the parent directories and push them down. */current=stk?stk->baselen:-1;strbuf_setlen(&dir->basebuf,current<0?0:current);+if(dir->untracked)+untracked=stk?stk->ucd:dir->untracked->root;+else+untracked=NULL;+while(current<baselen){structexclude_stack*stk=xcalloc(1,sizeof(*stk));constchar*cp;+structsha1_statss;if(current<0){cp=base;
@@ -873,10 +933,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)if(!cp)die("oops in prep_exclude");cp++;+untracked=+lookup_untracked(dir->untracked,untracked,+base+current,+cp-base-current);}stk->prev=dir->exclude_stack;stk->baselen=cp-base;stk->exclude_ix=group->nr;+stk->ucd=untracked;el=add_exclude_list(dir,EXC_DIRS,NULL);strbuf_add(&dir->basebuf,base+current,stk->baselen-current);assert(stk->baselen==dir->basebuf.len);
@@ -899,6 +964,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)}/* Try to read per-directory file */+hashclr(ss.sha1);if(dir->exclude_per_dir){/**dir->basebufgetsreusedbythetraversal,butwe
@@ -1343,24 +1426,36 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,if(!fdir)gotoout;+if(untracked)+untracked->check_only=!!check_only;+while((de=readdir(fdir))!=NULL){/* check how the file or directory should be treated */-state=treat_path(dir,de,&path,baselen,simplify);+state=treat_path(dir,untracked,de,&path,baselen,simplify);+if(state>dir_state)dir_state=state;/* recurse into subdir if instructed by treat_path */if(state==path_recurse){-subdir_state=read_directory_recursive(dir,path.buf,-path.len,check_only,simplify);+structuntracked_cache_dir*ud;+ud=lookup_untracked(dir->untracked,untracked,+path.buf+baselen,+path.len-baselen);+subdir_state=+read_directory_recursive(dir,path.buf,path.len,+ud,check_only,simplify);if(subdir_state>dir_state)dir_state=subdir_state;}if(check_only){/* abort early if maximum state has been reached */-if(dir_state==path_untracked)+if(dir_state==path_untracked){+if(untracked)+add_untracked(untracked,path.buf+baselen);break;+}/* skip the dir_add_* part */continue;}
@@ -1456,7 +1554,7 @@ static int treat_leading_path(struct dir_struct *dir,break;if(simplify_away(sb.buf,sb.len,simplify))break;-if(treat_one_path(dir,&sb,simplify,+if(treat_one_path(dir,NULL,&sb,simplify,DT_DIR,NULL)==path_none)break;/* do not recurse into it */if(len<=baselen){
@@ -1496,7 +1594,9 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru*/simplify=create_simplify(pathspec?pathspec->_raw:NULL);if(!len||treat_leading_path(dir,path,len,simplify))-read_directory_recursive(dir,path,len,0,simplify);+read_directory_recursive(dir,path,len,+dir->untracked?dir->untracked->root:NULL,+0,simplify);free_simplify(simplify);qsort(dir->entries,dir->nr,sizeof(structdir_entry*),cmp_name);qsort(dir->ignored,dir->ignored_nr,sizeof(structdir_entry*),cmp_name);
@@ -66,6 +66,7 @@ struct exclude_stack {structexclude_stack*prev;/* the struct exclude_stack for the parent directory */intbaselen;intexclude_ix;/* index of exclude_list within EXC_DIRS exclude_list_group */+structuntracked_cache_dir*ucd;};structexclude_list_group{
@@ -78,6 +79,60 @@ struct sha1_stat {unsignedcharsha1[20];};+/*+*Untrackedcache+*+*Thefollowinginputsaresufficienttodeterminewhatfilesina+*directoryareexcluded:+*+*-Thelistoffilesanddirectoriesofthedirectioninquestion+*-The$GIT_DIR/index+*-dir_structflags+*-Thecontentof$GIT_DIR/info/exclude+*-Thecontentofcore.excludesfile+*-Thecontent(orthelack)of.gitignoreofallparentdirectories+*from$GIT_WORK_TREE+*-Thecheck_onlyflaginread_directory_recursive(for+*DIR_HIDE_EMPTY_DIRECTORIES)+*+*Thefirstinputcanbecheckedusingdirectorymtime.Inmany+*filesystems,directorymtime(stat_datafield)isupdatedwhenits+*filesordirectsubdirsareaddedorremoved.+*+*Thesecondonecanbehookedfromcache_tree_invalidate_path().+*Wheneverafile(orasubmodule)isaddedorremovedfroma+*directory,weinvalidatethatdirectory.+*+*Theremaininginputsareeasy,theirSHA-1couldbeusedtoverify+*theircontents(exclude_sha1[],info_exclude_sha1[]and+*excludes_file_sha1[])+*/+structuntracked_cache_dir{+structuntracked_cache_dir**dirs;+char**untracked;+/* null SHA-1 means this directory does not have .gitignore */+unsignedcharexclude_sha1[20];+structstat_datastat_data;+unsignedintcheck_only:1;+unsignedintuntracked_nr:29;+unsignedintuntracked_alloc,dirs_nr,dirs_alloc;+charname[1];+};++structuntracked_cache{+structsha1_statss_info_exclude;+structsha1_statss_excludes_file;+constchar*exclude_per_dir;+/*+*dir_struct#flagsmustmatchdir_flagsortheuntracked+*cacheisignored.+*/+unsigneddir_flags;+structuntracked_cache_dir*root;+/* Statistics */+intdir_created;+};+structdir_struct{intnr,alloc;intignored_nr,ignored_alloc;
@@ -125,6 +180,11 @@ struct dir_struct {structexclude_stack*exclude_stack;structexclude*exclude;structstrbufbasebuf;++/* Enable untracked file cache if set */+structuntracked_cache*untracked;+structsha1_statss_info_exclude;+structsha1_statss_excludes_file;};/*
Make sure the starting conditions and all global exclude files are
good to go. If not, either disable untracked cache completely, or wipe
out the cache and start fresh.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
dir.h | 4 +++
2 files changed, 114 insertions(+), 3 deletions(-)
@@ -1567,9 +1591,87 @@ static int treat_leading_path(struct dir_struct *dir,returnrc;}+staticstructuntracked_cache_dir*validate_untracked_cache(structdir_struct*dir,+intbase_len,+conststructpathspec*pathspec)+{+structuntracked_cache_dir*root;++if(!dir->untracked)+returnNULL;++/*+*Weonlysupport$GIT_DIR/info/excludeandcore.excludesfile+*astheglobalignorerulefiles.Anyotheradditions+*(e.g.fromcommandline)invalidatethecache.This+*conditionalsocatchesrunningsetup_standard_excludes()+*beforesettingdir->untracked!+*/+if(dir->unmanaged_exclude_files)+returnNULL;++/*+*Optimizeforthemainusecaseonly:whole-treegit+*status.Moreworkinvolvedintreat_leading_path()ifwe+*usecacheonjustasubsetoftheworktree.pathspec+*supportcouldmakethematterevenworse.+*/+if(base_len||(pathspec&&pathspec->nr))+returnNULL;++/* Different set of flags may produce different results */+if(dir->flags!=dir->untracked->dir_flags||+/*+*Seetreat_directory(),caseindex_nonexistent.Without+*thisflag,wemayneedtoalsocache.gitfilecontent+*fortheresolve_gitlink_ref()call,whichwedon't.+*/+!(dir->flags&DIR_SHOW_OTHER_DIRECTORIES)||+/* We don't support collecting ignore files */+(dir->flags&(DIR_SHOW_IGNORED|DIR_SHOW_IGNORED_TOO|+DIR_COLLECT_IGNORED)))+returnNULL;++/*+*Ifweuse.gitignoreinthecacheandnowyouchangeitto+*.gitexclude,everythingwillgowrong.+*/+if(dir->exclude_per_dir!=dir->untracked->exclude_per_dir&&+strcmp(dir->exclude_per_dir,dir->untracked->exclude_per_dir))+returnNULL;++/*+*EXC_CMDLisnotconsideredinthecache.Ifpeoplesetit,+*skipthecache.+*/+if(dir->exclude_list_group[EXC_CMDL].nr)+returnNULL;++if(!dir->untracked->root){+constintlen=sizeof(*dir->untracked->root);+dir->untracked->root=xmalloc(len);+memset(dir->untracked->root,0,len);+}++/* Validate $GIT_DIR/info/exclude and core.excludesfile */+root=dir->untracked->root;+if(hashcmp(dir->ss_info_exclude.sha1,+dir->untracked->ss_info_exclude.sha1)){+invalidate_gitignore(dir->untracked,root);+dir->untracked->ss_info_exclude=dir->ss_info_exclude;+}+if(hashcmp(dir->ss_excludes_file.sha1,+dir->untracked->ss_excludes_file.sha1)){+invalidate_gitignore(dir->untracked,root);+dir->untracked->ss_excludes_file=dir->ss_excludes_file;+}+returnroot;+}+intread_directory(structdir_struct*dir,constchar*path,intlen,conststructpathspec*pathspec){structpath_simplify*simplify;+structuntracked_cache_dir*untracked;/**Checkoutcreate_simplify()
@@ -1593,10 +1695,15 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru*create_simplify().*/simplify=create_simplify(pathspec?pathspec->_raw:NULL);+untracked=validate_untracked_cache(dir,len,pathspec);+if(!untracked)+/*+*makesureuntrackedcachecodepathisdisabled,+*e.g.prep_exclude()+*/+dir->untracked=NULL;if(!len||treat_leading_path(dir,path,len,simplify))-read_directory_recursive(dir,path,len,-dir->untracked?dir->untracked->root:NULL,-0,simplify);+read_directory_recursive(dir,path,len,untracked,0,simplify);free_simplify(simplify);qsort(dir->entries,dir->nr,sizeof(structdir_entry*),cmp_name);qsort(dir->ignored,dir->ignored_nr,sizeof(structdir_entry*),cmp_name);
@@ -114,6 +114,8 @@ struct untracked_cache_dir {unsignedcharexclude_sha1[20];structstat_datastat_data;unsignedintcheck_only:1;+/* all data in this struct are good */+unsignedintvalid:1;unsignedintuntracked_nr:29;unsignedintuntracked_alloc,dirs_nr,dirs_alloc;charname[1];
It's easy to see that if an existing .gitignore changes, its SHA-1
would be different and invalidate_gitignore() is called.
If .gitignore is removed, add_excludes() will treat it like an empty
.gitignore, which again should invalidate the cached directory data.
if .gitignore is added, lookup_untracked() already fills initial
.gitignore SHA-1 as "empty file", so again invalidate_gitignore() is
called.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
This allows us to feed different info to read_directory_recursive()
based on untracked cache in the next patch.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 8 deletions(-)
@@ -1454,23 +1495,21 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,structuntracked_cache_dir*untracked,intcheck_only,conststructpath_simplify*simplify){-DIR*fdir;+structcached_dircdir;enumpath_treatmentstate,subdir_state,dir_state=path_none;-structdirent*de;structstrbufpath=STRBUF_INIT;strbuf_add(&path,base,baselen);-fdir=opendir(path.len?path.buf:".");-if(!fdir)+if(open_cached_dir(&cdir,dir,untracked,&path,check_only))gotoout;if(untracked)untracked->check_only=!!check_only;-while((de=readdir(fdir))!=NULL){+while(!read_cached_dir(&cdir)){/* check how the file or directory should be treated */-state=treat_path(dir,untracked,de,&path,baselen,simplify);+state=treat_path(dir,untracked,&cdir,&path,baselen,simplify);if(state>dir_state)dir_state=state;
The main readdir loop in read_directory_recursive() is replaced with a
new one that checks if cached results of a directory is still valid.
If a file is added or removed from the index, the containing directory
is invalidated (but not its subdirs). If directory's mtime is changed,
the same happens. If a .gitignore is updated, the containing directory
and all subdirs are invalidated recursively. If dir_struct#flags or
other conditions change, the cache is ignored.
If a directory is invalidated, we opendir/readdir/closedir and run the
exclude machinery on that directory listing as usual. If untracked
cache is also enabled, we'll update the cache along the way. If a
directory is validated, we simply pull the untracked listing out from
the cache. The cache also records the list of direct subdirs that we
have to recurse in. Fully excluded directories are seen as "untracked
files".
In the best case when no dirs are invalidated, read_directory()
becomes a series of
stat(dir), open(.gitignore), fstat(), read(), close() and optionally
hash_sha1_file()
For comparison, standard read_directory() is a sequence of
opendir(), readdir(), open(.gitignore), fstat(), read(), close(), the
expensive last_exclude_matching() and closedir().
We already try not to open(.gitignore) if we know it does not exist,
so open/fstat/read/close sequence does not apply to every
directory. The sequence could be reduced further, as noted in
prep_exclude() in another patch. So in theory, the entire best-case
read_directory sequence could be reduced to a series of stat() and
nothing else.
This is not a silver bullet approach. When you compile a C file, for
example, the old .o file is removed and a new one with the same name
created, effectively invalidating the containing directory's cache
(but not its subdirectories). If your build process touches every
directory, this cache adds extra overhead for nothing, so it's a good
idea to separate generated files from tracked files.. Editors may use
the same strategy for saving files. And of course you're out of luck
running your repo on an unsupported filesytem and/or operating system.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
dir.h | 2 ++
2 files changed, 123 insertions(+), 2 deletions(-)
@@ -1418,6 +1431,41 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,}}+staticenumpath_treatmenttreat_path_fast(structdir_struct*dir,+structuntracked_cache_dir*untracked,+structcached_dir*cdir,+structstrbuf*path,+intbaselen,+conststructpath_simplify*simplify)+{+if(!cdir->ucd){+strbuf_setlen(path,baselen);+strbuf_addstr(path,cdir->file);+returnpath_untracked;+}+strbuf_setlen(path,baselen);+strbuf_addstr(path,cdir->ucd->name);+/* treat_one_path() does this before it calls treat_directory() */+if(path->buf[path->len-1]!='/')+strbuf_addch(path,'/');+if(cdir->ucd->check_only)+/*+*check_onlyissetasaresultoftreat_directory()getting+*toitsbottom.Verifyagainthesamesetofdirectories+*withcheck_onlyset.+*/+returnread_directory_recursive(dir,path->buf,path->len,++cdir->ucd,1,simplify);+/*+*Wegetpath_recurseinthefirstrunwhen+*directory_exists_in_index()returnsindex_nonexistent.We+*aresurethatnewchangesintheindexdoesnotimpactthe+*outcome.Returnnow.+*/+returnpath_recurse;+}+staticenumpath_treatmenttreat_path(structdir_struct*dir,structuntracked_cache_dir*untracked,structcached_dir*cdir,
@@ -1530,7 +1649,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,if(check_only){/* abort early if maximum state has been reached */if(dir_state==path_untracked){-if(untracked)+if(cdir.fdir)add_untracked(untracked,path.buf+baselen);break;}
If we redo this thing in a functional style, we would have one struct
untracked_dir as input tree and another as output. The input is used
for verification. The output is a brand new tree, reflecting current
worktree.
But that means recreate a lot of dir nodes even if a lot could be
shared between input and output trees in good cases. So we go with the
messy but efficient way, combining both input and output trees into
one. We need a way to know which node in this combined tree belongs to
the output. This is the purpose of this "recurse" flag.
"valid" bit can't be used for this because it's about data of the node
except the subdirs. When we invalidate a directory, we want to keep
cached data of the subdirs intact even though we don't really know
what subdir still exists (yet). Then we check worktree to see what
actual subdir remains on disk. Those will have 'recurse' bit set
again. If cached data for those are still valid, we may be able to
avoid computing exclude files for them. Those subdirs that are deleted
will have 'recurse' remained clear and their 'valid' bits do not
matter.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 14 +++++++++++++-
dir.h | 3 ++-
2 files changed, 15 insertions(+), 2 deletions(-)
@@ -1838,6 +1847,9 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dinvalidate_gitignore(dir->untracked,root);dir->untracked->ss_excludes_file=dir->ss_excludes_file;}++/* Make sure this directory is not dropped out at saving phase */+root->recurse=1;returnroot;}
@@ -113,8 +113,9 @@ struct untracked_cache_dir {/* null SHA-1 means this directory does not have .gitignore */unsignedcharexclude_sha1[20];structstat_datastat_data;+unsignedintrecurse:1;unsignedintcheck_only:1;-/* all data in this struct are good */+/* all data except 'dirs' in this struct are good */unsignedintvalid:1;unsignedintuntracked_nr:29;unsignedintuntracked_alloc,dirs_nr,dirs_alloc;
This cuts down a signficant number of open(.gitignore) because most
directories usually don't have .gitignore files.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
@@ -1359,6 +1359,9 @@ static int read_index_extension(struct index_state *istate,if(read_link_extension(istate,data,sz))return-1;break;+caseCACHE_EXT_UNTRACKED:+istate->untracked=read_untracked_extension(data,sz);+break;default:if(*ext<'A'||'Z'<*ext)returnerror("index uses %.4s extension, which we do not understand",
@@ -1650,6 +1653,8 @@ int discard_index(struct index_state *istate)istate->cache=NULL;istate->cache_alloc=0;discard_split_index(istate);+free_untracked_cache(istate->untracked);+istate->untracked=NULL;return0;}
Ideally we should implement untracked_cache_remove_from_index() and
untracked_cache_add_to_index() so that they update untracked cache
right away instead of invalidating it and wait for read_directory()
next time to deal with it. But that may need some more work in
unpack-trees.c. So stay simple as the first step.
The new call in add_index_entry_with_check() may look strange because
new calls usually stay close to cache_tree_invalidate_path(). We do it
a bit later than c_t_i_p() in this function because if it's about
replacing the entry with the same name, we don't care (but cache-tree
does).
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 31 +++++++++++++++++++++++++++++++
dir.h | 4 ++++
read-cache.c | 4 ++++
unpack-trees.c | 7 +++++--
4 files changed, 44 insertions(+), 2 deletions(-)
@@ -271,20 +271,26 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)returnchanged;}-staticintis_racy_timestamp(conststructindex_state*istate,-conststructcache_entry*ce)+staticintis_racy_stat(conststructindex_state*istate,+conststructstat_data*sd){-return(!S_ISGITLINK(ce->ce_mode)&&-istate->timestamp.sec&&+return(istate->timestamp.sec&&#ifdef USE_NSEC/* nanosecond timestamped files can also be racy! */-(istate->timestamp.sec<ce->ce_stat_data.sd_mtime.sec||-(istate->timestamp.sec==ce->ce_stat_data.sd_mtime.sec&&-istate->timestamp.nsec<=ce->ce_stat_data.sd_mtime.nsec))+(istate->timestamp.sec<sd->sd_mtime.sec||+(istate->timestamp.sec==sd->sd_mtime.sec&&+istate->timestamp.nsec<=sd->sd_mtime.nsec))#else-istate->timestamp.sec<=ce->ce_stat_data.sd_mtime.sec+istate->timestamp.sec<=sd->sd_mtime.sec#endif-);+);+}++staticintis_racy_timestamp(conststructindex_state*istate,+conststructcache_entry*ce)+{+return(!S_ISGITLINK(ce->ce_mode)&&+is_racy_stat(istate,&ce->ce_stat_data));}intie_match_stat(conststructindex_state*istate,
When a directory is updated within the same second that its timestamp
is last saved, we cannot realize the directory has been updated by
checking timestamps. Assume the worst (something is update). See
29e4d36 (Racy GIT - 2005-12-20) for more information.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 2 ++
dir.c | 5 +++--
read-cache.c | 8 ++++++++
3 files changed, 13 insertions(+), 2 deletions(-)
@@ -678,7 +678,8 @@ static int add_excludes(const char *fname, const char *base, int baselen,close(fd);if(ss){intpos;-if(ss_valid&&!match_stat_data(&ss->stat,&st))+if(ss_valid&&+!match_stat_data_racy(&the_index,&ss->stat,&st));/* no content change, ss->sha1 still good */elseif(check_index&&(pos=cache_name_pos(fname,strlen(fname)))>=0&&
@@ -1533,7 +1534,7 @@ static int valid_cached_dir(struct dir_struct *dir,return0;}if(!untracked->valid||-match_stat_data(&untracked->stat_data,&st)){+match_stat_data_racy(&the_index,&untracked->stat_data,&st)){if(untracked->valid)invalidate_directory(dir->untracked,untracked);fill_stat_data(&untracked->stat_data,&st);
This could be used to verify correct behavior in tests
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
@@ -172,6 +172,14 @@ may not support it yet. the shared index file. This mode is designed for very large indexes that take a signficant amount of time to read or write.+--untracked-cache::+--no-untracked-cache::+ Enable or disable untracked cache extension. This could speed+ up for commands that involve determining untracked files such+ as `git status`. The underlying operating system and file+ system must change `st_mtime` field of a directory if files+ are added or deleted in that directory.+ \--:: Do not interpret any more arguments as options.
@@ -741,6 +741,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,intcmd_update_index(intargc,constchar**argv,constchar*prefix){intnewfd,entries,has_errors=0,line_termination='\n';+intuntracked_cache=-1;intread_from_stdin=0;intprefix_length=prefix?strlen(prefix):0;intpreferred_index_format=0;
@@ -832,6 +833,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)N_("write index in this format")),OPT_BOOL(0,"split-index",&split_index,N_("enable or disable split index")),+OPT_BOOL(0,"untracked-cache",&untracked_cache,+N_("enable/disable untracked cache")),OPT_END()};
@@ -938,6 +941,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)the_index.split_index=NULL;the_index.cache_changed|=SOMETHING_CHANGED;}+if(untracked_cache>0&&!the_index.untracked){+structuntracked_cache*uc;++uc=xcalloc(1,sizeof(*uc));+uc->exclude_per_dir=".gitignore";+/* should be the same flags used by git-status */+uc->dir_flags=DIR_SHOW_OTHER_DIRECTORIES|DIR_HIDE_EMPTY_DIRECTORIES;+the_index.untracked=uc;+the_index.cache_changed|=SOMETHING_CHANGED;+}elseif(!untracked_cache&&the_index.untracked){+the_index.untracked=NULL;+the_index.cache_changed|=SOMETHING_CHANGED;+}if(active_cache_changed){if(newfd<0){
@@ -180,6 +180,12 @@ may not support it yet. system must change `st_mtime` field of a directory if files are added or deleted in that directory.+--force-untracked-cache::+ For safety, `--untracked-cache` performs tests on the working+ directory to make sure untracked cache can be used. These+ tests can take a few seconds. `--force-untracked-cache` can be+ used to skip the tests.+ \--:: Do not interpret any more arguments as options.
@@ -48,6 +48,145 @@ static void report(const char *fmt, ...)va_end(vp);}+staticvoidremove_test_directory(void)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,"dir-mtime-test");+remove_dir_recursively(&sb,0);+strbuf_release(&sb);+}++staticvoidxmkdir(constchar*path)+{+if(mkdir(path,0700))+die_errno(_("failed to create directory %s"),path);+}++staticintxstat(constchar*path,structstat*st)+{+if(stat(path,st))+die_errno(_("failed to stat %s"),path);+return0;+}++staticintcreate_file(constchar*path)+{+intfd=open(path,O_CREAT|O_RDWR,0644);+if(fd<0)+die_errno(_("failed to create file %s"),path);+returnfd;+}++staticvoidxunlink(constchar*path)+{+if(unlink(path))+die_errno(_("failed to delete file %s"),path);+}++staticvoidxrmdir(constchar*path)+{+if(rmdir(path))+die_errno(_("failed to delete directory %s"),path);+}++staticvoidavoid_racy(void)+{+/*+*notuseifwecouldusleep(10)ifUSE_NSECisdefined.The+*fieldnseccouldbethere,buttheOScouldchooseto+*ignoreit?+*/+sleep(1);+}++staticinttest_if_untracked_cache_is_supported(void)+{+structstatst;+structstat_database;+intfd;++fprintf(stderr,_("Testing "));+xmkdir("dir-mtime-test");+atexit(remove_test_directory);+xstat("dir-mtime-test",&st);+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+fd=create_file("dir-mtime-test/newfile");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not "+"change after adding a new file"));+return0;+}+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+xmkdir("dir-mtime-test/new-dir");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not change "+"after adding a new directory"));+return0;+}+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+write_or_die(fd,"data",4);+close(fd);+xstat("dir-mtime-test",&st);+if(match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info changes "+"after updating a file"));+return0;+}+fputc('.',stderr);++avoid_racy();+close(create_file("dir-mtime-test/new-dir/new"));+xstat("dir-mtime-test",&st);+if(match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info changes after "+"adding a file inside subdirectory"));+return0;+}+fputc('.',stderr);++avoid_racy();+xunlink("dir-mtime-test/newfile");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not "+"change after deleting a file"));+return0;+}+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+xunlink("dir-mtime-test/new-dir/new");+xrmdir("dir-mtime-test/new-dir");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not "+"change after deleting a directory"));+return0;+}++xrmdir("dir-mtime-test");+fprintf_ln(stderr,_(" OK"));+return1;+}+staticintmark_ce_flags(constchar*path,intflag,intmark){intnamelen=strlen(path);
@@ -835,6 +974,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)N_("enable or disable split index")),OPT_BOOL(0,"untracked-cache",&untracked_cache,N_("enable/disable untracked cache")),+OPT_SET_INT(0,"force-untracked-cache",&untracked_cache,+N_("enable untracked cache without testing the filesystem"),2),OPT_END()};
@@ -944,6 +1085,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)if(untracked_cache>0&&!the_index.untracked){structuntracked_cache*uc;+if(untracked_cache<2){+setup_work_tree();+if(!test_if_untracked_cache_is_supported())+return1;+}uc=xcalloc(1,sizeof(*uc));uc->exclude_per_dir=".gitignore";/* should be the same flags used by git-status */
@@ -0,0 +1,353 @@+#!/bin/sh++test_description='test untracked cache'++../test-lib.sh++avoid_racy(){+sleep1+}++gitupdate-index--untracked-cache+# It's fine if git update-index returns an error code other than one,+# it'll be caught in the first test.+iftest$?-eq1;then+skip_all='This system does not support untracked cache'+test_done+fi++test_expect_success'setup''+gitinitworktree&&+cdworktree&&+mkdirdonedtwodthree&&+touchonetwothreedone/onedtwo/twodthree/three&&+gitaddonetwodone/one&&+:>.git/info/exclude&&+gitupdate-index--untracked-cache+'++test_expect_success'untracked cache is empty''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude0000000000000000000000000000000000000000+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+EOF+test_cmp../expect../actual+'++cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??dthree/+??dtwo/+??three+EOF++cat>../dump.expect<<EOF&&+info/excludee69de29bb2d1d6434b8b29ae775ad8c2e48c5391+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/0000000000000000000000000000000000000000recursevalid+dthree/+dtwo/+three+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+three+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF++test_expect_success'status first time (empty cache)''+avoid_racy&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:3+gitignoreinvalidation:1+directoryinvalidation:0+opendir:4+EOF+test_cmp../trace.expect../trace+'++test_expect_success'untracked cache after first status''+test-dump-untracked-cache>../actual&&+test_cmp../dump.expect../actual+'++test_expect_success'status second time (fully populated cache)''+avoid_racy&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:0+opendir:0+EOF+test_cmp../trace.expect../trace+'++test_expect_success'untracked cache after second status''+test-dump-untracked-cache>../actual&&+test_cmp../dump.expect../actual+'++test_expect_success'modify in root directory, one dir invalidation''+avoid_racy&&+:>four&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??dthree/+??dtwo/+??four+??three+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:1+opendir:1+EOF+test_cmp../trace.expect../trace++'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/excludee69de29bb2d1d6434b8b29ae775ad8c2e48c5391+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/0000000000000000000000000000000000000000recursevalid+dthree/+dtwo/+four+three+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+three+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'new .gitignore invalidates recursively''+avoid_racy&&+echofour>.gitignore&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??.gitignore+??dthree/+??dtwo/+??three+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:1+directoryinvalidation:1+opendir:4+EOF+test_cmp../trace.expect../trace++'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/excludee69de29bb2d1d6434b8b29ae775ad8c2e48c5391+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dthree/+dtwo/+three+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+three+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'new info/exclude invalidates everything''+avoid_racy&&+echothree>>.git/info/exclude&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??.gitignore+??dtwo/+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:1+directoryinvalidation:0+opendir:4+EOF+test_cmp../trace.expect../trace+'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dtwo/+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'move two from tracked to untracked''+gitrm--cachedtwo&&+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecurse+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'status after the move''+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+??.gitignore+??dtwo/+??two+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:0+opendir:1+EOF+test_cmp../trace.expect../trace+'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dtwo/+two+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'move two from untracked to tracked''+gitaddtwo&&+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecurse+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'status after the move''+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??.gitignore+??dtwo/+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:0+opendir:1+EOF+test_cmp../trace.expect../trace+'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dtwo/+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_done
update_index_if_able() is moved down so that the updated untracked
cache could be written out.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/commit.c | 5 +++--
wt-status.c | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
On 2014-10-27 13.10, Nguyễn Thái Ngọc Duy wrote:
[]
Nice serious, I can imagine to test & benchmark it (so I assume there is a branch
on github or so ?)
Another thing:
Can we switch the feature off?
It could be nice to benchmark with and without the cache on the command line,
and besides that we may want to switch it on or off, depending on the file system.
I think this can be easily done when reading and writing the index file.
(But may cost a config variable, core.dirmtime ??)
To my knowledge there is support for the mtime in SAMBA (and probably NFS),
but I can help to find out more.
quoted hunk
diff --git a/dir.c b/dir.c
+static int add_excludes(const char *fname, const char *base, int baselen,
+ struct exclude_list *el, int check_index,
+ struct sha1_stat *ss, int ss_valid)
Cosmetic question: does it make sense to write
struct sha1_stat *sha1_stat
or
struct sha1_stat *s_stat
+struct untracked_cache_dir {
+ struct untracked_cache_dir **dirs;
+ char **untracked;
+ /* null SHA-1 means this directory does not have .gitignore */
+ unsigned char exclude_sha1[20];
+ struct stat_data stat_data;
+ unsigned int check_only : 1;
+ unsigned int untracked_nr : 29;
+ unsigned int untracked_alloc, dirs_nr, dirs_alloc;
+ char name[1];
+};
Are we utilizing the CPU and the L2 cache in a good way ?
I would consider to re-arrange, according to the following rule:
- 64 bit ints first (we do not have any)
- pointers (may be 64 or 32 bits)
- structs
- ints
- chars
And then we have the question why untracked_nr gets 29 bits,
and check_only one bit, which means we have 2 bits spare ?
From what I know from CPUs and compilers it could be slighty better to give
32 bits to untracked_nr and either 1 bit to check_only,
or simply make check_only a char.
Or do I miss something ?
+ struct untracked_cache_dir **dirs;
+ char **untracked;
+ /* null SHA-1 means this directory does not have .gitignore */
+ struct stat_data stat_data;
+ unsigned char exclude_sha1[20];
+ unsigned int check_only : 1;
+ unsigned int untracked_nr : 29;
+ unsigned int untracked_alloc, dirs_nr, dirs_alloc;
+ char name[1];
On 2014-10-27 13.10, Nguyễn Thái Ngọc Duy wrote:
[]
+static void xmkdir(const char *path)
+{
+ if (mkdir(path, 0700))
+ die_errno(_("failed to create directory %s"), path);
+}
Does it makes sense to ignore EINTR and do a "retry" ?
Another question is if the function could be called mkdir_or_die() instead?
I realized that there are 2 families of xfunc() in wrapper.c, some die, some retry.
Would it make sense to move the strbuf_setlen(path,baseline) above the
conditional since it is common to both cases, or are they conceptually
distinct enough that it is clearer to duplicate the function call for
each case?
+ strbuf_addstr(path, cdir->ucd->name);
+ /* treat_one_path() does this before it calls treat_directory() */
+ if (path->buf[path->len - 1] != '/')
+ strbuf_addch(path, '/');
+ if (cdir->ucd->check_only)
+ /*
+ * check_only is set as a result of treat_directory() getting
+ * to its bottom. Verify again the same set of directories
+ * with check_only set.
+ */
+ return read_directory_recursive(dir, path->buf, path->len,
+
+ cdir->ucd, 1, simplify);
Unusual blank line placement.
quoted hunk
+ /*
+ * We get path_recurse in the first run when
+ * directory_exists_in_index() returns index_nonexistent. We
+ * are sure that new changes in the index does not impact the
+ * outcome. Return now.
+ */
+ return path_recurse;
+}
+
@@ -1477,6 +1590,12 @@ static void close_cached_dir(struct cached_dir *cdir) { if (cdir->fdir) closedir(cdir->fdir);+ /*+ * We have gone through this directory and found no untracked+ * entries. Mark it valid.+ */+ if (cdir->untracked && !cdir->untracked->valid)+ cdir->untracked->valid = 1;
Or, stated more simply:
if (cdir->untracked)
cdir->untracked->valid = 1;
On Wed, Oct 29, 2014 at 12:37 AM, Torsten Bögershausen [off-list ref] wrote:
On 2014-10-27 13.10, Nguyễn Thái Ngọc Duy wrote:
[]
Nice serious, I can imagine to test & benchmark it (so I assume there is a branch
on github or so ?)
It's on 'pu' now. There's a branch on my github repo, but it has some
extra debugging on top, so 'pu' is probably the best option.
Another thing:
Can we switch the feature off?
It could be nice to benchmark with and without the cache on the command line,
and besides that we may want to switch it on or off, depending on the file system.
I think this can be easily done when reading and writing the index file.
(But may cost a config variable, core.dirmtime ??)
You can permanently switch it off with "git update-index
--no-untracked-cache". An option to temporarily disable it is not
available. I'll add an environment variable for that.
For a normal case, "update-index --untracked-cache" would test if the
OS/FS supports this before enabling it. If the repo is moved to
another fs, or being used by a different OS, then the user has to
manually disable it first. I don't know what we can do here, maybe
record uname and filesystem in the index as well..
quoted
diff --git a/dir.c b/dir.c
+static int add_excludes(const char *fname, const char *base, int baselen,
+ struct exclude_list *el, int check_index,
+ struct sha1_stat *ss, int ss_valid)
Cosmetic question: does it make sense to write
struct sha1_stat *sha1_stat
or
struct sha1_stat *s_stat
On Wed, Oct 29, 2014 at 12:37 AM, Torsten Bögershausen [off-list ref] wrote:
On 2014-10-27 13.10, Nguyễn Thái Ngọc Duy wrote:
[]
quoted
+static void xmkdir(const char *path)
+{
+ if (mkdir(path, 0700))
+ die_errno(_("failed to create directory %s"), path);
+}
Does it makes sense to ignore EINTR and do a "retry" ?
Another question is if the function could be called mkdir_or_die() instead?
I realized that there are 2 families of xfunc() in wrapper.c, some die, some retry.
This is only used interactively, I think it's ok to ignore EINTR as
long as we report clearly the case (and hope the user to re-enter the
command)
--
Duy
Updated based on comments from the list, including three new patches:
- 16/22 allows to ignore untracked cache without destroying it (for
comparison and verification)
- 21/22 and 22/22 add some protection against filesystem or operating
system changes
Also fix 'update-index --untracked-cache' essentially merging the
split index back because I set wrong update flag.
This series is also available on github [1] but you will have to
ignore the few top debugging patches first. Diff against the version
on 'pu' below.
[1] https://github.com/pclouds/git.git untracked-cache
@@ -115,6 +115,7 @@ static int test_if_untracked_cache_is_supported(void)fd=create_file("dir-mtime-test/newfile");xstat("dir-mtime-test",&st);if(!match_stat_data(&base,&st)){+close(fd);fputc('\n',stderr);fprintf_ln(stderr,_("directory stat info does not ""change after adding a new file"));
@@ -127,6 +128,7 @@ static int test_if_untracked_cache_is_supported(void)xmkdir("dir-mtime-test/new-dir");xstat("dir-mtime-test",&st);if(!match_stat_data(&base,&st)){+close(fd);fputc('\n',stderr);fprintf_ln(stderr,_("directory stat info does not change ""after adding a new directory"));
@@ -1094,10 +1096,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)/* should be the same flags used by git-status */uc->dir_flags=DIR_SHOW_OTHER_DIRECTORIES|DIR_HIDE_EMPTY_DIRECTORIES;the_index.untracked=uc;-the_index.cache_changed|=SOMETHING_CHANGED;+the_index.cache_changed|=UNTRACKED_CHANGED;}elseif(!untracked_cache&&the_index.untracked){the_index.untracked=NULL;-the_index.cache_changed|=SOMETHING_CHANGED;+the_index.cache_changed|=UNTRACKED_CHANGED;}if(active_cache_changed){
@@ -648,7 +648,7 @@ static int add_excludes(const char *fname, const char *base, int baselen,if(0<=fd)close(fd);if(!check_index||-(buf=read_skip_worktree_file_from_index(fname,&size,ss))==NULL)+(buf=read_skip_worktree_file_from_index(fname,&size,sha1_stat))==NULL)return-1;if(size==0){free(buf);
@@ -661,9 +661,10 @@ static int add_excludes(const char *fname, const char *base, int baselen,}else{size=xsize_t(st.st_size);if(size==0){-if(ss){-fill_stat_data(&ss->stat,&st);-hashcpy(ss->sha1,EMPTY_BLOB_SHA1_BIN);+if(sha1_stat){+fill_stat_data(&sha1_stat->stat,&st);+hashcpy(sha1_stat->sha1,EMPTY_BLOB_SHA1_BIN);+sha1_stat->valid=1;}close(fd);return0;
@@ -676,19 +677,20 @@ static int add_excludes(const char *fname, const char *base, int baselen,}buf[size++]='\n';close(fd);-if(ss){+if(sha1_stat){intpos;-if(ss_valid&&-!match_stat_data_racy(&the_index,&ss->stat,&st))+if(sha1_stat->valid&&+!match_stat_data_racy(&the_index,&sha1_stat->stat,&st));/* no content change, ss->sha1 still good */elseif(check_index&&(pos=cache_name_pos(fname,strlen(fname)))>=0&&!ce_stage(active_cache[pos])&&ce_uptodate(active_cache[pos]))-hashcpy(ss->sha1,active_cache[pos]->sha1);+hashcpy(sha1_stat->sha1,active_cache[pos]->sha1);else-hash_sha1_file(buf,size,"blob",ss->sha1);-fill_stat_data(&ss->stat,&st);+hash_sha1_file(buf,size,"blob",sha1_stat->sha1);+fill_stat_data(&sha1_stat->stat,&st);+sha1_stat->valid=1;}}
@@ -1457,12 +1461,11 @@ static enum path_treatment treat_path_fast(struct dir_struct *dir,intbaselen,conststructpath_simplify*simplify){+strbuf_setlen(path,baselen);if(!cdir->ucd){-strbuf_setlen(path,baselen);strbuf_addstr(path,cdir->file);returnpath_untracked;}-strbuf_setlen(path,baselen);strbuf_addstr(path,cdir->ucd->name);/* treat_one_path() does this before it calls treat_directory() */if(path->buf[path->len-1]!='/')
@@ -2243,10 +2227,22 @@ static void write_one_dir(struct strbuf *out, struct untracked_cache_dir *untracwrite_one_dir(out,untracked->dirs[i]);}+staticvoidget_ident_string(structstrbuf*sb)+{+structutsnameuts;++if(uname(&uts))+die_errno(_("failed to get kernel name and information"));+strbuf_addf(sb,"Location %s, system %s %s %s",get_git_work_tree(),+uts.sysname,uts.release,uts.version);+}+voidwrite_untracked_extension(structstrbuf*out,structuntracked_cache*untracked){structondisk_untracked_cache*ouc;-intlen=0;+structstrbufsb=STRBUF_INIT;+unsignedcharvarbuf[16];+intlen=0,varint_len;if(untracked->exclude_per_dir)len=strlen(untracked->exclude_per_dir);ouc=xmalloc(sizeof(*ouc)+len);
@@ -110,15 +111,15 @@ struct sha1_stat {structuntracked_cache_dir{structuntracked_cache_dir**dirs;char**untracked;-/* null SHA-1 means this directory does not have .gitignore */-unsignedcharexclude_sha1[20];structstat_datastat_data;-unsignedintrecurse:1;+unsignedintuntracked_alloc,dirs_nr,dirs_alloc;+unsignedintuntracked_nr;unsignedintcheck_only:1;/* all data except 'dirs' in this struct are good */unsignedintvalid:1;-unsignedintuntracked_nr:29;-unsignedintuntracked_alloc,dirs_nr,dirs_alloc;+unsignedintrecurse:1;+/* null SHA-1 means this directory does not have .gitignore */+unsignedcharexclude_sha1[20];charname[1];};
@@ -44,6 +44,7 @@ int main(int ac, char **av){structuntracked_cache*uc;structstrbufbase=STRBUF_INIT;+setup_git_directory();if(read_cache()<0)die("unable to read index file");uc=the_index.untracked;
This is not used anywhere yet. But the goal is to compare quickly if a
.gitignore file has changed when we have the SHA-1 of both old (cached
somewhere) and new (from index or a tree) versions.
Helped-by: Junio C Hamano [off-list ref]
Helped-by: Torsten Bögershausen [off-list ref]
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
dir.h | 6 ++++++
2 files changed, 52 insertions(+), 7 deletions(-)
@@ -547,7 +559,7 @@ int add_excludes_from_file_to_list(const char *fname,if(0<=fd)close(fd);if(!check_index||-(buf=read_skip_worktree_file_from_index(fname,&size))==NULL)+(buf=read_skip_worktree_file_from_index(fname,&size,sha1_stat))==NULL)return-1;if(size==0){free(buf);
@@ -560,6 +572,11 @@ int add_excludes_from_file_to_list(const char *fname,}else{size=xsize_t(st.st_size);if(size==0){+if(sha1_stat){+fill_stat_data(&sha1_stat->stat,&st);+hashcpy(sha1_stat->sha1,EMPTY_BLOB_SHA1_BIN);+sha1_stat->valid=1;+}close(fd);return0;}
@@ -571,6 +588,21 @@ int add_excludes_from_file_to_list(const char *fname,}buf[size++]='\n';close(fd);+if(sha1_stat){+intpos;+if(sha1_stat->valid&&+!match_stat_data(&sha1_stat->stat,&st))+;/* no content change, ss->sha1 still good */+elseif(check_index&&+(pos=cache_name_pos(fname,strlen(fname)))>=0&&+!ce_stage(active_cache[pos])&&+ce_uptodate(active_cache[pos]))+hashcpy(sha1_stat->sha1,active_cache[pos]->sha1);+else+hash_sha1_file(buf,size,"blob",sha1_stat->sha1);+fill_stat_data(&sha1_stat->stat,&st);+sha1_stat->valid=1;+}}el->filebuf=buf;
@@ -589,6 +621,13 @@ int add_excludes_from_file_to_list(const char *fname,return0;}+intadd_excludes_from_file_to_list(constchar*fname,constchar*base,+intbaselen,structexclude_list*el,+intcheck_index)+{+returnadd_excludes(fname,base,baselen,el,check_index,NULL);+}+structexclude_list*add_exclude_list(structdir_struct*dir,intgroup_type,constchar*src){
Make sure the starting conditions and all global exclude files are
good to go. If not, either disable untracked cache completely, or wipe
out the cache and start fresh.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
dir.h | 4 +++
2 files changed, 114 insertions(+), 3 deletions(-)
@@ -1571,9 +1595,87 @@ static int treat_leading_path(struct dir_struct *dir,returnrc;}+staticstructuntracked_cache_dir*validate_untracked_cache(structdir_struct*dir,+intbase_len,+conststructpathspec*pathspec)+{+structuntracked_cache_dir*root;++if(!dir->untracked)+returnNULL;++/*+*Weonlysupport$GIT_DIR/info/excludeandcore.excludesfile+*astheglobalignorerulefiles.Anyotheradditions+*(e.g.fromcommandline)invalidatethecache.This+*conditionalsocatchesrunningsetup_standard_excludes()+*beforesettingdir->untracked!+*/+if(dir->unmanaged_exclude_files)+returnNULL;++/*+*Optimizeforthemainusecaseonly:whole-treegit+*status.Moreworkinvolvedintreat_leading_path()ifwe+*usecacheonjustasubsetoftheworktree.pathspec+*supportcouldmakethematterevenworse.+*/+if(base_len||(pathspec&&pathspec->nr))+returnNULL;++/* Different set of flags may produce different results */+if(dir->flags!=dir->untracked->dir_flags||+/*+*Seetreat_directory(),caseindex_nonexistent.Without+*thisflag,wemayneedtoalsocache.gitfilecontent+*fortheresolve_gitlink_ref()call,whichwedon't.+*/+!(dir->flags&DIR_SHOW_OTHER_DIRECTORIES)||+/* We don't support collecting ignore files */+(dir->flags&(DIR_SHOW_IGNORED|DIR_SHOW_IGNORED_TOO|+DIR_COLLECT_IGNORED)))+returnNULL;++/*+*Ifweuse.gitignoreinthecacheandnowyouchangeitto+*.gitexclude,everythingwillgowrong.+*/+if(dir->exclude_per_dir!=dir->untracked->exclude_per_dir&&+strcmp(dir->exclude_per_dir,dir->untracked->exclude_per_dir))+returnNULL;++/*+*EXC_CMDLisnotconsideredinthecache.Ifpeoplesetit,+*skipthecache.+*/+if(dir->exclude_list_group[EXC_CMDL].nr)+returnNULL;++if(!dir->untracked->root){+constintlen=sizeof(*dir->untracked->root);+dir->untracked->root=xmalloc(len);+memset(dir->untracked->root,0,len);+}++/* Validate $GIT_DIR/info/exclude and core.excludesfile */+root=dir->untracked->root;+if(hashcmp(dir->ss_info_exclude.sha1,+dir->untracked->ss_info_exclude.sha1)){+invalidate_gitignore(dir->untracked,root);+dir->untracked->ss_info_exclude=dir->ss_info_exclude;+}+if(hashcmp(dir->ss_excludes_file.sha1,+dir->untracked->ss_excludes_file.sha1)){+invalidate_gitignore(dir->untracked,root);+dir->untracked->ss_excludes_file=dir->ss_excludes_file;+}+returnroot;+}+intread_directory(structdir_struct*dir,constchar*path,intlen,conststructpathspec*pathspec){structpath_simplify*simplify;+structuntracked_cache_dir*untracked;/**Checkoutcreate_simplify()
@@ -1597,10 +1699,15 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru*create_simplify().*/simplify=create_simplify(pathspec?pathspec->_raw:NULL);+untracked=validate_untracked_cache(dir,len,pathspec);+if(!untracked)+/*+*makesureuntrackedcachecodepathisdisabled,+*e.g.prep_exclude()+*/+dir->untracked=NULL;if(!len||treat_leading_path(dir,path,len,simplify))-read_directory_recursive(dir,path,len,-dir->untracked?dir->untracked->root:NULL,-0,simplify);+read_directory_recursive(dir,path,len,untracked,0,simplify);free_simplify(simplify);qsort(dir->entries,dir->nr,sizeof(structdir_entry*),cmp_name);qsort(dir->ignored,dir->ignored_nr,sizeof(structdir_entry*),cmp_name);
@@ -115,6 +115,8 @@ struct untracked_cache_dir {unsignedintuntracked_alloc,dirs_nr,dirs_alloc;unsignedintuntracked_nr;unsignedintcheck_only:1;+/* all data in this struct are good */+unsignedintvalid:1;/* null SHA-1 means this directory does not have .gitignore */unsignedcharexclude_sha1[20];charname[1];
The idea is if we can capture all input and (non-rescursive) output of
read_directory_recursive(), and can verify later that all the input is
the same, then the second r_d_r() should produce the same output as in
the first run.
The requirement for this to work is stat info of a directory MUST
change if an entry is added to or removed from that directory (and
should not change often otherwise). If your OS and filesytem do not
meet this requirement, untracked cache is not for you. Most file
systems on *nix should be fine. On Windows, NTFS is fine while FAT may
be not [1] even though FAT on Linux seems to be fine.
The list of input of r_d_r() is in the big comment block in dir.h. In
short, the output of a directory (not counting subdirs) mainly depends
on stat info of the directory in question, all .gitignore leading to
it and the check_only flag when r_d_r() is called recursively. This
patch records all this info (and the output) as r_d_r() runs.
Two hash_sha1_file() are required for $GIT_DIR/info/exclude and
core.excludesfile unless their stat data matches. hash_sha1_file() is
only needed when .gitignore files in the worktree are modified,
otherwise their SHA-1 in index is used (see the previous patch).
We could store stat data for .gitignore files so we don't have to
rehash them if their content is different from index, but I think
.gitignore files are rarely modified, so not worth extra cache data
(and hashing penalty read-cache.c:verify_hdr(), as we will be storing
this as an index extension).
The implication is, if you change .gitignore, you better add it to the
index soon or you lose all the benefit of untracked cache because a
modified .gitignore invalidates all subdirs recursively. This is
especially bad for .gitignore at root.
This cached output is about untracked files only, not ignored files
because the number of tracked files is usually small, so small cache
overhead, while the number of ignored files could go really high
(e.g. *.o files mixing with source code).
[1] "Description of NTFS date and time stamps for files and folders"
http://support.microsoft.com/kb/299648
Helped-by: Torsten Bögershausen [off-list ref]
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
dir.h | 60 ++++++++++++++++++++++++++++
2 files changed, 182 insertions(+), 19 deletions(-)
@@ -645,14 +692,20 @@ struct exclude_list *add_exclude_list(struct dir_struct *dir,/**Usedtosetupcore.excludesfileand.git/info/excludelists.*/-voidadd_excludes_from_file(structdir_struct*dir,constchar*fname)+staticvoidadd_excludes_from_file_1(structdir_struct*dir,constchar*fname,+structsha1_stat*sha1_stat){structexclude_list*el;el=add_exclude_list(dir,EXC_FILE,fname);-if(add_excludes_from_file_to_list(fname,"",0,el,0)<0)+if(add_excludes(fname,"",0,el,0,sha1_stat)<0)die("cannot use %s as an exclude file",fname);}+voidadd_excludes_from_file(structdir_struct*dir,constchar*fname)+{+add_excludes_from_file_1(dir,fname,NULL);+}+intmatch_basename(constchar*basename,intbasenamelen,constchar*pattern,intprefix,intpatternlen,intflags)
@@ -864,9 +918,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)/* Read from the parent directories and push them down. */current=stk?stk->baselen:-1;strbuf_setlen(&dir->basebuf,current<0?0:current);+if(dir->untracked)+untracked=stk?stk->ucd:dir->untracked->root;+else+untracked=NULL;+while(current<baselen){structexclude_stack*stk=xcalloc(1,sizeof(*stk));constchar*cp;+structsha1_statsha1_stat;if(current<0){cp=base;
@@ -876,10 +936,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)if(!cp)die("oops in prep_exclude");cp++;+untracked=+lookup_untracked(dir->untracked,untracked,+base+current,+cp-base-current);}stk->prev=dir->exclude_stack;stk->baselen=cp-base;stk->exclude_ix=group->nr;+stk->ucd=untracked;el=add_exclude_list(dir,EXC_DIRS,NULL);strbuf_add(&dir->basebuf,base+current,stk->baselen-current);assert(stk->baselen==dir->basebuf.len);
@@ -902,6 +967,8 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)}/* Try to read per-directory file */+hashclr(sha1_stat.sha1);+sha1_stat.valid=0;if(dir->exclude_per_dir){/**dir->basebufgetsreusedbythetraversal,butwe
@@ -1346,24 +1430,36 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,if(!fdir)gotoout;+if(untracked)+untracked->check_only=!!check_only;+while((de=readdir(fdir))!=NULL){/* check how the file or directory should be treated */-state=treat_path(dir,de,&path,baselen,simplify);+state=treat_path(dir,untracked,de,&path,baselen,simplify);+if(state>dir_state)dir_state=state;/* recurse into subdir if instructed by treat_path */if(state==path_recurse){-subdir_state=read_directory_recursive(dir,path.buf,-path.len,check_only,simplify);+structuntracked_cache_dir*ud;+ud=lookup_untracked(dir->untracked,untracked,+path.buf+baselen,+path.len-baselen);+subdir_state=+read_directory_recursive(dir,path.buf,path.len,+ud,check_only,simplify);if(subdir_state>dir_state)dir_state=subdir_state;}if(check_only){/* abort early if maximum state has been reached */-if(dir_state==path_untracked)+if(dir_state==path_untracked){+if(untracked)+add_untracked(untracked,path.buf+baselen);break;+}/* skip the dir_add_* part */continue;}
@@ -1459,7 +1558,7 @@ static int treat_leading_path(struct dir_struct *dir,break;if(simplify_away(sb.buf,sb.len,simplify))break;-if(treat_one_path(dir,&sb,simplify,+if(treat_one_path(dir,NULL,&sb,simplify,DT_DIR,NULL)==path_none)break;/* do not recurse into it */if(len<=baselen){
@@ -1499,7 +1598,9 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru*/simplify=create_simplify(pathspec?pathspec->_raw:NULL);if(!len||treat_leading_path(dir,path,len,simplify))-read_directory_recursive(dir,path,len,0,simplify);+read_directory_recursive(dir,path,len,+dir->untracked?dir->untracked->root:NULL,+0,simplify);free_simplify(simplify);qsort(dir->entries,dir->nr,sizeof(structdir_entry*),cmp_name);qsort(dir->ignored,dir->ignored_nr,sizeof(structdir_entry*),cmp_name);
@@ -66,6 +66,7 @@ struct exclude_stack {structexclude_stack*prev;/* the struct exclude_stack for the parent directory */intbaselen;intexclude_ix;/* index of exclude_list within EXC_DIRS exclude_list_group */+structuntracked_cache_dir*ucd;};structexclude_list_group{
@@ -79,6 +80,60 @@ struct sha1_stat {intvalid;};+/*+*Untrackedcache+*+*Thefollowinginputsaresufficienttodeterminewhatfilesina+*directoryareexcluded:+*+*-Thelistoffilesanddirectoriesofthedirectioninquestion+*-The$GIT_DIR/index+*-dir_structflags+*-Thecontentof$GIT_DIR/info/exclude+*-Thecontentofcore.excludesfile+*-Thecontent(orthelack)of.gitignoreofallparentdirectories+*from$GIT_WORK_TREE+*-Thecheck_onlyflaginread_directory_recursive(for+*DIR_HIDE_EMPTY_DIRECTORIES)+*+*Thefirstinputcanbecheckedusingdirectorymtime.Inmany+*filesystems,directorymtime(stat_datafield)isupdatedwhenits+*filesordirectsubdirsareaddedorremoved.+*+*Thesecondonecanbehookedfromcache_tree_invalidate_path().+*Wheneverafile(orasubmodule)isaddedorremovedfroma+*directory,weinvalidatethatdirectory.+*+*Theremaininginputsareeasy,theirSHA-1couldbeusedtoverify+*theircontents(exclude_sha1[],info_exclude_sha1[]and+*excludes_file_sha1[])+*/+structuntracked_cache_dir{+structuntracked_cache_dir**dirs;+char**untracked;+structstat_datastat_data;+unsignedintuntracked_alloc,dirs_nr,dirs_alloc;+unsignedintuntracked_nr;+unsignedintcheck_only:1;+/* null SHA-1 means this directory does not have .gitignore */+unsignedcharexclude_sha1[20];+charname[1];+};++structuntracked_cache{+structsha1_statss_info_exclude;+structsha1_statss_excludes_file;+constchar*exclude_per_dir;+/*+*dir_struct#flagsmustmatchdir_flagsortheuntracked+*cacheisignored.+*/+unsigneddir_flags;+structuntracked_cache_dir*root;+/* Statistics */+intdir_created;+};+structdir_struct{intnr,alloc;intignored_nr,ignored_alloc;
@@ -126,6 +181,11 @@ struct dir_struct {structexclude_stack*exclude_stack;structexclude*exclude;structstrbufbasebuf;++/* Enable untracked file cache if set */+structuntracked_cache*untracked;+structsha1_statss_info_exclude;+structsha1_statss_excludes_file;};/*
It's easy to see that if an existing .gitignore changes, its SHA-1
would be different and invalidate_gitignore() is called.
If .gitignore is removed, add_excludes() will treat it like an empty
.gitignore, which again should invalidate the cached directory data.
if .gitignore is added, lookup_untracked() already fills initial
.gitignore SHA-1 as "empty file", so again invalidate_gitignore() is
called.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
This allows us to feed different info to read_directory_recursive()
based on untracked cache in the next patch.
Helped-by: Ramsay Jones [off-list ref]
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 8 deletions(-)
@@ -1459,23 +1500,21 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,structuntracked_cache_dir*untracked,intcheck_only,conststructpath_simplify*simplify){-DIR*fdir;+structcached_dircdir;enumpath_treatmentstate,subdir_state,dir_state=path_none;-structdirent*de;structstrbufpath=STRBUF_INIT;strbuf_add(&path,base,baselen);-fdir=opendir(path.len?path.buf:".");-if(!fdir)+if(open_cached_dir(&cdir,dir,untracked,&path,check_only))gotoout;if(untracked)untracked->check_only=!!check_only;-while((de=readdir(fdir))!=NULL){+while(!read_cached_dir(&cdir)){/* check how the file or directory should be treated */-state=treat_path(dir,untracked,de,&path,baselen,simplify);+state=treat_path(dir,untracked,&cdir,&path,baselen,simplify);if(state>dir_state)dir_state=state;
The main readdir loop in read_directory_recursive() is replaced with a
new one that checks if cached results of a directory is still valid.
If a file is added or removed from the index, the containing directory
is invalidated (but not its subdirs). If directory's mtime is changed,
the same happens. If a .gitignore is updated, the containing directory
and all subdirs are invalidated recursively. If dir_struct#flags or
other conditions change, the cache is ignored.
If a directory is invalidated, we opendir/readdir/closedir and run the
exclude machinery on that directory listing as usual. If untracked
cache is also enabled, we'll update the cache along the way. If a
directory is validated, we simply pull the untracked listing out from
the cache. The cache also records the list of direct subdirs that we
have to recurse in. Fully excluded directories are seen as "untracked
files".
In the best case when no dirs are invalidated, read_directory()
becomes a series of
stat(dir), open(.gitignore), fstat(), read(), close() and optionally
hash_sha1_file()
For comparison, standard read_directory() is a sequence of
opendir(), readdir(), open(.gitignore), fstat(), read(), close(), the
expensive last_exclude_matching() and closedir().
We already try not to open(.gitignore) if we know it does not exist,
so open/fstat/read/close sequence does not apply to every
directory. The sequence could be reduced further, as noted in
prep_exclude() in another patch. So in theory, the entire best-case
read_directory sequence could be reduced to a series of stat() and
nothing else.
This is not a silver bullet approach. When you compile a C file, for
example, the old .o file is removed and a new one with the same name
created, effectively invalidating the containing directory's cache
(but not its subdirectories). If your build process touches every
directory, this cache adds extra overhead for nothing, so it's a good
idea to separate generated files from tracked files.. Editors may use
the same strategy for saving files. And of course you're out of luck
running your repo on an unsupported filesytem and/or operating system.
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
dir.h | 2 ++
2 files changed, 121 insertions(+), 2 deletions(-)
@@ -1423,6 +1436,39 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,}}+staticenumpath_treatmenttreat_path_fast(structdir_struct*dir,+structuntracked_cache_dir*untracked,+structcached_dir*cdir,+structstrbuf*path,+intbaselen,+conststructpath_simplify*simplify)+{+strbuf_setlen(path,baselen);+if(!cdir->ucd){+strbuf_addstr(path,cdir->file);+returnpath_untracked;+}+strbuf_addstr(path,cdir->ucd->name);+/* treat_one_path() does this before it calls treat_directory() */+if(path->buf[path->len-1]!='/')+strbuf_addch(path,'/');+if(cdir->ucd->check_only)+/*+*check_onlyissetasaresultoftreat_directory()getting+*toitsbottom.Verifyagainthesamesetofdirectories+*withcheck_onlyset.+*/+returnread_directory_recursive(dir,path->buf,path->len,+cdir->ucd,1,simplify);+/*+*Wegetpath_recurseinthefirstrunwhen+*directory_exists_in_index()returnsindex_nonexistent.We+*aresurethatnewchangesintheindexdoesnotimpactthe+*outcome.Returnnow.+*/+returnpath_recurse;+}+staticenumpath_treatmenttreat_path(structdir_struct*dir,structuntracked_cache_dir*untracked,structcached_dir*cdir,
@@ -1535,7 +1652,7 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,if(check_only){/* abort early if maximum state has been reached */if(dir_state==path_untracked){-if(untracked)+if(cdir.fdir)add_untracked(untracked,path.buf+baselen);break;}
If we redo this thing in a functional style, we would have one struct
untracked_dir as input tree and another as output. The input is used
for verification. The output is a brand new tree, reflecting current
worktree.
But that means recreate a lot of dir nodes even if a lot could be
shared between input and output trees in good cases. So we go with the
messy but efficient way, combining both input and output trees into
one. We need a way to know which node in this combined tree belongs to
the output. This is the purpose of this "recurse" flag.
"valid" bit can't be used for this because it's about data of the node
except the subdirs. When we invalidate a directory, we want to keep
cached data of the subdirs intact even though we don't really know
what subdir still exists (yet). Then we check worktree to see what
actual subdir remains on disk. Those will have 'recurse' bit set
again. If cached data for those are still valid, we may be able to
avoid computing exclude files for them. Those subdirs that are deleted
will have 'recurse' remained clear and their 'valid' bits do not
matter.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 14 +++++++++++++-
dir.h | 3 ++-
2 files changed, 15 insertions(+), 2 deletions(-)
@@ -1841,6 +1850,9 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dinvalidate_gitignore(dir->untracked,root);dir->untracked->ss_excludes_file=dir->ss_excludes_file;}++/* Make sure this directory is not dropped out at saving phase */+root->recurse=1;returnroot;}
@@ -115,8 +115,9 @@ struct untracked_cache_dir {unsignedintuntracked_alloc,dirs_nr,dirs_alloc;unsignedintuntracked_nr;unsignedintcheck_only:1;-/* all data in this struct are good */+/* all data except 'dirs' in this struct are good */unsignedintvalid:1;+unsignedintrecurse:1;/* null SHA-1 means this directory does not have .gitignore */unsignedcharexclude_sha1[20];charname[1];
This cuts down a signficant number of open(.gitignore) because most
directories usually don't have .gitignore files.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
@@ -1366,6 +1366,9 @@ static int read_index_extension(struct index_state *istate,if(read_link_extension(istate,data,sz))return-1;break;+caseCACHE_EXT_UNTRACKED:+istate->untracked=read_untracked_extension(data,sz);+break;default:if(*ext<'A'||'Z'<*ext)returnerror("index uses %.4s extension, which we do not understand",
@@ -1631,6 +1634,8 @@ int discard_index(struct index_state *istate)istate->cache=NULL;istate->cache_alloc=0;discard_split_index(istate);+free_untracked_cache(istate->untracked);+istate->untracked=NULL;return0;}
Ideally we should implement untracked_cache_remove_from_index() and
untracked_cache_add_to_index() so that they update untracked cache
right away instead of invalidating it and wait for read_directory()
next time to deal with it. But that may need some more work in
unpack-trees.c. So stay simple as the first step.
The new call in add_index_entry_with_check() may look strange because
new calls usually stay close to cache_tree_invalidate_path(). We do it
a bit later than c_t_i_p() in this function because if it's about
replacing the entry with the same name, we don't care (but cache-tree
does).
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 31 +++++++++++++++++++++++++++++++
dir.h | 4 ++++
read-cache.c | 4 ++++
unpack-trees.c | 7 +++++--
4 files changed, 44 insertions(+), 2 deletions(-)
@@ -270,20 +270,26 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)returnchanged;}-staticintis_racy_timestamp(conststructindex_state*istate,-conststructcache_entry*ce)+staticintis_racy_stat(conststructindex_state*istate,+conststructstat_data*sd){-return(!S_ISGITLINK(ce->ce_mode)&&-istate->timestamp.sec&&+return(istate->timestamp.sec&&#ifdef USE_NSEC/* nanosecond timestamped files can also be racy! */-(istate->timestamp.sec<ce->ce_stat_data.sd_mtime.sec||-(istate->timestamp.sec==ce->ce_stat_data.sd_mtime.sec&&-istate->timestamp.nsec<=ce->ce_stat_data.sd_mtime.nsec))+(istate->timestamp.sec<sd->sd_mtime.sec||+(istate->timestamp.sec==sd->sd_mtime.sec&&+istate->timestamp.nsec<=sd->sd_mtime.nsec))#else-istate->timestamp.sec<=ce->ce_stat_data.sd_mtime.sec+istate->timestamp.sec<=sd->sd_mtime.sec#endif-);+);+}++staticintis_racy_timestamp(conststructindex_state*istate,+conststructcache_entry*ce)+{+return(!S_ISGITLINK(ce->ce_mode)&&+is_racy_stat(istate,&ce->ce_stat_data));}intie_match_stat(conststructindex_state*istate,
This could be used to verify correct behavior in tests
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
This can be used to double check if results with untracked cache are
correctly, compared to vanilla version. Untracked cache remains in
index, but not used.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
When a directory is updated within the same second that its timestamp
is last saved, we cannot realize the directory has been updated by
checking timestamps. Assume the worst (something is update). See
29e4d36 (Racy GIT - 2005-12-20) for more information.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 2 ++
dir.c | 4 ++--
read-cache.c | 8 ++++++++
3 files changed, 12 insertions(+), 2 deletions(-)
@@ -680,7 +680,7 @@ static int add_excludes(const char *fname, const char *base, int baselen,if(sha1_stat){intpos;if(sha1_stat->valid&&-!match_stat_data(&sha1_stat->stat,&st))+!match_stat_data_racy(&the_index,&sha1_stat->stat,&st));/* no content change, ss->sha1 still good */elseif(check_index&&(pos=cache_name_pos(fname,strlen(fname)))>=0&&
@@ -1536,7 +1536,7 @@ static int valid_cached_dir(struct dir_struct *dir,return0;}if(!untracked->valid||-match_stat_data(&untracked->stat_data,&st)){+match_stat_data_racy(&the_index,&untracked->stat_data,&st)){if(untracked->valid)invalidate_directory(dir->untracked,untracked);fill_stat_data(&untracked->stat_data,&st);
update_index_if_able() is moved down so that the updated untracked
cache could be written out.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/commit.c | 5 +++--
wt-status.c | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
@@ -172,6 +172,14 @@ may not support it yet. the shared index file. This mode is designed for very large indexes that take a signficant amount of time to read or write.+--untracked-cache::+--no-untracked-cache::+ Enable or disable untracked cache extension. This could speed+ up for commands that involve determining untracked files such+ as `git status`. The underlying operating system and file+ system must change `st_mtime` field of a directory if files+ are added or deleted in that directory.+ \--:: Do not interpret any more arguments as options.
@@ -740,6 +740,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,intcmd_update_index(intargc,constchar**argv,constchar*prefix){intnewfd,entries,has_errors=0,line_termination='\n';+intuntracked_cache=-1;intread_from_stdin=0;intprefix_length=prefix?strlen(prefix):0;intpreferred_index_format=0;
@@ -831,6 +832,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)N_("write index in this format")),OPT_BOOL(0,"split-index",&split_index,N_("enable or disable split index")),+OPT_BOOL(0,"untracked-cache",&untracked_cache,+N_("enable/disable untracked cache")),OPT_END()};
@@ -937,6 +940,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)the_index.split_index=NULL;the_index.cache_changed|=SOMETHING_CHANGED;}+if(untracked_cache>0&&!the_index.untracked){+structuntracked_cache*uc;++uc=xcalloc(1,sizeof(*uc));+uc->exclude_per_dir=".gitignore";+/* should be the same flags used by git-status */+uc->dir_flags=DIR_SHOW_OTHER_DIRECTORIES|DIR_HIDE_EMPTY_DIRECTORIES;+the_index.untracked=uc;+the_index.cache_changed|=UNTRACKED_CHANGED;+}elseif(!untracked_cache&&the_index.untracked){+the_index.untracked=NULL;+the_index.cache_changed|=UNTRACKED_CHANGED;+}if(active_cache_changed){if(newfd<0){
@@ -180,6 +180,12 @@ may not support it yet. system must change `st_mtime` field of a directory if files are added or deleted in that directory.+--force-untracked-cache::+ For safety, `--untracked-cache` performs tests on the working+ directory to make sure untracked cache can be used. These+ tests can take a few seconds. `--force-untracked-cache` can be+ used to skip the tests.+ \--:: Do not interpret any more arguments as options.
@@ -47,6 +47,147 @@ static void report(const char *fmt, ...)va_end(vp);}+staticvoidremove_test_directory(void)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,"dir-mtime-test");+remove_dir_recursively(&sb,0);+strbuf_release(&sb);+}++staticvoidxmkdir(constchar*path)+{+if(mkdir(path,0700))+die_errno(_("failed to create directory %s"),path);+}++staticintxstat(constchar*path,structstat*st)+{+if(stat(path,st))+die_errno(_("failed to stat %s"),path);+return0;+}++staticintcreate_file(constchar*path)+{+intfd=open(path,O_CREAT|O_RDWR,0644);+if(fd<0)+die_errno(_("failed to create file %s"),path);+returnfd;+}++staticvoidxunlink(constchar*path)+{+if(unlink(path))+die_errno(_("failed to delete file %s"),path);+}++staticvoidxrmdir(constchar*path)+{+if(rmdir(path))+die_errno(_("failed to delete directory %s"),path);+}++staticvoidavoid_racy(void)+{+/*+*notuseifwecouldusleep(10)ifUSE_NSECisdefined.The+*fieldnseccouldbethere,buttheOScouldchooseto+*ignoreit?+*/+sleep(1);+}++staticinttest_if_untracked_cache_is_supported(void)+{+structstatst;+structstat_database;+intfd;++fprintf(stderr,_("Testing "));+xmkdir("dir-mtime-test");+atexit(remove_test_directory);+xstat("dir-mtime-test",&st);+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+fd=create_file("dir-mtime-test/newfile");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+close(fd);+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not "+"change after adding a new file"));+return0;+}+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+xmkdir("dir-mtime-test/new-dir");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+close(fd);+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not change "+"after adding a new directory"));+return0;+}+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+write_or_die(fd,"data",4);+close(fd);+xstat("dir-mtime-test",&st);+if(match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info changes "+"after updating a file"));+return0;+}+fputc('.',stderr);++avoid_racy();+close(create_file("dir-mtime-test/new-dir/new"));+xstat("dir-mtime-test",&st);+if(match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info changes after "+"adding a file inside subdirectory"));+return0;+}+fputc('.',stderr);++avoid_racy();+xunlink("dir-mtime-test/newfile");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not "+"change after deleting a file"));+return0;+}+fill_stat_data(&base,&st);+fputc('.',stderr);++avoid_racy();+xunlink("dir-mtime-test/new-dir/new");+xrmdir("dir-mtime-test/new-dir");+xstat("dir-mtime-test",&st);+if(!match_stat_data(&base,&st)){+fputc('\n',stderr);+fprintf_ln(stderr,_("directory stat info does not "+"change after deleting a directory"));+return0;+}++xrmdir("dir-mtime-test");+fprintf_ln(stderr,_(" OK"));+return1;+}+staticintmark_ce_flags(constchar*path,intflag,intmark){intnamelen=strlen(path);
@@ -834,6 +975,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)N_("enable or disable split index")),OPT_BOOL(0,"untracked-cache",&untracked_cache,N_("enable/disable untracked cache")),+OPT_SET_INT(0,"force-untracked-cache",&untracked_cache,+N_("enable untracked cache without testing the filesystem"),2),OPT_END()};
@@ -943,6 +1086,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)if(untracked_cache>0&&!the_index.untracked){structuntracked_cache*uc;+if(untracked_cache<2){+setup_work_tree();+if(!test_if_untracked_cache_is_supported())+return1;+}uc=xcalloc(1,sizeof(*uc));uc->exclude_per_dir=".gitignore";/* should be the same flags used by git-status */
@@ -0,0 +1,353 @@+#!/bin/sh++test_description='test untracked cache'++../test-lib.sh++avoid_racy(){+sleep1+}++gitupdate-index--untracked-cache+# It's fine if git update-index returns an error code other than one,+# it'll be caught in the first test.+iftest$?-eq1;then+skip_all='This system does not support untracked cache'+test_done+fi++test_expect_success'setup''+gitinitworktree&&+cdworktree&&+mkdirdonedtwodthree&&+touchonetwothreedone/onedtwo/twodthree/three&&+gitaddonetwodone/one&&+:>.git/info/exclude&&+gitupdate-index--untracked-cache+'++test_expect_success'untracked cache is empty''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude0000000000000000000000000000000000000000+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+EOF+test_cmp../expect../actual+'++cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??dthree/+??dtwo/+??three+EOF++cat>../dump.expect<<EOF&&+info/excludee69de29bb2d1d6434b8b29ae775ad8c2e48c5391+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/0000000000000000000000000000000000000000recursevalid+dthree/+dtwo/+three+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+three+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF++test_expect_success'status first time (empty cache)''+avoid_racy&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:3+gitignoreinvalidation:1+directoryinvalidation:0+opendir:4+EOF+test_cmp../trace.expect../trace+'++test_expect_success'untracked cache after first status''+test-dump-untracked-cache>../actual&&+test_cmp../dump.expect../actual+'++test_expect_success'status second time (fully populated cache)''+avoid_racy&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:0+opendir:0+EOF+test_cmp../trace.expect../trace+'++test_expect_success'untracked cache after second status''+test-dump-untracked-cache>../actual&&+test_cmp../dump.expect../actual+'++test_expect_success'modify in root directory, one dir invalidation''+avoid_racy&&+:>four&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??dthree/+??dtwo/+??four+??three+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:1+opendir:1+EOF+test_cmp../trace.expect../trace++'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/excludee69de29bb2d1d6434b8b29ae775ad8c2e48c5391+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/0000000000000000000000000000000000000000recursevalid+dthree/+dtwo/+four+three+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+three+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'new .gitignore invalidates recursively''+avoid_racy&&+echofour>.gitignore&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??.gitignore+??dthree/+??dtwo/+??three+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:1+directoryinvalidation:1+opendir:4+EOF+test_cmp../trace.expect../trace++'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/excludee69de29bb2d1d6434b8b29ae775ad8c2e48c5391+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dthree/+dtwo/+three+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+three+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'new info/exclude invalidates everything''+avoid_racy&&+echothree>>.git/info/exclude&&+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??.gitignore+??dtwo/+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:1+directoryinvalidation:0+opendir:4+EOF+test_cmp../trace.expect../trace+'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dtwo/+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'move two from tracked to untracked''+gitrm--cachedtwo&&+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecurse+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'status after the move''+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+??.gitignore+??dtwo/+??two+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:0+opendir:1+EOF+test_cmp../trace.expect../trace+'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dtwo/+two+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'move two from untracked to tracked''+gitaddtwo&&+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecurse+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_expect_success'status after the move''+:>../trace&&+GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace"\+gitstatus--porcelain>../actual&&+cat>../status.expect<<EOF&&+Adone/one+Aone+Atwo+??.gitignore+??dtwo/+EOF+test_cmp../status.expect../actual&&+cat>../trace.expect<<EOF&&+nodecreation:0+gitignoreinvalidation:0+directoryinvalidation:0+opendir:1+EOF+test_cmp../trace.expect../trace+'++test_expect_success'verify untracked cache dump''+test-dump-untracked-cache>../actual&&+cat>../expect<<EOF&&+info/exclude13263c0978fb9fad16b2d580fb800b6d811c3ff0+core.excludesfile0000000000000000000000000000000000000000+exclude_per_dir.gitignore+flags00000006+/e6fcc8f2ee31bae321d66afd183fcb7237afae6erecursevalid+.gitignore+dtwo/+/done/0000000000000000000000000000000000000000recursevalid+/dthree/0000000000000000000000000000000000000000recursecheck_onlyvalid+/dtwo/0000000000000000000000000000000000000000recursecheck_onlyvalid+two+EOF+test_cmp../expect../actual+'++test_done
If the user enables untracked cache, then
- move worktree to an unsupported filesystem
- or simply upgrade OS
- or move the whole (portable) disk from one machine to another
- or access a shared fs from another machine
there's no guarantee that untracked cache can still function properly.
Record the worktree location and OS footprint in the cache. If it
changes, err on the safe side and disable the cache. The user can
'update-index --untracked-cache' again to make sure all conditions are
met.
This change does not cover all bases, you can fool it if you try
hard. The point is to stop accidents.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
dir.c | 44 +++++++++++++++++++++++++++++++++++++++++---
git-compat-util.h | 1 +
test-dump-untracked-cache.c | 1 +
3 files changed, 43 insertions(+), 3 deletions(-)
@@ -2227,10 +2227,22 @@ static void write_one_dir(struct strbuf *out, struct untracked_cache_dir *untracwrite_one_dir(out,untracked->dirs[i]);}+staticvoidget_ident_string(structstrbuf*sb)+{+structutsnameuts;++if(uname(&uts))+die_errno(_("failed to get kernel name and information"));+strbuf_addf(sb,"Location %s, system %s %s %s",get_git_work_tree(),+uts.sysname,uts.release,uts.version);+}+voidwrite_untracked_extension(structstrbuf*out,structuntracked_cache*untracked){structondisk_untracked_cache*ouc;-intlen=0;+structstrbufsb=STRBUF_INIT;+unsignedcharvarbuf[16];+intlen=0,varint_len;if(untracked->exclude_per_dir)len=strlen(untracked->exclude_per_dir);ouc=xmalloc(sizeof(*ouc)+len);
@@ -44,6 +44,7 @@ int main(int ac, char **av){structuntracked_cache*uc;structstrbufbase=STRBUF_INIT;+setup_git_directory();if(read_cache()<0)die("unable to read index file");uc=the_index.untracked;
From: brian m. carlson <hidden> Date: 2016-06-15 23:02:53
On Sat, Nov 08, 2014 at 04:39:35PM +0700, Nguyễn Thái Ngọc Duy wrote:
The requirement for this to work is stat info of a directory MUST
change if an entry is added to or removed from that directory (and
should not change often otherwise). If your OS and filesytem do not
Should be "filesystem" (or "file system").
meet this requirement, untracked cache is not for you. Most file
systems on *nix should be fine. On Windows, NTFS is fine while FAT may
be not [1] even though FAT on Linux seems to be fine.
Tiny nit: "may be not" should probably be "may not be".
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
From: Eric Sunshine <hidden> Date: 2016-06-15 23:02:53
On Sat, Nov 8, 2014 at 4:39 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
quoted hunk
If the user enables untracked cache, then
- move worktree to an unsupported filesystem
- or simply upgrade OS
- or move the whole (portable) disk from one machine to another
- or access a shared fs from another machine
there's no guarantee that untracked cache can still function properly.
Record the worktree location and OS footprint in the cache. If it
changes, err on the safe side and disable the cache. The user can
'update-index --untracked-cache' again to make sure all conditions are
met.
This change does not cover all bases, you can fool it if you try
hard. The point is to stop accidents.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
@@ -44,6 +44,7 @@ int main(int ac, char **av){structuntracked_cache*uc;structstrbufbase=STRBUF_INIT;+setup_git_directory();
What is this change about? Is it related to the rest of this patch?
Yes. This patch makes use of get_git_work_tree() from read_cache()
below. Without setup_git_... worktree is not set up,
read_untracked_extension() thinks the repo is moved and returns no
cache. I'll make a note about this.
quoted
if (read_cache() < 0)
die("unable to read index file");
uc = the_index.untracked;
It was originally strcpy, then I wanted to get fancy and show Win3.1,
Win95... but it got complicated (couldn't just do it based on the last
bit of 'v'). Will revert.
--
Duy
It was originally strcpy, then I wanted to get fancy and show Win3.1,
Win95... but it got complicated (couldn't just do it based on the last
bit of 'v'). Will revert.
Why not use strlcpy() ?
(This feels little like an overkill, but on the other hand it is safe to use regardless
how long the buf is, and it is a good example how to avoid "overrunning" code)
If the user enables untracked cache, then
- move worktree to an unsupported filesystem
How do we detect this move ?
Shouldn't we be able to detect an unsupported file system
(by probing if stat(root_dir_of_repo) == stat(what_we_have_in_index_file))
- or simply upgrade OS
- or move the whole (portable) disk from one machine to another
How does this effect Git ?
I would rather expect an update of Git to be an issue,
but knowing that Git strongly tends to be backward compatible, there
shouldn't be a issue.
- or access a shared fs from another machine
This is interesting.
I have done some basic test on git.git using a medium fast laptop
talking to a medium fast server using a medium normal WLAN.
git status was is in a range of 2-3 seconds, with your patch 1-1.5 seconds.
(That all depends on the network load, some caching here or there)
But roughly twice the speed, very nice!
I will do some tests with networking file systems, like
Linux+ext4 -- SMB -- Windows (cygwin/Git for Windows)
Linux+ext4 -- SMB -- Mac OS X
Linux+ext4 -- NFS -- Linux
Windows -- SMB -- Linux
Windows -- SMB -- Mac OS X
(and then we have some cases where a virtual machine runs a "shared" FS with a host file system,
where the untracked cache looks promising)
I am not really sure when we need this protection.
What I understand is that stat(dir).mtime must be reliable.
Another problem may be mixing old Git with new Git, but the old Git
should write an index file without UNTR, and we should be safe ?
The new Git will write an index file with UNTR, which the old Git will ignore.
What do I miss ?
On Mon, Nov 10, 2014 at 4:39 AM, Torsten Bögershausen [off-list ref] wrote:
On 2014-11-08 10.39, Nguyễn Thái Ngọc Duy wrote:
quoted
If the user enables untracked cache, then
- move worktree to an unsupported filesystem
How do we detect this move ?
Shouldn't we be able to detect an unsupported file system
(by probing if stat(root_dir_of_repo) == stat(what_we_have_in_index_file))
I don't see any generic way of detecting this. So I just save $(cwd)
and check if the repo is moved. False positive if you move your repo
within the same filesystem. If you move your stuff to a new filesystem
and mount it to the same place as before, my test fails.
quoted
- or simply upgrade OS
- or move the whole (portable) disk from one machine to another
How does this effect Git ?
I would rather expect an update of Git to be an issue,
but knowing that Git strongly tends to be backward compatible, there
shouldn't be a issue.
If this link [1] is true and you use vfat on Linux, then we should
disable the cache when moving it to Windows.
[1] http://support.microsoft.com/kb/299648
quoted
- or access a shared fs from another machine
This is interesting.
I have done some basic test on git.git using a medium fast laptop
talking to a medium fast server using a medium normal WLAN.
git status was is in a range of 2-3 seconds, with your patch 1-1.5 seconds.
(That all depends on the network load, some caching here or there)
But roughly twice the speed, very nice!
For network fs, that's probably about it. For local fs, we still have
watchman option to speed it up a little more. Still not sure if I can
beat ".. made Mercurial's status command more than 5x faster than
Git's status command."
I am not really sure when we need this protection.
What I understand is that stat(dir).mtime must be reliable.
Yes and [1] shows that mtime is not reliable, at least on Windows+vfat.
Another problem may be mixing old Git with new Git, but the old Git
should write an index file without UNTR, and we should be safe ?
The new Git will write an index file with UNTR, which the old Git will ignore.
Old git should ignore (and discard) untr extension and go with the slow old way.
--
Duy
On 11/10/2014 12:47 AM, Duy Nguyen wrote:
Some updates from the test lab, Windows 7
"Working" means git update-index --untracked-cache reports Testing...OK
"Rejected" means "..does not change.."
cygwin + NTFS: Working
cygwin + VFAT: Rejected
The same good news for Msysgit, running your github branch
(I needed to create an empty compat/win32/sys/utsname.h to get it compiled)
And I'm still not sure if we need to store the OS in the index, or if it
is enough
to store the $pwd.
From: David Turner <hidden> Date: 2016-06-15 23:02:57
On Sat, 2014-11-08 at 16:39 +0700, Nguyễn Thái Ngọc Duy wrote:
+ * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
+ * stat data from disk (only valid if add_excludes returns zero). If
+ * ss_valid is non-zero, "ss" must contain good value as input.
ss and ss_valid should be sha1_stat and sha1_stat.valid
It might be good to document what "valid" means here e.g. "a sha1_stat
is valid if both sha1 and stat_data match the working tree's version of
the file" or whatever.
calloc instead of malloc+memset? But do we really need this memset to
include name if we're about to use a memcpy? Couldn't we just add a
trailing zero?
+ * - The list of files and directories of the direction in question
s/direction/directory/
+struct untracked_cache_dir {
+ struct untracked_cache_dir **dirs;
+ char **untracked;
+ struct stat_data stat_data;
+ unsigned int untracked_alloc, dirs_nr, dirs_alloc;
+ unsigned int untracked_nr;
+ unsigned int check_only : 1;
+ /* null SHA-1 means this directory does not have .gitignore */
+ unsigned char exclude_sha1[20];
+ char name[1];
For consistency, should this be char name[FLEX_ARRAY]? (this will entail
some changes when allocating these, of course)