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
From: KuhnChris
--- .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 ;; \ - " 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