[lit] Run builtin cat / diff in-process instead of spawning

[prasoon054]

cat and diff are the only two builtins that still spawn a subprocess: every cat/diff on a RUN line spawns a fresh Python interpreter, which dominates wall time given how small lit's typical inputs are. Run them in-process instead.

The spawned-script path stays as a fallback for 'env VAR=... cat/diff' and 'not --crash cat/diff' for now. Removing it entirely is a follow-up PR. Output is byte-identical to the spawn path either way.

diff.py also switches its four output-encode sites from locale.getpreferredencoding to 'utf-8', fixing a UnicodeEncodeError that crashed Windows CI.

I chose six suites based on high cat / diff RUN-line density: Interpreter (47.4%), Integer (30.8%), UpdateTestChecks (28.1%), Feature (22.0%), ClangScanDeps (7.8%), and ExtractAPI (6.9%). Results:

	Wall clock (median / mean)	Peak RSS (median)
Baseline (spawn)	67.656 s / 67.756 s	201.7 MiB
This change (in-process)	63.018 s / 63.065 s	201.7 MiB
Delta	-4.64 s median (~6.9% speedup)	0 MiB

[llvmorg-github-actions]

@llvm/pr-subscribers-testing-tools

Author: Prasoon Kumar (prasoon054)

Changes

Every cat / diff invocation in a RUN line spawned a fresh Python interpretor (~20-40ms), which dominates wall time given how tiny lit's typical inputs are. Run these two builtins in-process instead, with the spawned-script path kept only as a fallback for cases an in-process function call cannot honor: 'env VAR=... cat/diff' (needs a per-command environment/locale) and 'not --crash cat/diff' (needs to die by signal). Output is byte-identical to the spawn path.

A pipeline stage that isn't last spools its output in memory and only spills to disk past 1 MiB, or when a downstream external stage forces a real fd out of it

---

Full diff: https://github.com/llvm/llvm-project/pull/208024.diff

2 Files Affected:

• (modified) llvm/utils/lit/lit/ShellEnvironment.py (+53)
• (modified) llvm/utils/lit/lit/TestRunner.py (+124-3)

diff --git a/llvm/utils/lit/lit/ShellEnvironment.py b/llvm/utils/lit/lit/ShellEnvironment.py
index 1945865b19199..066aca416e9d4 100644
--- a/llvm/utils/lit/lit/ShellEnvironment.py
+++ b/llvm/utils/lit/lit/ShellEnvironment.py
@@ -1,3 +1,4 @@
+import io
 import os
 import platform
 import subprocess
@@ -206,6 +207,58 @@ def processRedirects(cmd, stdin_source, cmd_shenv, opened_files):
     return std_fds
 
 
+def as_binary_reader(stream):
+    """Adapts an in-process builtin's stdin source into a binary, read()-able stream.
+
+    Args:
+        stream: The stdin source handed to the builtin by the pipeline dispatch:
+            None or a subprocess sentinel (subprocess.PIPE/DEVNULL/STDOUT) for no
+            input, a binary stream (BytesIO, a spool/temp file, a file opened
+            "rb"), or a text stream (a '<' redirect opened in text mode, or an
+            upstream stage's universal_newlines pipe).
+
+    Returns:
+        A binary, read()-able stream. Text streams are unwrapped to their
+        underlying buffer so cat/diff see the same raw bytes a real child
+        process would, with no newline translation.
+    """
+    if stream is None or isinstance(stream, int):
+        # No real input to read.
+        return io.BytesIO(b"")
+    if isinstance(stream, io.TextIOBase):
+        buffer = getattr(stream, "buffer", None)
+        if buffer is not None:
+            return buffer
+        data = stream.read()
+        return io.BytesIO(data.encode() if isinstance(data, str) else data)
+    # Already a binary reader.
+    return stream
+
+
+class BinaryFileWriter:
+    """Writes bytes straight to a redirect file's fd, matching Popen's behavior.
+
+    processRedirects opens '>'/'>>'/'2>' targets in text mode, which is fine for
+    the subprocess path since Popen writes raw bytes to fileno() and bypasses the
+    text wrapper. An in-process builtin hands us bytes directly instead, so this
+    routes them to the same fd with os.write -- byte-exact, no newline
+    translation, no double encoding.
+    """
+
+    __slots__ = ("fd",)
+
+    def __init__(self, fileobj):
+        self.fd = fileobj.fileno()  # fd is owned by opened_files; never closed here
+
+    def write(self, data):
+        return os.write(self.fd, data)
+
+
+def binary_fd(fileobj):
+    """Wraps a redirect file object as a byte-exact writer for in-process builtins."""
+    return BinaryFileWriter(fileobj)
+
+
 def expand_glob(arg, cwd):
     if isinstance(arg, GlobItem):
         return sorted(arg.resolve(cwd))
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 97cb54c63be64..6628ea965a999 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import enum
+import io
 import os
 import pathlib
 import re
@@ -16,12 +17,16 @@
 import lit.ShUtil as ShUtil
 import lit.Test as Test
 import lit.util
