user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 00/10] first steps towards eliminating TIEHANDLE
@ 2023-10-27 22:21  7% Eric Wong
  2023-10-27 22:21  6% ` [PATCH 03/10] spawn: avoid alloca in C pi_fork_exec Eric Wong
  0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2023-10-27 22:21 UTC (permalink / raw)
  To: meta

ProcessIO being tied is problematic since perlops like stat
and fcntl don't work directly on the handle, and we need
separate classes to do buffered read and unbuffered sysread.

It's possible to do as common packages (e.g. File::Temp,
IO::Socket::IP, IO::Socket::SSL) do by subclassing IO::Handle
to avoid tie completely.  I also want to avoid the self-tie
thing IO::Socket::SSL to avoid having to rely on weaken().

So, this essentially becomes a war on `close($fh)' an relying
on $fh->close where appropriate.  This loses some usefulness
of autodie, so we keep `close($fh)' for places which aren't
tied at the moment.

One key place is to replace many calls to popen_rd with the new
run_qx to consolidate close calls.  There's also few other
cleanups, code reductions, and safety improvements along the way.

And more changes coming.

Eric Wong (10):
  spawn: croak directly in C pi_fork_exec
  spawnpp: use autodie for syscall failures
  spawn: avoid alloca in C pi_fork_exec
  git: use run_qx to read `git --version'
  git: avoid extra stat(2) for git version
  gcf2: simplify pkg-config and Inline::C setup
  treewide: use run_qx where appropriate
  www_altid: reduce FD pressure in qspawn queues
  xt/eml_check_limits: remove useless import
  lei_ale: use v5.12, autodie, and try_cat

 lib/PublicInbox/Admin.pm           |  7 ++---
 lib/PublicInbox/Config.pm          | 14 ++++-----
 lib/PublicInbox/Fetch.pm           |  7 ++---
 lib/PublicInbox/Gcf2.pm            | 27 +++++-----------
 lib/PublicInbox/Git.pm             | 15 +++++----
 lib/PublicInbox/Import.pm          |  6 ++--
 lib/PublicInbox/LEI.pm             |  7 ++---
 lib/PublicInbox/LeiALE.pm          | 24 +++++++--------
 lib/PublicInbox/LeiBlob.pm         | 12 +++-----
 lib/PublicInbox/LeiMirror.pm       |  7 ++---
 lib/PublicInbox/MultiGit.pm        |  7 ++---
 lib/PublicInbox/Spamcheck/Spamc.pm | 11 ++-----
 lib/PublicInbox/Spawn.pm           | 49 +++++++++++++++---------------
 lib/PublicInbox/SpawnPP.pm         | 10 +++---
 lib/PublicInbox/WwwAltId.pm        |  9 ++----
 lib/PublicInbox/XapHelperCxx.pm    | 11 ++-----
 t/solver_git.t                     | 12 +++-----
 xt/eml_check_limits.t              |  4 +--
 18 files changed, 94 insertions(+), 145 deletions(-)


^ permalink raw reply	[relevance 7%]

* [PATCH 03/10] spawn: avoid alloca in C pi_fork_exec
  2023-10-27 22:21  7% [PATCH 00/10] first steps towards eliminating TIEHANDLE Eric Wong
@ 2023-10-27 22:21  6% ` Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2023-10-27 22:21 UTC (permalink / raw)
  To: meta

We don't have thread-safety to worry about, so just leave a few
allocations at process exit at worst.  We'll also update some
comments about usage while we're at it.
---
 lib/PublicInbox/Spawn.pm | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 5740ee58..8382c4d2 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -6,10 +6,8 @@
 # is explicitly defined in the environment (and writable).
 # Under Linux, vfork can make a big difference in spawning performance
 # as process size increases (fork still needs to mark pages for CoW use).
-# Currently, we only use this for code intended for long running
-# daemons (inside the PSGI code (-httpd) and -nntpd).  The short-lived
-# scripts (-mda, -index, -learn, -init) either use IPC::run or standard
-# Perl routines.
+# None of this is intended to be thread-safe since Perl5 maintainers
+# officially discourage the use of threads.
 #
 # There'll probably be more OS-level C stuff here, down the line.
 # We don't want too many DSOs: https://udrepper.livejournal.com/8790.html
@@ -39,12 +37,6 @@ BEGIN {
 #include <time.h>
 #include <stdio.h>
 #include <string.h>
-
-/* some platforms need alloca.h, but some don't */
-#if defined(__GNUC__) && !defined(alloca)
-#  define alloca(sz) __builtin_alloca(sz)
-#endif
-
 #include <signal.h>
 #include <assert.h>
 
@@ -56,11 +48,17 @@ BEGIN {
  *   This is unlike "sv_len", which returns what you would expect.
  */
 #define AV2C_COPY(dst, src) do { \
+	static size_t dst##__capa; \
 	I32 i; \
 	I32 top_index = av_len(src); \
 	I32 real_len = top_index + 1; \
 	I32 capa = real_len + 1; \
-	dst = alloca(capa * sizeof(char *)); \
+	if (capa > dst##__capa) { \
+		dst##__capa = 0; /* in case Newx croaks */ \
+		Safefree(dst); \
+		Newx(dst, capa, char *); \
+		dst##__capa = capa; \
+	} \
 	for (i = 0; i < real_len; i++) { \
 		SV **sv = av_fetch(src, i, 0); \
 		dst[i] = SvPV_nolen(*sv); \
@@ -90,7 +88,7 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
 	AV *rlim = (AV *)SvRV(rlimref);
 	const char *filename = SvPV_nolen(file);
 	pid_t pid = -1;
-	char **argv, **envp;
+	static char **argv, **envp;
 	sigset_t set, old;
 	int ret, perrnum;
 	volatile int cerrnum = 0; /* shared due to vfork */
@@ -172,7 +170,8 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
 		errno = perrnum;
 	}
 out:
-	if (pid < 0) croak("E: fork_exec %s: %s\n", filename, strerror(errno));
+	if (pid < 0)
+		croak("E: fork_exec %s: %s\n", filename, strerror(errno));
 	return (int)pid;
 }
 

^ permalink raw reply related	[relevance 6%]

Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-10-27 22:21  7% [PATCH 00/10] first steps towards eliminating TIEHANDLE Eric Wong
2023-10-27 22:21  6% ` [PATCH 03/10] spawn: avoid alloca in C pi_fork_exec Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).