[PATCH] MAKE: Fix parallel builds; .o files; .gitignore; new makedocs

[PATCH] MAKE: Fix parallel builds; .o files; .gitignore; new makedocs

[KuhnChris]

From: KuhnChris <kuhnchris@kuhnchris.eu>

---
 .gitignore  |  1 +
 Makefile    | 51 +++++++++++++++++++++++++--------------------------
 makedocs.pl | 19 +++++++++++++++++++
 3 files changed, 45 insertions(+), 26 deletions(-)
 create mode 100644 makedocs.pl

diff --git a/.gitignore b/.gitignore
index d3d0e2c..caeafa5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 *~
+*.o
 /passt
 /passt.avx2
 /pasta
diff --git a/Makefile b/Makefile
index a5256f5..4c99771 100644
--- a/Makefile
+++ b/Makefile
@@ -100,29 +100,41 @@ else
 BIN := passt pasta qrap
 endif
 
-all: $(BIN) $(MANPAGES) docs
+.NOTPARALLEL: seccomp.h
+all: seccomp.h $(BIN) $(MANPAGES) docs
+
+PASST_OBJS = $(PASST_SRCS:.c=.o)
+PASST_AVX2_OBJS = $(PASST_SRCS:.c=.avx2.o)
+OBJS = $(SRCS:.c=.o)
+QRAP_OBJS = $(QRAP_SRCS:.c=.o)
+AVXFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops
 
 static: FLAGS += -static -DGLIBC_NO_STATIC_NSS
-static: clean all
+static: clean seccomp.h all
 
 seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
 	@ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
 
-passt: $(PASST_SRCS) $(HEADERS)
-	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
+.c.o:
+	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -c $< -o $@
+
+$(PASST_AVX2_OBJS): seccomp.h
+	$(CC) $(AVXFLAGS) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -c $(@:.avx2.o=.c) -o $@
+
+passt: $(PASST_OBJS) seccomp.h
+	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_OBJS) -o passt $(LDFLAGS)
 
-passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops
-passt.avx2: $(PASST_SRCS) $(HEADERS)
-	$(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(CPPFLAGS) \
-		$(PASST_SRCS) -o passt.avx2 $(LDFLAGS)
+passt.avx2: $(PASST_AVX2_OBJS) $(HEADERS) seccomp.h
+	$(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(CPPFLAGS) $(AVXFLAGS) \
+		$(PASST_AVX2_OBJS) -o passt.avx2 $(LDFLAGS)
 
-passt.avx2: passt
+passt.avx2: passt seccomp.h
 
 pasta.avx2 pasta.1 pasta: pasta%: passt%
 	ln -sf $< $@
 
-qrap: $(QRAP_SRCS) passt.h
-	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(QRAP_SRCS) -o qrap $(LDFLAGS)
+qrap: $(QRAP_OBJS) passt.h seccomp.h
+	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(QRAP_OBJS) -o qrap $(LDFLAGS)
 
 valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction	\
 			    getpid gettid kill clock_gettime mmap	\
@@ -170,21 +182,8 @@ pkgs: static
 # other way around: the web version should be obtained by adding HTML and
 # JavaScript portions to a plain Markdown, instead. However, cgit needs to use
 # a file in the git tree. Find a better way around this.
-docs: README.md
-	@(								\
-		skip=0;							\
-		while read l; do					\
-			case $$l in					\
-			"## Demo")	exit 0		;;		\
-			"<!"*)				;;		\
-			"</"*)		skip=1		;;		\
-			"<"*)		skip=2		;;		\
-			esac;						\
-									\
-			[ $$skip -eq 0 ]	&& echo "$$l";		\
-			[ $$skip -eq 1 ]	&& skip=0;		\
-		done < README.md;					\
-	) > README.plain.md
+docs:
+	[ ! -e doc/ ]; mkdir docs; perl makedocs.pl > docs/README.plain.md
 
 # Checkers currently disabled for clang-tidy:
 # - llvmlibc-restrict-system-libc-headers