+import lit.builtin_commands.cat as builtin_cat
+import lit.builtin_commands.diff as builtin_diff
 from lit.BooleanExpression import BooleanExpression
 from lit.ShCommands import Command
 from lit.ShellEnvironment import (
     InternalShellError,
     ShellCommandResult,
     ShellEnvironment,
+    as_binary_reader,
+    binary_fd,
     expand_glob,
     expand_glob_expressions,
     kAvoidDevNull,
@@ -212,6 +217,65 @@ def _replaceReadFile(match):
     return arguments
 
 
+class InProcessPipe:
+    """Popen-compatible shim that runs an in-process builtin (cat/diff) inline.
+
+    We want lit's cat/diff RUN lines to stop paying a fresh Python-interpreter
+    spawn on every invocation -- that overhead (~20-40ms) dwarfs the actual work
+    on lit's typically tiny inputs. The current implementation always shells out
+    to cat.py/diff.py as a separate sys.executable subprocess. To close that gap,
+    this shim instead runs cat.run/diff.run directly in the worker process and
+    exposes the same communicate()/wait()/poll()/stdout/stderr surface a
+    finished Popen would, so the rest of _executeShCmd's result loop and pipeline
+    composition work unchanged -- unblocking an all-builtin chain like
+    'cat %t | diff - %t2' to run with zero subprocess spawns.
+    """
+
+    # TODO: Replace __slots__ with @dataclass(slots=True)
+    # once the minimum Python version is bumped to 3.10
+    # https://github.com/llvm/llvm-project/issues/200531
+    __slots__ = ("returncode", "stdout", "stderr", "_out", "_err")
+
+    def __init__(self, run_fn, args, stdin, out_sink, redirect_out, capture_out, err_sink, cwd, merge_err=False):
+        in_stream = as_binary_reader(stdin)
+        out_buf = io.BytesIO() if capture_out else None
+        out_target = out_buf if capture_out else (redirect_out or out_sink)
+        if merge_err:
+            # 2>&1: route stderr wherever stdout is going.
+            err_target, err_buf = out_target, None
+        elif err_sink is not None:
+            # 2>file: caller already gave us a target.
+            err_target, err_buf = err_sink, None
+        else:
+            err_buf = io.BytesIO()
+            err_target = err_buf
+
+        self.returncode = run_fn(args, in_stream, out_target, err_target, cwd)
+
+        self._out = out_buf.getvalue() if out_buf is not None else b""
+        self._err = err_buf.getvalue() if err_buf is not None else b""
+        # .stdout is non-None only when this stage captured (last stage on a
+        # pipe); otherwise a real Popen would report empty here too, since
+        # output already went to the temp file/redirect.
+        self.stdout = io.BytesIO(self._out) if capture_out else None
+        self.stderr = io.BytesIO(self._err)
+
+    def communicate(self):
+        return (self._out, self._err)
+
+    def wait(self):
+        return self.returncode
+
+    def poll(self):
+        return self.returncode
+
+    def kill(self):
+        pass
+
+    def terminate(self):
+        pass
+
+
 def _executeShCmd(cmd, shenv, results, timeoutHelper):
     if timeoutHelper.timeoutReached():
         # Prevent further recursion if the timeout has been hit
@@ -268,6 +332,11 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
         "umask": InprocBuiltins.executeBuiltinUmask,
         ":": InprocBuiltins.executeBuiltinColon,
     }
+    pipeline_builtins = {
+        # cat/diff run in-process via these run() cores, wrapped in InProcessPipe.
+        "cat": builtin_cat.run,
+        "diff": builtin_diff.run,
+    }
     # To avoid deadlock, we use a single stderr stream for piped
     # output. This is null until we have seen some output using
     # stderr.
@@ -362,13 +431,17 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
             results.append(result)
             return result.exitCode
 
-        # Resolve any out-of-process builtin command before adding back 'not'
-        # commands.
-        if args[0] in builtin_commands:
+        builtin_fn = pipeline_builtins.get(args[0])
+        # Fall back to spawning the script only where in-process can't cope:
+        # a per-command 'env' (diff also reads locale) or 'not --crash' (a
+        # function call can't die by signal).
+        use_inproc = builtin_fn is not None and not not_crash and cmd_shenv is shenv
+        if not use_inproc and args[0] in builtin_commands:
             args.insert(0, sys.executable)
             cmd_shenv.env["PYTHONPATH"] = os.path.dirname(os.path.abspath(__file__))
             args[1] = os.path.join(builtin_commands_dir, args[1] + ".py")
 
+
         # We had to search through the 'not' commands to find all the 'env'
         # commands and any other in-process builtin command.  We don't want to
         # reimplement 'not' and its '--crash' here, so just push all 'not'
@@ -395,6 +468,54 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
             j, default_stdin, cmd_shenv, opened_files
         )
 
