From 05f8ec7b2d14b86ff70c37694b6301690f500e55 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Thu, 13 Aug 2026 06:56:27 +0200 Subject: [PATCH] perf(coverage): scan function declarations in one awk pass extract_functions walked each file in Bash and counted braces with two pattern substitutions per line: 17.5ms per file against a 3.5ms fork floor, paid once per file per renderer. The same state machine in awk is 2238ms to 399ms over 128 files, and report_lcov 5052ms to 2638ms. The rules are unchanged, quirks included, and the output was diffed against the Bash implementation over all 463 shell files in the repo. The characterization tests pin the spans that carry the risk: nested braces, single-line bodies, every declaration spelling, an unclosed body at EOF and the shapes that are not declarations. Closes #1084 --- CHANGELOG.md | 1 + src/coverage/functions.sh | 182 ++++++++++++++-------------- tests/unit/coverage/helpers_test.sh | 123 +++++++++++++++++++ 3 files changed, 213 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c7da29b..3680f491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - Performance: hit data is grouped once per run by one awk pass, instead of `grep | cut | sort | uniq -c` per file — 1294ms to 203ms at 121 files and 10,890 records (#1057) - Performance: the DEBUG trap rejects a line from an untracked file before calling the recorder — a run matching no coverage path went from 2609ms to 497ms, against a 480ms no-coverage baseline (#1060) - Performance: the LCOV emitter classifies and writes each file in one awk pass instead of a Bash loop per line — 8520ms to 6632ms for 40 files of 752 lines. The awk rules are diffed against the Bash reference line by line over every shell file in the repo (#1059) +- Performance: function declarations are scanned in one awk pass instead of a Bash loop counting braces with pattern substitution — 2238ms to 399ms for 128 files, and a `--coverage` run over `src` from 9.23s to 6.81s (#1084) ### Fixed - Coverage reports every file under `--coverage-paths`, not only the ones a test executed: an untouched file shows as `0/N (0%)` and `--coverage-min` gates on that denominator. This repo reported 11 of its own 121 files. **Percentages drop, because the old ones were measured over the files that ran** (#1053) diff --git a/src/coverage/functions.sh b/src/coverage/functions.sh index e064192f..7a415ee2 100644 --- a/src/coverage/functions.sh +++ b/src/coverage/functions.sh @@ -2,105 +2,101 @@ # Locating function definitions and their line spans, for the reports. -# Extract function definitions from a bash file -# Output format: function_name:start_line:end_line (one per function) -function bashunit::coverage::extract_functions() { - local file="$1" - - local lineno=0 - local in_function=0 - local brace_count=0 - local current_fn="" - local fn_start=0 - local line - - while IFS= read -r line || [ -n "$line" ]; do - ((++lineno)) - - # Check for function definition patterns +# The declaration scanner, as awk source. +# +# It was a Bash `while read` loop with two `${line//[^\{]/}` substitutions per +# line to count braces. Bash 3.2 pattern substitution over a whole file is the +# single most expensive thing in the report phase: 17.5 ms per file against +# awk's 3.1 ms, and the report calls this once per file per renderer. One pass +# in awk instead (#1084). +# +# The rules are unchanged, quirks included -- notably that braces are counted +# without regard for strings or comments, so `echo "{"` inside a body extends +# the span. Changing that is a numbers change, not a perf change. +# +# It lives in a shell string rather than a .awk file because the build flattens +# *.sh into one artifact (ADR-011); a separate file would not ship. +# shellcheck disable=SC2016 # the $0 in here is awk's, not the shell's +_BASHUNIT_COVERAGE_AWK_FUNCTIONS=' +{ + line = $0 + if (in_function == 0) { # Pattern 1: function name() { or function name { # Pattern 2: name() { or name () { - if [ "$in_function" -eq 0 ]; then - local fn_name="" - - # Extract function name using pure Bash string operations (avoids sed subshell) - local stripped="${line#"${line%%[![:space:]]*}"}" - - # Strip "function " prefix if present - case "$stripped" in - function[\ \ ]*) - stripped="${stripped#function}" - stripped="${stripped#"${stripped%%[![:space:]]*}"}" - ;; - esac + stripped = line + sub(/^[ \t]+/, "", stripped) + if (stripped ~ /^function[ \t]/) { + sub(/^function/, "", stripped) + sub(/^[ \t]+/, "", stripped) + } - # Extract first word as candidate function name - fn_name="${stripped%%[[:space:]\(\{]*}" + # The candidate name is the first word, ending at whitespace, `(` or `{`. + name = stripped + sub(/[ \t({].*$/, "", name) - # Validate: must BE an identifier, and rest must have () or { - if [ -n "$fn_name" ]; then - case "$fn_name" in - # A candidate holding anything outside the identifier alphabet is not a - # function name. Cutting at the first `{` means `VAR="x${Y}"` yields - # `VAR="x$`, whose trailing `{Y}"` then looks like a body opener — every - # such assignment became a phantom FN record, and one containing the - # record separator `|` shifted the fields and crashed report_lcov's - # arithmetic (#936). Checking only the first character let all of that - # through. - *[!a-zA-Z0-9_:]*) fn_name="" ;; - [a-zA-Z_]*) - local after_name="${stripped#"$fn_name"}" - after_name="${after_name#"${after_name%%[![:space:]]*}"}" - case "$after_name" in - '()'* | '{'*) ;; - *) fn_name="" ;; - esac - ;; - *) fn_name="" ;; - esac - fi + if (name != "") { + ok = 1 + # A candidate holding anything outside the identifier alphabet is not a + # function name. Cutting at the first `{` means `VAR="x${Y}"` yields + # `VAR="x$`, whose trailing `{Y}"` then looks like a body opener -- every + # such assignment became a phantom FN record, and one containing the + # record separator `|` shifted the fields and crashed the arithmetic in + # report_lcov (#936). Checking only the first character let all of that + # through. + if (name ~ /[^a-zA-Z0-9_:]/) { + ok = 0 + } else if (name !~ /^[a-zA-Z_]/) { + ok = 0 + } else { + # A declaration continues with `()` or `{`; a call does not. + after = substr(stripped, length(name) + 1) + sub(/^[ \t]+/, "", after) + if (substr(after, 1, 2) != "()" && substr(after, 1, 1) != "{") { ok = 0 } + } - if [ -n "$fn_name" ]; then - in_function=1 - current_fn="$fn_name" - fn_start=$lineno - brace_count=0 + if (ok) { + in_function = 1 + current_fn = name + fn_start = NR + tmp = line; nopen = gsub(/\{/, "{", tmp) + tmp = line; nclose = gsub(/\}/, "}", tmp) + brace_count = nopen - nclose + # Single-line function: braces balance on the same line, both present. + if (brace_count == 0 && nopen > 0 && nclose > 0) { + print current_fn "|" fn_start "|" NR + in_function = 0 + current_fn = "" + } + next + } + } + } - # Count opening braces on this line - local open_braces="${line//[^\{]/}" - local close_braces="${line//[^\}]/}" - local open_count=${#open_braces} - local close_count=${#close_braces} - brace_count=$((brace_count + open_count - close_count)) - - # Single-line function: braces balance on same line and both present - if [ "$brace_count" -eq 0 ] && [ "$open_count" -gt 0 ] && [ "$close_count" -gt 0 ]; then - echo "${current_fn}|${fn_start}|${lineno}" - in_function=0 - current_fn="" - fi - continue - fi - fi - - # Track braces inside function - if [ "$in_function" -eq 1 ]; then - local open_braces="${line//[^\{]/}" - local close_braces="${line//[^\}]/}" - brace_count=$((brace_count + ${#open_braces} - ${#close_braces})) + if (in_function == 1) { + tmp = line; nopen = gsub(/\{/, "{", tmp) + tmp = line; nclose = gsub(/\}/, "}", tmp) + brace_count = brace_count + nopen - nclose + if (brace_count <= 0) { + print current_fn "|" fn_start "|" NR + in_function = 0 + current_fn = "" + brace_count = 0 + } + } +} - # Function ended - if [ "$brace_count" -le 0 ]; then - echo "${current_fn}|${fn_start}|${lineno}" - in_function=0 - current_fn="" - brace_count=0 - fi - fi - done <"$file" +# An unclosed function (should not happen in valid code) still gets a record, +# ending at the last line, so a truncated file cannot drop one silently. +END { + if (in_function == 1 && current_fn != "") { print current_fn "|" fn_start "|" NR } +} +' - # Handle unclosed function (shouldn't happen in valid code) - if [ "$in_function" -eq 1 ] && [ -n "$current_fn" ]; then - echo "${current_fn}|${fn_start}|${lineno}" - fi +## +# Extract function definitions from a bash file. +# Output format: function_name|start_line|end_line (one per function) +# Arguments: $1 - source file +## +function bashunit::coverage::extract_functions() { + env LC_ALL=C "$AWK" "$_BASHUNIT_COVERAGE_AWK_FUNCTIONS" "$1" } diff --git a/tests/unit/coverage/helpers_test.sh b/tests/unit/coverage/helpers_test.sh index e103ddaa..b01e8e1b 100644 --- a/tests/unit/coverage/helpers_test.sh +++ b/tests/unit/coverage/helpers_test.sh @@ -265,6 +265,129 @@ EOF rm -f "$temp_file" } +# The span of a function is decided by brace balance, so a nested block inside +# the body must not close it early. +function test_coverage_extract_functions_spans_nested_braces() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +function outer() { + local map="${x:-fallback}" + if [ -n "$map" ]; then + echo "deep" + fi +} +function after() { + echo "after" +} +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "outer|2|7 +after|8|10" "$result" + + rm -f "$temp_file" +} + +# A body that opens and closes on the declaration line is one record whose start +# and end are the same line. +function test_coverage_extract_functions_reports_a_single_line_function() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +function one_liner() { echo "hi"; } +bare_liner() { echo "there"; } +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "one_liner|2|2 +bare_liner|3|3" "$result" + + rm -f "$temp_file" +} + +# The three declaration spellings bash accepts, plus an indented one: `name()`, +# `function name()` and `function name` with no parentheses at all. +function test_coverage_extract_functions_accepts_every_declaration_spelling() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +bare() { + echo "a" +} +function keyword() { + echo "b" +} +function no_parens { + echo "c" +} + indented() { + echo "d" + } +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "bare|2|4 +keyword|5|7 +no_parens|8|10 +indented|11|13" "$result" + + rm -f "$temp_file" +} + +# An unbalanced file still reports the function, ending at the last line, so a +# truncated or generated file cannot drop a record silently. +function test_coverage_extract_functions_closes_an_unclosed_function_at_eof() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +function unclosed() { + echo "no closing brace" +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "unclosed|2|3" "$result" + + rm -f "$temp_file" +} + +# A name is only a name inside [a-zA-Z0-9_:] and starting with a letter or `_`, +# and the declaration must continue with `()` or `{` -- a call is not a +# definition. +function test_coverage_extract_functions_rejects_non_declarations() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +9lives() { + echo "starts with a digit" +} +call_me arg +real_fn() { + echo "yes" +} +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "real_fn|6|8" "$result" + + rm -f "$temp_file" +} + # === Line hits tests === function test_coverage_get_all_line_hits_counts_per_line() {