diff --git a/makedocs.pl b/makedocs.pl
new file mode 100644
index 0000000..295e1d9
--- /dev/null
+++ b/makedocs.pl
@@ -0,0 +1,19 @@
+use strict;
+
+my $str = '';
+my $regex = qr/(<[^\/].*?>.*?<\/.*?>\n?)|(<\/div>)|(<\/p>)/msp;
+my $subst = '';
+
+
+local $/=undef;
+open FILE, '<', 'README.md' or die "Can't open file $!";
+my $file_content = <FILE>;
+close FILE;
+#print "Source: $file_content\n";
+my $result = $file_content =~ s/$regex//rg;
+
+my $regex2 = qr/\n[ \n]{3,}\n/msp;
+$result = $result =~ s/$regex2/\n\n/rg;
+
+#print "The result of the substitution is: $result\n";
+print "$result\n";
-- 
@@ -0,0 +1,19 @@
+use strict;
+
+my $str = '';
+my $regex = qr/(<[^\/].*?>.*?<\/.*?>\n?)|(<\/div>)|(<\/p>)/msp;
+my $subst = '';
+
+
+local $/=undef;
+open FILE, '<', 'README.md' or die "Can't open file $!";
+my $file_content = <FILE>;
+close FILE;
+#print "Source: $file_content\n";
+my $result = $file_content =~ s/$regex//rg;
+
+my $regex2 = qr/\n[ \n]{3,}\n/msp;
+$result = $result =~ s/$regex2/\n\n/rg;
+
+#print "The result of the substitution is: $result\n";
+print "$result\n";
-- 
2.30.2

Re: [PATCH] MAKE: Fix parallel builds; .o files; .gitignore; new makedocs

[Stefano Brivio]

Thanks for the patch, and sorry for the delay!

First off, as I was mentioning on IRC: it would help to have this as
two/three patches -- not just because it's easier to review, but also
because the Makefile changes have, I guess, a relatively high risk of
introducing some regressions, so keeping it simple for 'git bisect'
would also be a plus here.

If you want to split this in your working tree, you can do something
like:

  git reset HEAD~

meaning: reset (the git state, not the file contents) to the commit
before head (~ is a shortcut for "just before"). Then 'git add' your
pieces back -- and if you have two changes to the Makefile you want to
keep separated, go with 'git add -i', which lets you add just parts.

If that's not your HEAD anymore, 'git rebase -i' to something before
this commit, and do the same git reset/git add trick, then 'git rebase
--continue'.

Another general comment (about the title): this doesn't actually fix
parallel builds... it makes builds more parallel.

Now, to the changes:

On Wed, 28 Jun 2023 16:07:28 +0200
KuhnChris <kuhnchris+git@kuhnchris.eu> wrote:

> From: KuhnChris <kuhnchris@kuhnchris.eu>
> 
> ---
>  .gitignore  |  1 +
>  Makefile    | 51 +++++++++++++++++++++++++--------------------------
>  makedocs.pl | 19 +++++++++++++++++++
>  3 files changed, 45 insertions(+), 26 deletions(-)
>  create mode 100644 makedocs.pl
> 
> diff --git a/.gitignore b/.gitignore
> index d3d0e2c..caeafa5 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,4 +1,5 @@
>  *~
> +*.o
>  /passt
>  /passt.avx2
>  /pasta
> diff --git a/Makefile b/Makefile
> index a5256f5..4c99771 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -100,29 +100,41 @@ else
>  BIN := passt pasta qrap
>  endif
>  
> -all: $(BIN) $(MANPAGES) docs
> +.NOTPARALLEL: seccomp.h
> +all: seccomp.h $(BIN) $(MANPAGES) docs
> +
> +PASST_OBJS = $(PASST_SRCS:.c=.o)
> +PASST_AVX2_OBJS = $(PASST_SRCS:.c=.avx2.o)
> +OBJS = $(SRCS:.c=.o)
> +QRAP_OBJS = $(QRAP_SRCS:.c=.o)
> +AVXFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops

This all looks correct to me, but for some reason, on a machine
with 12 (free-ish) CPU threads, 'make -j1' takes the same time as
'make -j10', and seems to lead to the same amount of parallel gcc
processes... no idea why, yet. I would have expected actual parallelism
resulting from this.

By the way, build time (after a make clean) is the same as without this
change, with -j1 or -j10.

I know you're introducing this to avoid a full rebuild if only some
source files changed, which is an advantage in any case, but I'm
missing something...