+        if use_inproc:
+            if kAvoidDevNull:
+                for arg_idx, arg in enumerate(args):
+                    if isinstance(arg, str) and kDevNull in arg:
+                        devnull = tempfile.NamedTemporaryFile(delete=False)
+                        devnull.close()
+                        named_temp_files.append(devnull.name)
+                        args[arg_idx] = arg.replace(kDevNull, devnull.name)
+            args = expand_glob_expressions(args, cmd_shenv.cwd)
+            is_last = j is cmd.commands[-1]
+            merge_err = stderr == subprocess.STDOUT
+            redirect_out = (
+                binary_fd(stdout)
+                if stdout not in (subprocess.PIPE, subprocess.STDOUT)
+                else None
+            )
+            capture_out = is_last and stdout == subprocess.PIPE  # only the last stage reports output to the caller
+            out_sink = (
+                tempfile.SpooledTemporaryFile(max_size=1<<20)  # stays in-memory unless size or a downstream .fileno() forces it to disk
+                if stdout == subprocess.PIPE and not is_last
+                else None
+            )
+            err_sink = (
+                binary_fd(stderr)  # 2>file target; 2>&1 is already handled via merge_err
+                if not merge_err and stderr not in (subprocess.PIPE, subprocess.STDOUT)
+                else None
+            )
+            procs.append(
+                InProcessPipe(
+                    builtin_fn,
+                    args,
+                    stdin,
+                    out_sink,
+                    redirect_out,
+                    capture_out,
+                    err_sink,
+                    cmd_shenv.cwd,
+                    merge_err=merge_err,
+                )
+            )
+            proc_not_counts.append(not_count)
+            proc_not_fail_if_crash.append(False)
+            if out_sink is not None:
+                out_sink.seek(0)
+                default_stdin = out_sink
+            else:
+                default_stdin = subprocess.PIPE
+            continue
         # If stderr wants to come from stdout, but stdout isn't a pipe, then put
         # stderr on a pipe and treat it as stdout.
         if stderr == subprocess.STDOUT and stdout != subprocess.PIPE:

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[boomanaiden154]

with the spawned-script path kept only as a fallback for cases an in-process function call cannot honor: 'env VAR=... cat/diff' (needs a per-command environment/locale) and 'not --crash cat/diff' (needs to die by signal). Output is byte-identical to the spawn path.

If I'm understanding correctly, this is specifically for passing environment variables to cat and diff/catching signals from them? If that's the case, then I would prefer to just not support those. It doesn't look like they read any environment variables, and if we probably shouldn't be writing tests that expect CPython to crash. Although cleanup can happen in a future PR.

Otherwise largely looks like a good approach to me.

[ilovepi]

@prasoon054 have you looked at the windows failure? it seems related to this change.

 UNRESOLVED: Clang :: Headers/opencl-c-header.cl (202 of 25122)
  ******************** TEST 'Clang :: Headers/opencl-c-header.cl' FAILED ********************
  Exception during script execution:
  Traceback (most recent call last):
    File "C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\builtin_commands\diff.py", line 73, in compareTwoFiles
      return compareTwoTextFiles(
             ^^^^^^^^^^^^^^^^^^^^
    File "C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\builtin_commands\diff.py", line 109, in compareTwoTextFiles
      line = line_bin.decode(encoding=encoding)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "C:\Python312\Lib\encodings\cp1252.py", line 15, in decode
      return codecs.charmap_decode(input,errors,decoding_table)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  UnicodeDecodeError: 'charmap' codec can't decode byte 0x90 in position 44: character maps to <undefined>
  decoding with 'cp1252' codec failed
  
  During handling of the above exception, another exception occurred:
  
  Traceback (most recent call last):
    File "C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\builtin_commands\diff.py", line 78, in compareTwoFiles
      return compareTwoTextFiles(flags, filepaths, filelines, "utf-8", stdout)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\builtin_commands\diff.py", line 109, in compareTwoTextFiles
      line = line_bin.decode(encoding=encoding)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa7 in position 8: invalid start byte
  

[prasoon054]

@prasoon054 have you looked at the windows failure? it seems related to this change.

Yes. I looked at it. It's a dormant bug from my previous change to diff.py. I'll add a fix in this PR.

[ilovepi]

This is better in some ways, and a regression in others. while I think a reasonable chunk has nicely simplified, I think some of the other logic is a bit messy, and the descriptions/documentation is unclear.

[ilovepi]

This very much feels like it was written by an LLM. I'd suggest auditing all these comments for that kind of thing. If an LLM was used, that is fine, but do follow the guidelines and call out its use in the PR description.

[prasoon054]

I used google antigravity for generating comments in some places. Seems like it didn't stick to my instructions about following the guidelines at https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings in most places. I'll fix all of those. Going forward I'll be more careful applying changes suggested by these models.

[boomanaiden154]

One nit, but I think this largely LGTM at this point.

[boomanaiden154]

Should this be a dataclass?

[prasoon054]

Yes, ideally. But @dataclass(slots=True) is only available in Python 3.10+.
We could use a plain @dataclass, and that would work with Python 3.8. But we'd lose the __slots__ memory benefits.

[ilovepi]

Largely I'm happy with the shape of this change. The doc strings seem to need a more thorough audit though. generally these see to encode a lot of irrelevant information, or things specific to the patch/transition. I left a few comments inline, but looking again, this seems like a more pervasive issue.