| Message ID | 835ba84cf1fb7619fa6672d194aaf279795a5246.1659116941.git.edvin.torok@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/ocaml code and build cleanups | expand |
On 29 Jul 2022, at 18:53, Edwin Török <edvin.torok@citrix.com<mailto:edvin.torok@citrix.com>> wrote:
Trying to include .ocamldep.make will cause it to be generated if it
doesn't exist.
We do not want this during make clean: we would remove it anyway.
Speeds up make clean.
Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>
On 29.07.2022 19:53, Edwin Török wrote: > Trying to include .ocamldep.make will cause it to be generated if it > doesn't exist. > We do not want this during make clean: we would remove it anyway. > > Speeds up make clean. > > Before (measured on f732240fd3bac25116151db5ddeb7203b62e85ce, July 2022): > ``` > Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl > Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl > Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl > Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl > Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl > > Performance counter stats for 'make clean -j8 -s' (5 runs): > > 4.2233 +- 0.0208 seconds time elapsed ( +- 0.49% ) > ``` > > After: > ``` > perf stat -r 5 --null make clean -j8 -s > > Performance counter stats for 'make clean -j8 -s' (5 runs): > > 2.7325 +- 0.0138 seconds time elapsed ( +- 0.51% ) > ``` > > No functional change. > > Signed-off-by: Edwin Török <edvin.torok@citrix.com> I've committed this as is since it was acked and is an improvement in any event, but ... > --- a/tools/ocaml/Makefile.rules > +++ b/tools/ocaml/Makefile.rules > @@ -44,8 +44,10 @@ META: META.in > > ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS)) > > +ifneq ($(MAKECMDGOALS),clean) > .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules > $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) > +endif ... what about the distclean goal? Jan
> On 3 Aug 2022, at 11:16, Jan Beulich <jbeulich@suse.com> wrote: > > On 29.07.2022 19:53, Edwin Török wrote: >> Trying to include .ocamldep.make will cause it to be generated if it >> doesn't exist. >> We do not want this during make clean: we would remove it anyway. >> >> Speeds up make clean. >> >> Before (measured on f732240fd3bac25116151db5ddeb7203b62e85ce, July 2022): >> ``` >> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >> >> Performance counter stats for 'make clean -j8 -s' (5 runs): >> >> 4.2233 +- 0.0208 seconds time elapsed ( +- 0.49% ) >> ``` >> >> After: >> ``` >> perf stat -r 5 --null make clean -j8 -s >> >> Performance counter stats for 'make clean -j8 -s' (5 runs): >> >> 2.7325 +- 0.0138 seconds time elapsed ( +- 0.51% ) >> ``` >> >> No functional change. >> >> Signed-off-by: Edwin Török <edvin.torok@citrix.com> > > I've committed this as is since it was acked and is an improvement > in any event, but ... > >> --- a/tools/ocaml/Makefile.rules >> +++ b/tools/ocaml/Makefile.rules >> @@ -44,8 +44,10 @@ META: META.in >> >> ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS)) >> >> +ifneq ($(MAKECMDGOALS),clean) >> .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules >> $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) >> +endif > > ... what about the distclean goal? Thanks for the suggestion, I see other Makefiles using 'findstring clean', would that be appropriate here? Something like this perhaps? From 53cbf81a9c7d5090443b5fe5de37a47118ac53d8 Mon Sep 17 00:00:00 2001 Message-Id: <53cbf81a9c7d5090443b5fe5de37a47118ac53d8.1659522178.git.edvin.torok@citrix.com> From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edvin.torok@citrix.com> To: xen-devel@lists.xenproject.org Cc: Christian Lindig <christian.lindig@citrix.com> Cc: David Scott <dave@recoil.org> Cc: Wei Liu <wl@xen.org> Cc: Anthony PERARD <anthony.perard@citrix.com> Date: Wed, 3 Aug 2022 11:21:46 +0100 Subject: [PATCH] tools/ocaml/Makefile.rules: do not run ocamldep on distclean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Edwin Török <edvin.torok@citrix.com> --- tools/ocaml/Makefile.rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ocaml/Makefile.rules b/tools/ocaml/Makefile.rules index d368308d9b..c1c5dd3b39 100644 --- a/tools/ocaml/Makefile.rules +++ b/tools/ocaml/Makefile.rules @@ -44,7 +44,7 @@ META: META.in ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS)) -ifneq ($(MAKECMDGOALS),clean) +ifeq (,$(findstring clean,$(MAKECMDGOALS))) .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) endif -- 2.34.1
On 03.08.2022 12:24, Edwin Torok wrote: > > >> On 3 Aug 2022, at 11:16, Jan Beulich <jbeulich@suse.com> wrote: >> >> On 29.07.2022 19:53, Edwin Török wrote: >>> Trying to include .ocamldep.make will cause it to be generated if it >>> doesn't exist. >>> We do not want this during make clean: we would remove it anyway. >>> >>> Speeds up make clean. >>> >>> Before (measured on f732240fd3bac25116151db5ddeb7203b62e85ce, July 2022): >>> ``` >>> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >>> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >>> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >>> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >>> Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl >>> >>> Performance counter stats for 'make clean -j8 -s' (5 runs): >>> >>> 4.2233 +- 0.0208 seconds time elapsed ( +- 0.49% ) >>> ``` >>> >>> After: >>> ``` >>> perf stat -r 5 --null make clean -j8 -s >>> >>> Performance counter stats for 'make clean -j8 -s' (5 runs): >>> >>> 2.7325 +- 0.0138 seconds time elapsed ( +- 0.51% ) >>> ``` >>> >>> No functional change. >>> >>> Signed-off-by: Edwin Török <edvin.torok@citrix.com> >> >> I've committed this as is since it was acked and is an improvement >> in any event, but ... >> >>> --- a/tools/ocaml/Makefile.rules >>> +++ b/tools/ocaml/Makefile.rules >>> @@ -44,8 +44,10 @@ META: META.in >>> >>> ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS)) >>> >>> +ifneq ($(MAKECMDGOALS),clean) >>> .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules >>> $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) >>> +endif >> >> ... what about the distclean goal? > > > Thanks for the suggestion, I see other Makefiles using 'findstring clean', would that be appropriate here? Hmm, not sure this couldn't end up suppressing the rul when it's needed. How about "ifeq ($(filter-out %clean,$(MAKECMDGOALS)),)"? (Off the top of my head I don't recall whether this may additionally need wrapping in $(strip ...).) Jan
On Wed, Aug 03, 2022 at 10:24:26AM +0000, Edwin Torok wrote: > > -ifneq ($(MAKECMDGOALS),clean) > +ifeq (,$(findstring clean,$(MAKECMDGOALS))) I think it would be better with $(filter-out,): ifeq (,$(filter-out %clean,$(MAKECMDGOALS))) > .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules > $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) Also, don't hide this rule, instead, hide the "-include", there is no need to have make waist time trying to find a rule to make ".ocamldep.make" and failing when not needed. > endif > -- > 2.34.1
On 03.08.2022 12:57, Anthony PERARD wrote: > On Wed, Aug 03, 2022 at 10:24:26AM +0000, Edwin Torok wrote: >> >> -ifneq ($(MAKECMDGOALS),clean) >> +ifeq (,$(findstring clean,$(MAKECMDGOALS))) > > I think it would be better with $(filter-out,): > > ifeq (,$(filter-out %clean,$(MAKECMDGOALS))) > >> .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules >> $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) > > Also, don't hide this rule, instead, hide the "-include", there is no > need to have make waist time trying to find a rule to make > ".ocamldep.make" and failing when not needed. Hmm, this sounds like I should be reverting the commit? Jan
On Wed, Aug 03, 2022 at 01:58:34PM +0200, Jan Beulich wrote: > On 03.08.2022 12:57, Anthony PERARD wrote: > > On Wed, Aug 03, 2022 at 10:24:26AM +0000, Edwin Torok wrote: > >> > >> -ifneq ($(MAKECMDGOALS),clean) > >> +ifeq (,$(findstring clean,$(MAKECMDGOALS))) > > > > I think it would be better with $(filter-out,): > > > > ifeq (,$(filter-out %clean,$(MAKECMDGOALS))) > > > >> .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules > >> $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) > > > > Also, don't hide this rule, instead, hide the "-include", there is no > > need to have make waist time trying to find a rule to make > > ".ocamldep.make" and failing when not needed. > > Hmm, this sounds like I should be reverting the commit? Well, it's not exactly an issue as there isn't any alternative rules, and make is told to ignore failures to make ".ocamldep.make"; so `make clean` and other targets still works as expected. Just a follow-up patch would be fine I think.
diff --git a/tools/ocaml/Makefile.rules b/tools/ocaml/Makefile.rules index 7e4db457a1..d368308d9b 100644 --- a/tools/ocaml/Makefile.rules +++ b/tools/ocaml/Makefile.rules @@ -44,8 +44,10 @@ META: META.in ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS)) +ifneq ($(MAKECMDGOALS),clean) .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) +endif clean: $(CLEAN_HOOKS) $(Q)rm -f .*.d *.o *.so *.a *.cmo *.cmi *.cma *.cmx *.cmxa *.annot *.spot *.spit $(LIBS) $(PROGRAMS) $(GENERATED_FILES) .ocamldep.make META
Trying to include .ocamldep.make will cause it to be generated if it doesn't exist. We do not want this during make clean: we would remove it anyway. Speeds up make clean. Before (measured on f732240fd3bac25116151db5ddeb7203b62e85ce, July 2022): ``` Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl Parsing /home/edwin/xen2/tools/ocaml/libs/xl/../../../../tools/libs/light/libxl_types.idl Performance counter stats for 'make clean -j8 -s' (5 runs): 4.2233 +- 0.0208 seconds time elapsed ( +- 0.49% ) ``` After: ``` perf stat -r 5 --null make clean -j8 -s Performance counter stats for 'make clean -j8 -s' (5 runs): 2.7325 +- 0.0138 seconds time elapsed ( +- 0.51% ) ``` No functional change. Signed-off-by: Edwin Török <edvin.torok@citrix.com> --- tools/ocaml/Makefile.rules | 2 ++ 1 file changed, 2 insertions(+)