>  
>  static: FLAGS += -static -DGLIBC_NO_STATIC_NSS
> -static: clean all
> +static: clean seccomp.h all
>  
>  seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
>  	@ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
>  
> -passt: $(PASST_SRCS) $(HEADERS)
> -	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
> +.c.o:
> +	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -c $< -o $@
> +
> +$(PASST_AVX2_OBJS): seccomp.h
> +	$(CC) $(AVXFLAGS) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -c $(@:.avx2.o=.c) -o $@
> +
> +passt: $(PASST_OBJS) seccomp.h
> +	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_OBJS) -o passt $(LDFLAGS)
>  
> -passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops
> -passt.avx2: $(PASST_SRCS) $(HEADERS)
> -	$(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(CPPFLAGS) \
> -		$(PASST_SRCS) -o passt.avx2 $(LDFLAGS)
> +passt.avx2: $(PASST_AVX2_OBJS) $(HEADERS) seccomp.h
> +	$(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(CPPFLAGS) $(AVXFLAGS) \
> +		$(PASST_AVX2_OBJS) -o passt.avx2 $(LDFLAGS)
>  
> -passt.avx2: passt
> +passt.avx2: passt seccomp.h
>  
>  pasta.avx2 pasta.1 pasta: pasta%: passt%
>  	ln -sf $< $@
>  
> -qrap: $(QRAP_SRCS) passt.h
> -	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(QRAP_SRCS) -o qrap $(LDFLAGS)
> +qrap: $(QRAP_OBJS) passt.h seccomp.h
> +	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(QRAP_OBJS) -o qrap $(LDFLAGS)
>  
>  valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction	\
>  			    getpid gettid kill clock_gettime mmap	\
> @@ -170,21 +182,8 @@ pkgs: static
>  # other way around: the web version should be obtained by adding HTML and
>  # JavaScript portions to a plain Markdown, instead. However, cgit needs to use
>  # a file in the git tree. Find a better way around this.
> -docs: README.md
> -	@(								\
> -		skip=0;							\
> -		while read l; do					\
> -			case $$l in					\
> -			"## Demo")	exit 0		;;		\
> -			"<!"*)				;;		\
> -			"</"*)		skip=1		;;		\
> -			"<"*)		skip=2		;;		\
> -			esac;						\
> -									\
> -			[ $$skip -eq 0 ]	&& echo "$$l";		\
> -			[ $$skip -eq 1 ]	&& skip=0;		\
> -		done < README.md;					\
> -	) > README.plain.md
> +docs:

The output is much more complete now (and the Makefile cleaner).

> +	[ ! -e doc/ ]; mkdir docs; perl makedocs.pl > docs/README.plain.md

Shouldn't we (naturally) use Makefile dependencies for this? Also note
that you're moving the README.plain.md file to docs/, so you would also
need to change the install target:

        cp -d README.plain.md $(DESTDIR)$(docdir)/README.md

...but, anyway, I think it would make more sense to keep the output in
the root directory (simply because README.md is already there, so one
can more easily spot the "plain" version).

>  
>  # Checkers currently disabled for clang-tidy:
>  # - llvmlibc-restrict-system-libc-headers
> diff --git a/makedocs.pl b/makedocs.pl
> new file mode 100644

This should probably have 0755 permissions, it's an interpreted
executable after all.

> index 0000000..295e1d9
> --- /dev/null
> +++ b/makedocs.pl

This makes it sound like it does more than just stripping away the HTML
stuff. Perhaps make_plain_readme.pl, or plain_readme.pl,
strip_html.pl... something like that?

> @@ -0,0 +1,19 @@

Missing license header:

# SPDX-License-Identifier: GPL-2.0-or-later

and:

#!/usr/bin/perl

> +use strict;
> +
> +my $str = '';
> +my $regex = qr/(<[^\/].*?>.*?<\/.*?>\n?)|(<\/div>)|(<\/p>)/msp;
> +my $subst = '';
> +
> +
> +local $/=undef;
> +open FILE, '<', 'README.md' or die "Can't open file $!";
> +my $file_content = <FILE>;
> +close FILE;
> +#print "Source: $file_content\n";
> +my $result = $file_content =~ s/$regex//rg;
> +
> +my $regex2 = qr/\n[ \n]{3,}\n/msp;
> +$result = $result =~ s/$regex2/\n\n/rg;
> +
> +#print "The result of the substitution is: $result\n";
> +print "$result\n";

This looks good to me... I'm not sure if you intended to leave in those
comments, but I'm fine either way.

-- 
Stefano

Re: [PATCH] MAKE: Fix parallel builds; .o files; .gitignore; new makedocs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

On Fri, Jun 30, 2023 at 07:16:19PM +0200, Stefano Brivio wrote:
> Thanks for the patch, and sorry for the delay!
> 
> First off, as I was mentioning on IRC: it would help to have this as
> two/three patches -- not just because it's easier to review, but also
> because the Makefile changes have, I guess, a relatively high risk of
> introducing some regressions, so keeping it simple for 'git bisect'
> would also be a plus here.
> 
> If you want to split this in your working tree, you can do something
> like:
> 
>   git reset HEAD~
> 
> meaning: reset (the git state, not the file contents) to the commit
> before head (~ is a shortcut for "just before"). Then 'git add' your
> pieces back -- and if you have two changes to the Makefile you want to
> keep separated, go with 'git add -i', which lets you add just parts.

git citool also allows doing this interactively quite nicely.  You can
move files, or individual hunks or lines between being in the commit,
or not.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]