mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-24 06:26:11 +00:00
perf(hooks): eliminate multi-second foreground stalls before the detached launch
Three foreground costs ran on every commit before the detached rebuild launcher even started: 1. Interpreter probes imported the full package. Each probe executed 'import graphify' wholesale — measured 13s per probe cold on a Windows 11 dev box with AV-scanned site-packages — and up to four probes could run synchronously. Probes now use importlib.util.find_spec, which locates the package without executing it (interpreter startup cost only). A broken install under the selected interpreter still fails loudly in the rebuild log, as before. 2. The shebang probe read a binary. Git for Windows' command -v can return the launcher path WITHOUT its .exe suffix, so the '*.exe)' guard missed and head -1 read a PE binary: the shell warned 'ignored null byte in input' on every commit and the garbage always fell through to the slow python3/python fallbacks. The Windows pip layout is now resolved directly (Scripts/graphify -> sibling ../python.exe, or ./python.exe for venvs), and the remaining POSIX shebang read strips NULs first. 3. GIT_DIR was re-derived. git exports GIT_DIR to hooks; the unconditional rev-parse added ~1.3s more on machines where every git exec is scanned. Now reused from the environment with rev-parse as the manual-run fallback. Measured on the affected machine: hook foreground drops from 26s+ (cold) to ~1.4s, warnings gone. Behavior is unchanged on healthy POSIX setups — probe order and fallback semantics are preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Fable 5
parent
44c0a5e33c
commit
1256d65214
+38
-9
@@ -21,9 +21,18 @@ _PYTHON_DETECT = """\
|
||||
# Detect the correct Python interpreter (handles uv tool, pipx, venv, system installs).
|
||||
# _PINNED was recorded at hook-install time; tried first so the hook works even
|
||||
# when the graphify launcher is not on PATH (common in GUI clients and CI).
|
||||
#
|
||||
# Probes check availability with importlib.util.find_spec instead of importing
|
||||
# the package: a probe that imports graphify wholesale executes the full package
|
||||
# import (10s+ cold on machines with AV-scanned or large site-packages) and used
|
||||
# to run up to FOUR times synchronously, stalling every commit before the
|
||||
# detached launch even started. find_spec locates the package without executing
|
||||
# it, so each probe costs interpreter startup only. The detached rebuild still
|
||||
# fails loudly in the log if the package is broken under that interpreter.
|
||||
_GFY_PROBE="import importlib.util, sys; sys.exit(0 if importlib.util.find_spec('graphify') else 1)"
|
||||
GRAPHIFY_PYTHON=""
|
||||
_PINNED='__PINNED_PYTHON__'
|
||||
if [ -n "$_PINNED" ] && [ -x "$_PINNED" ] && "$_PINNED" -c "import graphify" 2>/dev/null; then
|
||||
if [ -n "$_PINNED" ] && [ -x "$_PINNED" ] && "$_PINNED" -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON="$_PINNED"
|
||||
fi
|
||||
# Second probe: read graphify-out/.graphify_python (written by the skill and
|
||||
@@ -35,18 +44,34 @@ if [ -z "$GRAPHIFY_PYTHON" ]; then
|
||||
case "$_FROM_FILE" in
|
||||
*[!a-zA-Z0-9/_.@:\\-]*) _FROM_FILE="" ;; # allowlist (covers Windows paths)
|
||||
esac
|
||||
if [ -n "$_FROM_FILE" ] && [ -x "$_FROM_FILE" ] && "$_FROM_FILE" -c "import graphify" 2>/dev/null; then
|
||||
if [ -n "$_FROM_FILE" ] && [ -x "$_FROM_FILE" ] && "$_FROM_FILE" -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON="$_FROM_FILE"
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
# Third probe: resolve via the graphify launcher on PATH (shebang probe).
|
||||
# Third probe: resolve via the graphify launcher on PATH.
|
||||
if [ -z "$GRAPHIFY_PYTHON" ]; then
|
||||
GRAPHIFY_BIN=$(command -v graphify 2>/dev/null)
|
||||
if [ -n "$GRAPHIFY_BIN" ]; then
|
||||
# Windows pip layout: Scripts/graphify(.exe) sits beside ..\\python.exe
|
||||
# (or .\\python.exe inside a venv's Scripts dir). NOTE: command -v may
|
||||
# return the launcher path WITHOUT the .exe suffix, so this cannot key
|
||||
# on the extension.
|
||||
_GFY_BINDIR=$(dirname "$GRAPHIFY_BIN")
|
||||
if [ -x "$_GFY_BINDIR/../python.exe" ] && "$_GFY_BINDIR/../python.exe" -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON="$_GFY_BINDIR/../python.exe"
|
||||
elif [ -x "$_GFY_BINDIR/python.exe" ] && "$_GFY_BINDIR/python.exe" -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON="$_GFY_BINDIR/python.exe"
|
||||
fi
|
||||
fi
|
||||
if [ -z "$GRAPHIFY_PYTHON" ] && [ -n "$GRAPHIFY_BIN" ]; then
|
||||
# POSIX launcher: parse the shebang. head -c + tr strip NUL bytes first —
|
||||
# when the launcher is a Windows binary reached without its .exe suffix,
|
||||
# a raw `head -1` reads binary into the command substitution and the
|
||||
# shell warns about ignored null bytes on every commit.
|
||||
case "$GRAPHIFY_BIN" in
|
||||
*.exe) _SHEBANG="" ;;
|
||||
*) _SHEBANG=$(head -1 "$GRAPHIFY_BIN" | sed 's/^#![[:space:]]*//') ;;
|
||||
*) _SHEBANG=$(head -c 256 "$GRAPHIFY_BIN" 2>/dev/null | tr -d '\\000' | head -n 1 | sed 's/^#![[:space:]]*//') ;;
|
||||
esac
|
||||
case "$_SHEBANG" in
|
||||
*/env\\ *) GRAPHIFY_PYTHON="${_SHEBANG#*/env }" ;;
|
||||
@@ -57,16 +82,16 @@ if [ -z "$GRAPHIFY_PYTHON" ]; then
|
||||
case "$GRAPHIFY_PYTHON" in
|
||||
*[!a-zA-Z0-9/_.@-]*) GRAPHIFY_PYTHON="" ;;
|
||||
esac
|
||||
if [ -n "$GRAPHIFY_PYTHON" ] && ! "$GRAPHIFY_PYTHON" -c "import graphify" 2>/dev/null; then
|
||||
if [ -n "$GRAPHIFY_PYTHON" ] && ! "$GRAPHIFY_PYTHON" -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON=""
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
# Last resort: try python3 / python (works for system/venv installs on PATH).
|
||||
if [ -z "$GRAPHIFY_PYTHON" ]; then
|
||||
if command -v python3 >/dev/null 2>&1 && python3 -c "import graphify" 2>/dev/null; then
|
||||
if command -v python3 >/dev/null 2>&1 && python3 -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON="python3"
|
||||
elif command -v python >/dev/null 2>&1 && python -c "import graphify" 2>/dev/null; then
|
||||
elif command -v python >/dev/null 2>&1 && python -c "$_GFY_PROBE" 2>/dev/null; then
|
||||
GRAPHIFY_PYTHON="python"
|
||||
else
|
||||
echo "[graphify hook] could not locate a Python with graphify installed. Add the graphify bin dir to PATH or re-run 'graphify hook install' from the env where graphify lives." >&2
|
||||
@@ -233,7 +258,9 @@ if [ -n "${WINDIR:-}" ] || [ -n "${MSYSTEM:-}" ]; then
|
||||
fi
|
||||
|
||||
# Skip during rebase/merge/cherry-pick to avoid blocking --continue with unstaged changes
|
||||
GIT_DIR=$(git rev-parse --git-dir 2>/dev/null)
|
||||
# git exports GIT_DIR to hooks; the rev-parse fallback only runs when invoked by
|
||||
# hand (each git exec costs 1s+ on AV-scanned Windows machines).
|
||||
GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)}
|
||||
[ -d "$GIT_DIR/rebase-merge" ] && exit 0
|
||||
[ -d "$GIT_DIR/rebase-apply" ] && exit 0
|
||||
[ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0
|
||||
@@ -299,7 +326,9 @@ if [ ! -d "graphify-out" ]; then
|
||||
fi
|
||||
|
||||
# Skip during rebase/merge/cherry-pick
|
||||
GIT_DIR=$(git rev-parse --git-dir 2>/dev/null)
|
||||
# git exports GIT_DIR to hooks; the rev-parse fallback only runs when invoked by
|
||||
# hand (each git exec costs 1s+ on AV-scanned Windows machines).
|
||||
GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)}
|
||||
[ -d "$GIT_DIR/rebase-merge" ] && exit 0
|
||||
[ -d "$GIT_DIR/rebase-apply" ] && exit 0
|
||||
[ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0
|
||||
|
||||
+44
-1
@@ -268,7 +268,7 @@ def _launcher_payload(script: str) -> str:
|
||||
|
||||
The launcher is the only `-c` invocation whose body begins with
|
||||
`import os, subprocess, sys` (the interpreter-detection probes in
|
||||
_PYTHON_DETECT use `-c "import graphify"`)."""
|
||||
_PYTHON_DETECT use `-c "$_GFY_PROBE"`)."""
|
||||
m = re.search(r'-c "(import os, subprocess, sys.*?)"\n', script, re.DOTALL)
|
||||
assert m, "launcher payload not found"
|
||||
return m.group(1)
|
||||
@@ -387,3 +387,46 @@ def test_default_hooks_dir_unaffected(tmp_path):
|
||||
repo = _make_git_repo(tmp_path)
|
||||
install(repo)
|
||||
assert (repo / ".git" / "hooks" / "post-commit").exists()
|
||||
|
||||
|
||||
# ── foreground hook cost: probes must be cheap and quiet ─────────────────────
|
||||
|
||||
def test_probes_use_find_spec_not_full_import():
|
||||
"""`python -c "import graphify"` executes the FULL package import — 10s+ on a
|
||||
cold cache or AV-scanned site-packages — and could run up to four times
|
||||
synchronously before the detached launch even started, so every commit
|
||||
stalled for tens of seconds. Probes must locate the package with
|
||||
importlib.util.find_spec (no execution); the detached rebuild still reports
|
||||
a broken install loudly in its log."""
|
||||
from graphify.hooks import _PYTHON_DETECT
|
||||
assert '-c "import graphify"' not in _PYTHON_DETECT, (
|
||||
"interpreter probe still imports the full package in the hook foreground"
|
||||
)
|
||||
assert "find_spec" in _PYTHON_DETECT
|
||||
|
||||
|
||||
def test_shebang_read_is_null_byte_safe():
|
||||
"""On Windows, `command -v graphify` can return the launcher path WITHOUT its
|
||||
.exe suffix, so the `*.exe)` guard misses and the shebang probe reads a
|
||||
BINARY: the shell then warns 'ignored null byte in input' on every commit and
|
||||
the extracted garbage always falls through to the slow fallbacks. The read
|
||||
must strip NULs before the command substitution sees them."""
|
||||
from graphify.hooks import _PYTHON_DETECT
|
||||
assert "tr -d '\\000'" in _PYTHON_DETECT, "shebang read is not NUL-safe"
|
||||
|
||||
|
||||
def test_probe_prefers_sibling_python_exe_on_windows_layouts():
|
||||
"""pip on Windows puts Scripts/graphify(.exe) beside ..\\python.exe (or
|
||||
.\\python.exe in a venv). Resolving that directly beats shebang-parsing a
|
||||
binary launcher — and works whether or not command -v kept the suffix."""
|
||||
from graphify.hooks import _PYTHON_DETECT
|
||||
assert "/../python.exe" in _PYTHON_DETECT
|
||||
assert "/python.exe" in _PYTHON_DETECT
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name,script", _HOOK_SCRIPTS)
|
||||
def test_hooks_reuse_git_dir_from_env(name, script):
|
||||
"""git exports GIT_DIR to hooks, so the rev-parse fallback should only run
|
||||
when the script is invoked by hand — each extra git exec costs 1s+ on
|
||||
AV-scanned Windows machines and lands in the commit's foreground."""
|
||||
assert "GIT_DIR=${GIT_DIR:-" in script, f"{name} always re-runs git rev-parse"
|
||||
|
||||
Reference in New Issue
Block a user