Skip to content

Commit 316e388

Browse files
avarttaylorr
authored andcommitted
cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES
Improve the incremental rebuilding support of "coccicheck" by piggy-backing on the computed dependency information of the corresponding *.o file, rather than rebuilding all <RULE>/<FILE> pairs if either their corresponding file changes, or if any header changes. This in effect uses the same method that the "sparse" target was made to use in c234e8a (Makefile: make the "sparse" target non-.PHONY, 2021-09-23), except that the dependency on the *.o file isn't a hard one, we check with $(wildcard) if the *.o file exists, and if so we'll depend on it. This means that the common case of: make make coccicheck Will benefit from incremental rebuilding, now changing e.g. a header will only re-run "spatch" on those those *.c files that make use of it: By depending on the *.o we piggy-back on COMPUTE_HEADER_DEPENDENCIES. See c234e8a (Makefile: make the "sparse" target non-.PHONY, 2021-09-23) for prior art of doing that for the *.sp files. E.g.: make contrib/coccinelle/free.cocci.patch make -W column.h contrib/coccinelle/free.cocci.patch Will take around 15 seconds for the second command on my 8 core box if I didn't run "make" beforehand to create the *.o files. But around 2 seconds if I did and we have those "*.o" files. Notes about the approach of piggy-backing on *.o for dependencies: * It *is* a trade-off since we'll pay the extra cost of running the C compiler, but we're probably doing that anyway. The compiler is much faster than "spatch", so even though we need to re-compile the *.o to create the dependency info for the *.c for "spatch" it's faster (especially if using "ccache"). * There *are* use-cases where some would like to have *.o files around, but to have the "make coccicheck" ignore them. See: https://lore.kernel.org/git/[email protected]/ For those users a: make make coccicheck SPATCH_USE_O_DEPENDENCIES= Will avoid considering the *.o files. * If that *.o file doesn't exist we'll depend on an intermediate file of ours which in turn depends on $(FOUND_H_SOURCES). This covers both an initial build, or where "coccicheck" is run without running "all" beforehand, and because we run "coccicheck" on e.g. files in compat/* that we don't know how to build unless the requisite flag was provided to the Makefile. Most of the runtime of "incremental" runs is now spent on various compat/* files, i.e. we conditionally add files to COMPAT_OBJS, and therefore conflate whether we *can* compile an object and generate dependency information for it with whether we'd like to link it into our binary. Before this change the distinction didn't matter, but now one way to make this even faster on incremental builds would be to peel those concerns apart so that we can see that e.g. compat/mmap.c doesn't depend on column.h. Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent f1c903d commit 316e388

File tree

2 files changed

+29
-2
lines changed

2 files changed

+29
-2
lines changed

Makefile

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,6 +1299,13 @@ SPATCH_INCLUDE_FLAGS = --all-includes
12991299
SPATCH_FLAGS =
13001300
SPATCH_TEST_FLAGS =
13011301

1302+
# If *.o files are present, have "coccicheck" depend on them, with
1303+
# COMPUTE_HEADER_DEPENDENCIES this will speed up the common-case of
1304+
# only needing to re-generate coccicheck results for the users of a
1305+
# given API if it's changed, and not all files in the project. If
1306+
# COMPUTE_HEADER_DEPENDENCIES=no this will be unset too.
1307+
SPATCH_USE_O_DEPENDENCIES = YesPlease
1308+
13021309
# Rebuild 'coccicheck' if $(SPATCH), its flags etc. change
13031310
TRACK_SPATCH_DEFINES =
13041311
TRACK_SPATCH_DEFINES += $(SPATCH)
@@ -3176,14 +3183,18 @@ COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
31763183
$(call mkdir_p_parent_template)
31773184
$(QUIET_GEN) >$@
31783185

3186+
ifeq ($(COMPUTE_HEADER_DEPENDENCIES),no)
3187+
SPATCH_USE_O_DEPENDENCIES =
3188+
endif
31793189
define cocci-rule
31803190

31813191
## Rule for .build/$(1).patch/$(2); Params:
31823192
# $(1) = e.g. "free.cocci"
31833193
# $(2) = e.g. "grep.c"
3194+
# $(3) = e.g. "grep.o"
31843195
COCCI_$(1:contrib/coccinelle/%.cocci=%) += .build/$(1).patch/$(2)
31853196
.build/$(1).patch/$(2): GIT-SPATCH-DEFINES
3186-
.build/$(1).patch/$(2): .build/contrib/coccinelle/FOUND_H_SOURCES
3197+
.build/$(1).patch/$(2): $(if $(and $(SPATCH_USE_O_DEPENDENCIES),$(wildcard $(3))),$(3),.build/contrib/coccinelle/FOUND_H_SOURCES)
31873198
.build/$(1).patch/$(2): $(1)
31883199
.build/$(1).patch/$(2): .build/$(1).patch/% : %
31893200
$$(call mkdir_p_parent_template)
@@ -3200,7 +3211,7 @@ endef
32003211

32013212
define cocci-matrix
32023213

3203-
$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(1),$(s)))
3214+
$(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s),$(s:%.c=%.o)))
32043215
endef
32053216

32063217
ifdef COCCI_GOALS

contrib/coccinelle/README

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,19 @@ There are two types of semantic patches:
4141

4242
This allows to expose plans of pending large scale refactorings without
4343
impacting the bad pattern checks.
44+
45+
Git-specific tips & things to know about how we run "spatch":
46+
47+
* The "make coccicheck" will piggy-back on
48+
"COMPUTE_HEADER_DEPENDENCIES". If you've built a given object file
49+
the "coccicheck" target will consider its depednency to decide if
50+
it needs to re-run on the corresponding source file.
51+
52+
This means that a "make coccicheck" will re-compile object files
53+
before running. This might be unexpected, but speeds up the run in
54+
the common case, as e.g. a change to "column.h" won't require all
55+
coccinelle rules to be re-run against "grep.c" (or another file
56+
that happens not to use "column.h").
57+
58+
To disable this behavior use the "SPATCH_USE_O_DEPENDENCIES=NoThanks"
59+
flag.

0 commit comments

Comments
 (0)