| Message ID | 20210306192525.15197-1-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/4] Makefile/coccicheck: add comment heading for all SPATCH flags | expand |
On Sat, Mar 06, 2021 at 08:25:25PM +0100, Ævar Arnfjörð Bjarmason wrote: > Change the recently added --no-includes out for --all-includes > --include-headers-for-types. > > When I moved from --all-includes to --no-includes I missed that rules > that use "type T" such as the one added dbc540c7a58 (add QSORT, > 2016-09-29) won't work as intended. See [1] for a report of that. > > So let's move back to --all-includes to potentially scour our includes > for definitions, but supply --include-headers-for-types so we won't > racily emit duplicate hunks. See [2] for more discussion about that > issue. > > This undoes most of the optimization we gained with --no-includes, > using a very basic benchmark script [3] I get these results: > > * ~1m50s: --all-includes (before my recent patches) > 38 files changed, 74 insertions(+), 78 deletions(-) > * ~55s: --no-includes > 27 files changed, 55 insertions(+), 57 deletions(-) > * ~1m35s: --all-includes --include-headers-for-types > 38 files changed, 74 insertions(+), 78 deletions(-) > * ~1m20s: --local-includes --include-headers-for-types > 36 files changed, 70 insertions(+), 72 deletions(-) > * ~1m20s: --local-includes --include-headers-for-types --no-loops > 36 files changed, 70 insertions(+), 72 deletions(-) Where do these changes come from? I can't reproduce it running your script below, and get empty contrib/coccinelle/*.patch files for each sets of options. > And only the "--all-includes --include-headers-for-types" gets the > same results as the previous "--all-includes", even with > "--local-includes" we miss out on some definitions. > > 1. https://lore.kernel.org/git/3aac381e-2ce9-e35e-498c-9c26df235aed@web.de/ > 2. https://lore.kernel.org/git/YENdUMLTM+cerfqJ@coredump.intra.peff.net/ > 3. > for flags in \ > '--all-includes' \ > '--no-includes' \ > '--all-includes --include-headers-for-types' \ > '--local-includes --include-headers-for-types' \ > '--local-includes --include-headers-for-types --no-loops' > do > git reset --hard && > git clean -dxfq contrib/ > time make -j8 coccicheck SPATCH_FLAGS="$flags --patch ." SPATCH_XARGS="xargs -P 8 -n 16" 2>&1 | grep -v SPATCH > cat contrib/coccinelle/*.patch | git apply > git --no-pager diff --shortstat > git --no-pager diff | git patch-id > done
On Thu, Mar 18 2021, SZEDER Gábor wrote: > On Sat, Mar 06, 2021 at 08:25:25PM +0100, Ævar Arnfjörð Bjarmason wrote: >> Change the recently added --no-includes out for --all-includes >> --include-headers-for-types. >> >> When I moved from --all-includes to --no-includes I missed that rules >> that use "type T" such as the one added dbc540c7a58 (add QSORT, >> 2016-09-29) won't work as intended. See [1] for a report of that. >> >> So let's move back to --all-includes to potentially scour our includes >> for definitions, but supply --include-headers-for-types so we won't >> racily emit duplicate hunks. See [2] for more discussion about that >> issue. >> >> This undoes most of the optimization we gained with --no-includes, >> using a very basic benchmark script [3] I get these results: >> >> * ~1m50s: --all-includes (before my recent patches) >> 38 files changed, 74 insertions(+), 78 deletions(-) >> * ~55s: --no-includes >> 27 files changed, 55 insertions(+), 57 deletions(-) >> * ~1m35s: --all-includes --include-headers-for-types >> 38 files changed, 74 insertions(+), 78 deletions(-) >> * ~1m20s: --local-includes --include-headers-for-types >> 36 files changed, 70 insertions(+), 72 deletions(-) >> * ~1m20s: --local-includes --include-headers-for-types --no-loops >> 36 files changed, 70 insertions(+), 72 deletions(-) > > Where do these changes come from? I can't reproduce it running your > script below, and get empty contrib/coccinelle/*.patch files for each > sets of options. It's from René's patch in <3aac381e-2ce9-e35e-498c-9c26df235aed@web.de> patch in the side-thread. I'll clarify that commit message in any re-roll, i.e. it should just be something like: Here's a test of a patch that applies widely to the tree, showing that using [options] it's the same or different as before, as expected. >> And only the "--all-includes --include-headers-for-types" gets the >> same results as the previous "--all-includes", even with >> "--local-includes" we miss out on some definitions. >> >> 1. https://lore.kernel.org/git/3aac381e-2ce9-e35e-498c-9c26df235aed@web.de/ >> 2. https://lore.kernel.org/git/YENdUMLTM+cerfqJ@coredump.intra.peff.net/ >> 3. >> for flags in \ >> '--all-includes' \ >> '--no-includes' \ >> '--all-includes --include-headers-for-types' \ >> '--local-includes --include-headers-for-types' \ >> '--local-includes --include-headers-for-types --no-loops' >> do >> git reset --hard && >> git clean -dxfq contrib/ >> time make -j8 coccicheck SPATCH_FLAGS="$flags --patch ." SPATCH_XARGS="xargs -P 8 -n 16" 2>&1 | grep -v SPATCH >> cat contrib/coccinelle/*.patch | git apply >> git --no-pager diff --shortstat >> git --no-pager diff | git patch-id >> done
diff --git a/Makefile b/Makefile index 8aca96c6921..93b1009fbb1 100644 --- a/Makefile +++ b/Makefile @@ -1199,7 +1199,7 @@ SPARSE_FLAGS ?= SP_EXTRA_FLAGS = -Wno-universal-initializer # For the 'coccicheck' target -SPATCH_FLAGS = --no-includes --patch . +SPATCH_FLAGS = --all-includes --include-headers-for-types --patch . # For the 'coccicheck' target; setting SPATCH_BATCH_SIZE higher will # usually result in less CPU usage at the cost of higher peak memory.
Change the recently added --no-includes out for --all-includes --include-headers-for-types. When I moved from --all-includes to --no-includes I missed that rules that use "type T" such as the one added dbc540c7a58 (add QSORT, 2016-09-29) won't work as intended. See [1] for a report of that. So let's move back to --all-includes to potentially scour our includes for definitions, but supply --include-headers-for-types so we won't racily emit duplicate hunks. See [2] for more discussion about that issue. This undoes most of the optimization we gained with --no-includes, using a very basic benchmark script [3] I get these results: * ~1m50s: --all-includes (before my recent patches) 38 files changed, 74 insertions(+), 78 deletions(-) * ~55s: --no-includes 27 files changed, 55 insertions(+), 57 deletions(-) * ~1m35s: --all-includes --include-headers-for-types 38 files changed, 74 insertions(+), 78 deletions(-) * ~1m20s: --local-includes --include-headers-for-types 36 files changed, 70 insertions(+), 72 deletions(-) * ~1m20s: --local-includes --include-headers-for-types --no-loops 36 files changed, 70 insertions(+), 72 deletions(-) And only the "--all-includes --include-headers-for-types" gets the same results as the previous "--all-includes", even with "--local-includes" we miss out on some definitions. 1. https://lore.kernel.org/git/3aac381e-2ce9-e35e-498c-9c26df235aed@web.de/ 2. https://lore.kernel.org/git/YENdUMLTM+cerfqJ@coredump.intra.peff.net/ 3. for flags in \ '--all-includes' \ '--no-includes' \ '--all-includes --include-headers-for-types' \ '--local-includes --include-headers-for-types' \ '--local-includes --include-headers-for-types --no-loops' do git reset --hard && git clean -dxfq contrib/ time make -j8 coccicheck SPATCH_FLAGS="$flags --patch ." SPATCH_XARGS="xargs -P 8 -n 16" 2>&1 | grep -v SPATCH cat contrib/coccinelle/*.patch | git apply git --no-pager diff --shortstat git --no-pager diff | git patch-id done Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)