Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build

Thanks for fixing the missing SANITY prerequisite Stefan. As for the error handling logic in setup.c: is_nonbare_repository_dir (was clean.c: is_git_repository) my reasoning is as follows: READ_GITFILE_ERR_STAT_FAILED READ_GITFILE_ERR_NOT_A_FILE: When checking random paths for .git files these

Re: [PATCH] t7300: fix broken && chains

On Mon, Aug 31, 2015 at 8:54 PM, Jeff King <p...@peff.net> wrote: > On Sun, Aug 30, 2015 at 11:18:09AM +0200, Erik Elfström wrote: > > Unfortunately CHAIN_LINT cannot reach inside a nested subshell. I cannot > think of a way to make it do so, and besides, that is also the

Re: [PATCH] t7300: fix broken && chains

On Mon, Aug 31, 2015 at 6:58 PM, Junio C Hamano wrote: > > Many of the constructs we see here shows clearly that this is an > ancient part of the codebase ;-), as we would be using the one > parameter form of "git init" and more test_* helpers if we were > writing this script

[PATCH] t7300: fix broken chains

While we are here, remove some boilerplate by using test_commit. Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 16 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 27557d6..86ceb38 100755

Re: [PATCH v8 1/5] setup: add gentle version of read_gitfile

On Fri, Jun 26, 2015 at 11:03 AM, Jeff King p...@peff.net wrote: I happened to be playing with clang's static analyzer today, and it noticed that there is a subtle use-after-free here. Doh, sorry about that. Thanks for fixing my bug. /Erik -- To unsubscribe from this list: send the line

[PATCH v8 3/5] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 142 +++ 1 file changed, 142 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..fbfdf2d 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh

[PATCH v8 4/5] p7300: add performance tests for clean

The tests are run in dry-run mode to avoid having to restore the test directories for each timed iteration. Using dry-run is an acceptable compromise since we are mostly interested in the initial computation of what to clean and not so much in the cleaning it self. Signed-off-by: Erik Elfström

[PATCH v8 0/5] Improving performance of git clean

This is v8 of this series. v7 can be found here: http://thread.gmane.org/gmane.comp.version-control.git/271220 Changes in v8: * change name and type of file size limit variable in read_git_file_gently Erik Elfström (5): setup: add gentle version of read_gitfile setup: sanity check file

[PATCH v8 2/5] setup: sanity check file size in read_gitfile_gently

unreasonable size that should never be exceeded by a genuine .git file. Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 1 + setup.c | 7 +++ 2 files changed, 8 insertions(+) diff --git a/cache.h b/cache.h index 25578cb..858d9b3 100644 --- a/cache.h +++ b/cache.h @@ -454,6

[PATCH v8 1/5] setup: add gentle version of read_gitfile

gits...@pobox.com Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 11 - setup.c | 84 ++--- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/cache.h b/cache.h index

[PATCH v7 1/5] setup: add gentle version of read_gitfile

gits...@pobox.com Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 11 - setup.c | 84 ++--- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/cache.h b/cache.h index

[PATCH v7 3/5] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 142 +++ 1 file changed, 142 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..fbfdf2d 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh

[PATCH v7 2/5] setup: sanity check file size in read_gitfile_gently

unreasonable size that should never be exceeded by a genuine .git file. Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 1 + setup.c | 7 +++ 2 files changed, 8 insertions(+) diff --git a/cache.h b/cache.h index 25578cb..858d9b3 100644 --- a/cache.h +++ b/cache.h @@ -454,6

[PATCH v7 4/5] p7300: add performance tests for clean

The tests are run in dry-run mode to avoid having to restore the test directories for each timed iteration. Using dry-run is an acceptable compromise since we are mostly interested in the initial computation of what to clean and not so much in the cleaning it self. Signed-off-by: Erik Elfström

[PATCH v7 0/5] Improving performance of git clean

. Erik Elfström (5): setup: add gentle version of read_gitfile setup: sanity check file size in read_gitfile_gently t7300: add tests to document behavior of clean and nested git p7300: add performance tests for clean clean: improve performance when removing lots of directories builtin

[PATCH v7 5/5] clean: improve performance when removing lots of directories

King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- builtin/clean.c | 30 ++ t/t7300-clean.sh | 10 -- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 6dcb72e..df53def 100644

[PATCH v6 6/7] clean: improve performance when removing lots of directories

King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- builtin/clean.c | 31 +++ t/t7300-clean.sh | 10 -- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 98c103f..d739dcf 100644

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

On Tue, Apr 28, 2015 at 8:17 AM, Jeff King p...@peff.net wrote: There was a discussion not too long ago on strategies for returning errors, and one of the suggestions was to return an error strbuf rather than a code[1]. That's less flexible, as the caller can't react differently based on the

Re: [PATCH v5 5/5] clean: improve performance when removing lots of directories

On Tue, Apr 28, 2015 at 8:24 AM, Jeff King p...@peff.net wrote: This iteration looks reasonable overall to me. Should this is_git_repository() helper be available to other files? I think there are other calls to resolve_gitlink_ref() that would want the same treatment (e.g., I think git

Re: [PATCH v5 4/5] p7300: add performance tests for clean

On Tue, Apr 28, 2015 at 8:33 AM, Jeff King p...@peff.net wrote: Do we actually need a large repo here? The real cost is coming from the directories we create. We could actually start with a totally empty repository if we wanted (though I don't think the t/perf system handles that right now).

Re: [PATCH v5 2/5] setup: sanity check file size in read_gitfile_gently

On Tue, Apr 28, 2015 at 8:02 AM, Jeff King p...@peff.net wrote: My understanding is that PATH_MAX is set absurdly low on Windows systems (and doesn't actually represent the real limit of a path!). Since the value is picked arbitrarily anyway, could use something more independent (like 100K or

[PATCH v5 2/5] setup: sanity check file size in read_gitfile_gently

unreasonable size that should never be exceeded by a genuine .git file. Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 1 + setup.c | 6 ++ 2 files changed, 7 insertions(+) diff --git a/cache.h b/cache.h index 868e4d3..c9f1f8e 100644 --- a/cache.h +++ b/cache.h @@ -439,6

[PATCH v5 4/5] p7300: add performance tests for clean

The tests are run in dry-run mode to avoid having to restore the test directories for each timed iteration. Using dry-run is an acceptable compromise since we are mostly interested in the initial computation of what to clean and not so much in the cleaning it self. Signed-off-by: Erik Elfström

[PATCH v5 3/5] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 128 +++ 1 file changed, 128 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..11f3a6d 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh

[PATCH v5 1/5] setup: add gentle version of read_gitfile

gits...@pobox.com Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 11 - setup.c | 82 +++-- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/cache.h b/cache.h index

[PATCH v5 5/5] clean: improve performance when removing lots of directories

directories went from 61s to 1.7s after this change. Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- builtin/clean.c | 26 ++ t/t7300-clean.sh | 8 +++- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/builtin

[PATCH v5 0/5] Improving performance of git clean

Changes in v5: * Added defines for read_gitfile_gently error codes. This was a silly mistake, sorry about that. Erik Elfström (5): setup: add gentle version of read_gitfile setup: sanity check file size in read_gitfile_gently t7300: add tests to document behavior of clean and nested git

Re: [PATCH v4 2/5] setup: sanity check file size in read_gitfile_gently

On Sat, Apr 25, 2015 at 6:47 PM, Junio C Hamano gits...@pobox.com wrote: I do not think it is wrong per-se, but the changes in this patch shows why hardcoded values assigned to error_code without #define is not a good idea, as these values are now exposed to the callers of the new function.

[PATCH v4 3/5] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 128 +++ 1 file changed, 128 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..11f3a6d 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh

[PATCH v4 1/5] setup: add gentle version of read_gitfile

gits...@pobox.com Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- cache.h | 3 ++- setup.c | 82 +++-- 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/cache.h b/cache.h index

[PATCH v4 5/5] clean: improve performance when removing lots of directories

directories went from 61s to 1.7s after this change. Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- builtin/clean.c | 26 ++ t/t7300-clean.sh | 8 +++- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/builtin

[PATCH v4 4/5] p7300: add performance tests for clean

The tests are run in dry-run mode to avoid having to restore the test directories for each timed iteration. Using dry-run is an acceptable compromise since we are mostly interested in the initial computation of what to clean and not so much in the cleaning it self. Signed-off-by: Erik Elfström

[PATCH v4 2/5] setup: sanity check file size in read_gitfile_gently

unreasonable size that should never be exceeded by a genuine .git file. Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- I'm not sure about this one but it felt like the safe thing to do. This patch can be dropped if it is not desired. I considered testing it using mkdir foo truncate -s

[PATCH v4 0/5] Improving performance of git clean

] to more clearly motivate remaining behavioral changes of git clean. Thanks to Junio C Hamano and Jeff King for comments and help on v3. Erik Elfström (5): setup: add gentle version of read_gitfile setup: sanity check file size in read_gitfile_gently t7300: add tests to document behavior of clean

Re: [PATCH/RFC v3 0/4] Improving performance of git clean

On Tue, Apr 21, 2015 at 11:24 PM, Jeff King p...@peff.net wrote: If I understand correctly, the reason that you need per-run setup is that your git clean command actually cleans things, and you need to restore the original state for each time-trial. Can you instead use git clean -n to do a

Re: [PATCH/RFC v3 0/4] Improving performance of git clean

On Wed, Apr 22, 2015 at 9:46 PM, Jeff King p...@peff.net wrote: On Wed, Apr 22, 2015 at 09:30:20PM +0200, erik elfström wrote: Yes, that is the problem. A dry run will spot this particular performance issue but maybe we lose some value as a general performance test if we only do half

Re: [PATCH/RFC v3 0/4] Improving performance of git clean

On Sun, Apr 19, 2015 at 3:14 AM, Junio C Hamano gits...@pobox.com wrote: Erik Elfström erik.elfst...@gmail.com writes: Known Problems: * Unsure about the setup.c:read_gitfile refactor, feels a bit messy? The interface indeed feels somewhat messy. I suspect that a better interface might

Re: [PATCH/RFC v3 0/4] Improving performance of git clean

tests, either directly in this series or as a follow up if that is preferred. /Erik On Tue, Apr 21, 2015 at 12:14 AM, Thomas Gummerer t.gumme...@gmail.com wrote: On 04/18, Erik Elfström wrote: * Still have issues in the performance tests, see comments from Thomas Gummerer on v2 I've looked

[PATCH/RFC v3 4/4] clean: improve performance when removing lots of directories

after this change. Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- builtin/clean.c | 25 + t/t7300-clean.sh | 8 +++- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index

[PATCH/RFC v3 2/4] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 127 +++ 1 file changed, 127 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..4b9a72a 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh

[PATCH/RFC v3 0/4] Improving performance of git clean

either. How serious is this? Is there an easy fix (preferebly to clean all bare repositories)? * Still have issues in the performance tests, see comments from Thomas Gummerer on v2 Thanks to Junio C Hamano and Jeff King for spotting fundamental problems in v2 and suggesting a solution. Erik

[PATCH/RFC v3 1/4] setup: add gentle version of read_gitfile

gits...@pobox.com Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- If this is going to be used for speculative probing should there be a sanity check before: buf = xmalloc(st.st_size + 1); len = read_in_full(fd, buf, st.st_size); Something like

[PATCH/RFC v3 3/4] p7300: add performance tests for clean

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/perf/p7300-clean.sh | 37 + 1 file changed, 37 insertions(+) create mode 100755 t/perf/p7300-clean.sh diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh new file mode 100755 index 000

Re: [PATCH v2 3/3] clean: improve performance when removing lots of directories

On Wed, Apr 15, 2015 at 7:56 PM, Junio C Hamano gits...@pobox.com wrote: Erik Elfström erik.elfst...@gmail.com writes: Before this change, clean used resolve_gitlink_ref to check for the presence of nested git repositories. This had the drawback of creating a ref_cache entry for every

Re: [PATCH v2 2/3] p7300: add performance tests for clean

On Sat, Apr 11, 2015 at 7:59 PM, Thomas Gummerer t.gumme...@gmail.com wrote: On 04/11, Erik Elfström wrote: Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/perf/p7300-clean.sh | 37 + 1 file changed, 37 insertions(+) create mode 100755 t/perf

[PATCH v2 0/3] Improving performance of git clean

fixes held back for cleanup patches: * fixed existing broken chains * added assert in existing code to guard against negative array index Thanks to Eric Sunshine and Torsten Bögershausen for the very helpful review! Erik Elfström (3): t7300: add tests to document behavior of clean and nested

[PATCH v2 1/3] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 72 1 file changed, 72 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..58e6b4a 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh

[PATCH v2 3/3] clean: improve performance when removing lots of directories

after this change. Helped-by: Jeff King p...@peff.net Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- builtin/clean.c | 24 t/t7300-clean.sh | 4 ++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 98c103f

[PATCH v2 2/3] p7300: add performance tests for clean

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/perf/p7300-clean.sh | 37 + 1 file changed, 37 insertions(+) create mode 100755 t/perf/p7300-clean.sh diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh new file mode 100755 index 000

Re: [PATCH 2/3] p7300: added performance tests for clean

to be called be before each iteration. On Tue, Apr 7, 2015 at 12:09 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Apr 6, 2015 at 4:40 PM, Torsten Bögershausen tbo...@web.de wrote: On 2015-04-06 13.48, Erik Elfström wrote: Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- diff --git

Re: [PATCH 3/3] clean: improve performance when removing lots of directories

On Tue, Apr 7, 2015 at 12:10 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Apr 6, 2015 at 7:48 AM, Erik Elfström erik.elfst...@gmail.com wrote: Before this change, clean used resolve_gitlink_ref to check for the presence of nested git repositories. This had the drawback of creating

Re: [PATCH 1/3] t7300: add tests to document behavior of clean and nested git

will fix! On Tue, Apr 7, 2015 at 12:06 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Apr 6, 2015 at 7:48 AM, Erik Elfström erik.elfst...@gmail.com wrote: Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9

[PATCH 3/3] clean: improve performance when removing lots of directories

after this change. Signed-off-by: Erik Elfström erik.elfst...@gmail.com Helped-by: Jeff King p...@peff.net --- builtin/clean.c | 23 +++ t/t7300-clean.sh | 4 ++-- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 98c103f

[PATCH 1/3] t7300: add tests to document behavior of clean and nested git

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- These tests were added so that I could understand the corner case behaviors of clean and nested repositories and document the changes in behavior that this series will cause. The ones marked as expect failure will be changed to expect

[PATCH 2/3] p7300: added performance tests for clean

Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/perf/p7300-clean.sh | 37 + 1 file changed, 37 insertions(+) create mode 100755 t/perf/p7300-clean.sh diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh new file mode 100755 index 000

[PATCH 0/3] Improving performance of git clean

reviewing. Jeff King also expressed concerns that we may have similar performance issues in other commands and that it could be good to unify these is this a repo?-checks. This series only attempts to solve the git-clean case. Erik Elfström (3): t7300: add tests to document behavior of clean

git clean performance issues

Hi, I'm having a performance issue with git clean -qxfd (note, not using -ff). The performance issue shows up when trying to clean untracked directories that themselves contain many sub directories. The performance is highly non linear with the number of sub directories. Some test numbers: Dirs

Re: git clean performance issues

On Sat, Apr 4, 2015 at 9:55 PM, Jeff King p...@peff.net wrote: On Sat, Apr 04, 2015 at 08:32:45PM +0200, erik elfström wrote: In my scenario get_ref_cache will be called 1+ times, each time with a new path. The final few calls will need to search through and compare 1+ entries before