diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b84064f..f2d317bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - 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) +- Performance: every tracked file's line stats are computed by one awk invocation for the whole run instead of a Bash loop and three subshells per file — 2585ms to 153ms for 128 files, taking that `--coverage` run to 3.77s (#1088) ### 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/rules_awk.sh b/src/coverage/rules_awk.sh index 98e770ae..c94d9a80 100644 --- a/src/coverage/rules_awk.sh +++ b/src/coverage/rules_awk.sh @@ -92,16 +92,10 @@ function bu_is_executable(line, tmp, stripped, trimmed, first, rest, fn_rest, return 1 } -' -# The DA/LF/LH block of one file's LCOV record, in one pass. -# -# Reads the file's aggregated hit block first (#1057), then the source, and -# applies the same continuation propagation the Bash reader does: the DEBUG -# trap attributes a multi-line statement to its starting line, so the count -# carries forward across the backslash chain (#722). -# shellcheck disable=SC2016 -_BASHUNIT_COVERAGE_AWK_LCOV=' +# Whether a source line ends with a line continuation: an odd number of +# trailing backslashes, and not a comment. Lives here because both the LCOV +# emitter and the stats pass propagate hits along a continuation chain (#722). function bu_ends_with_continuation(line, lead, i, n) { lead = line sub(/^[ \t]+/, "", lead) @@ -112,7 +106,16 @@ function bu_ends_with_continuation(line, lead, i, n) { } return (n % 2) == 1 } +' +# The DA/LF/LH block of one file's LCOV record, in one pass. +# +# Reads the file's aggregated hit block first (#1057), then the source, and +# applies the same continuation propagation the Bash reader does: the DEBUG +# trap attributes a multi-line statement to its starting line, so the count +# carries forward across the backslash chain (#722). +# shellcheck disable=SC2016 +_BASHUNIT_COVERAGE_AWK_LCOV=' # The guard is FILENAME, not the usual `FNR == NR`: a run with no recorded hits # passes an EMPTY first file, and `FNR == NR` is then true for the first record # of the SECOND file, which would swallow the source line 1. @@ -149,6 +152,62 @@ END { } ' +# Executable and hit counts for MANY files, in one awk invocation. +# +# The report needs a count per tracked file, and computing it per file meant a +# Bash loop over every line of every file: 1956ms for 128 files, the last +# per-line Bash loop in the report phase. Reading the manifest and walking each +# pair with getline pays the cost of a fork once for the whole run (#1088). +# +# Input is a manifest of "\t" lines; output is +# "\t\t". The source path comes last so a path holding +# a tab still reads back whole. +# shellcheck disable=SC2016 +_BASHUNIT_COVERAGE_AWK_STATS=' +{ + hitsfile = $0 + sub(/\t.*$/, "", hitsfile) + src = $0 + sub(/^[^\t]*\t/, "", src) + + split("", hits) + if (hitsfile != "") { + while ((getline hline < hitsfile) > 0) { + split(hline, hp, " ") + hits[hp[1] + 0] = hp[2] + 0 + } + close(hitsfile) + } + + total = 0 + split("", sl) + while ((getline sline < src) > 0) { + total++ + sl[total] = sline + } + close(src) + + # The DEBUG trap attributes a multi-line statement to its starting line, so + # the count carries forward across the backslash chain (#722). + carry = 0 + for (ln = 1; ln <= total; ln++) { + h = (ln in hits) ? hits[ln] : 0 + if (carry > 0 && h < carry) { h = carry; hits[ln] = h } + if (h > 0 && bu_ends_with_continuation(sl[ln])) { carry = h } else { carry = 0 } + } + + executable = 0 + hit = 0 + for (ln = 1; ln <= total; ln++) { + if (!bu_is_executable(sl[ln])) { continue } + executable++ + if ((ln in hits) && hits[ln] > 0) { hit++ } + } + + print executable "\t" hit "\t" src +} +' + ## # The awk source of the shared classification rules. ## @@ -175,3 +234,14 @@ function bashunit::coverage::awk_lcov_lines() { "${_BASHUNIT_COVERAGE_AWK_RULES}${_BASHUNIT_COVERAGE_AWK_LCOV}" \ "$hits_file" "$file" } + +## +# Emits "\t\t" for every pair in the manifest, in one +# awk invocation. +# Arguments: $1 - manifest of "\t" lines +## +function bashunit::coverage::awk_file_stats() { + env LC_ALL=C "$AWK" \ + "${_BASHUNIT_COVERAGE_AWK_RULES}${_BASHUNIT_COVERAGE_AWK_STATS}" \ + "$1" +} diff --git a/src/coverage/stats.sh b/src/coverage/stats.sh index 4fc55a20..9d834538 100644 --- a/src/coverage/stats.sh +++ b/src/coverage/stats.sh @@ -47,12 +47,31 @@ function bashunit::coverage::_compute_file_stats() { local file="$1" local stats stats=$(bashunit::coverage::compute_file_coverage "$file") - _BASHUNIT_COVERAGE_FILE_STATS_EXEC_OUT="${stats%%:*}" - _BASHUNIT_COVERAGE_FILE_STATS_HIT_OUT="${stats##*:}" - _BASHUNIT_COVERAGE_FILE_STATS_PCT_OUT=$(bashunit::coverage::calculate_percentage \ - "$_BASHUNIT_COVERAGE_FILE_STATS_HIT_OUT" "$_BASHUNIT_COVERAGE_FILE_STATS_EXEC_OUT") - _BASHUNIT_COVERAGE_FILE_STATS_CLASS_OUT=$(bashunit::coverage::get_coverage_class \ - "$_BASHUNIT_COVERAGE_FILE_STATS_PCT_OUT") + bashunit::coverage::_derive_file_stats "${stats%%:*}" "${stats##*:}" +} + +# Fills the four slots from an executable/hit pair. Split out so the batch pass +# and the per-file path derive pct and class the same way, and so neither forks +# for them: percentage and class each used to cost a subshell per file, which +# at 128 tracked files was more than the arithmetic they wrapped (#1088). +function bashunit::coverage::_derive_file_stats() { + local executable="$1" hit="$2" + _BASHUNIT_COVERAGE_FILE_STATS_EXEC_OUT="$executable" + _BASHUNIT_COVERAGE_FILE_STATS_HIT_OUT="$hit" + + local pct=0 + if [ "$executable" -gt 0 ]; then + pct=$((hit * 100 / executable)) + fi + _BASHUNIT_COVERAGE_FILE_STATS_PCT_OUT="$pct" + + if [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}" ]; then + _BASHUNIT_COVERAGE_FILE_STATS_CLASS_OUT="high" + elif [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_LOW:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW}" ]; then + _BASHUNIT_COVERAGE_FILE_STATS_CLASS_OUT="medium" + else + _BASHUNIT_COVERAGE_FILE_STATS_CLASS_OUT="low" + fi } # Get file coverage stats as "executable:hit:pct:class" @@ -86,23 +105,70 @@ function bashunit::coverage::precompute_file_stats() { _BASHUNIT_COVERAGE_STATS_COUNT=0 bashunit::coverage::reset_lookup_namespace "_BASHUNIT_COVLOOKUP_STATS_" + if bashunit::coverage::_precompute_batch; then + return 0 + fi + local file while IFS= read -r file; do { [ -z "$file" ] || [ ! -f "$file" ]; } && continue bashunit::coverage::_compute_file_stats "$file" - - local idx="$_BASHUNIT_COVERAGE_STATS_COUNT" - _BASHUNIT_COVERAGE_STATS_FILES[idx]="$file" - _BASHUNIT_COVERAGE_STATS_EXEC[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_EXEC_OUT" - _BASHUNIT_COVERAGE_STATS_HIT[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_HIT_OUT" - _BASHUNIT_COVERAGE_STATS_PCT[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_PCT_OUT" - _BASHUNIT_COVERAGE_STATS_CLASS[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_CLASS_OUT" - _BASHUNIT_COVERAGE_STATS_COUNT=$((idx + 1)) - bashunit::coverage::lookup_put "_BASHUNIT_COVLOOKUP_STATS_" "$file" "$idx" + bashunit::coverage::_record_file_stats "$file" \ + "$_BASHUNIT_COVERAGE_FILE_STATS_EXEC_OUT" "$_BASHUNIT_COVERAGE_FILE_STATS_HIT_OUT" done < <(bashunit::coverage::get_tracked_files) } +# Appends one file to the stats cache. +function bashunit::coverage::_record_file_stats() { + local file="$1" + bashunit::coverage::_derive_file_stats "$2" "$3" + + local idx="$_BASHUNIT_COVERAGE_STATS_COUNT" + _BASHUNIT_COVERAGE_STATS_FILES[idx]="$file" + _BASHUNIT_COVERAGE_STATS_EXEC[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_EXEC_OUT" + _BASHUNIT_COVERAGE_STATS_HIT[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_HIT_OUT" + _BASHUNIT_COVERAGE_STATS_PCT[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_PCT_OUT" + _BASHUNIT_COVERAGE_STATS_CLASS[idx]="$_BASHUNIT_COVERAGE_FILE_STATS_CLASS_OUT" + _BASHUNIT_COVERAGE_STATS_COUNT=$((idx + 1)) + bashunit::coverage::lookup_put "_BASHUNIT_COVLOOKUP_STATS_" "$file" "$idx" +} + +# Fills the whole cache with one awk invocation, and reports whether it could. +# +# Returns 1 without touching the cache when there is nowhere to write the +# manifest or the pass produced nothing for a non-empty tracked list, so the +# caller falls back to the per-file path and a report is never silently empty. +function bashunit::coverage::_precompute_batch() { + local data_dir="${_BASHUNIT_COVERAGE_DATA_FILE%/*}" + { [ -n "${_BASHUNIT_COVERAGE_DATA_FILE:-}" ] && [ -d "$data_dir" ]; } || return 1 + + bashunit::coverage::ensure_hits_aggregated + + local manifest="$data_dir/stats-manifest" + local tracked=0 file + { + while IFS= read -r file; do + { [ -z "$file" ] || [ ! -f "$file" ]; } && continue + tracked=$((tracked + 1)) + bashunit::coverage::hits_file_for "$file" + printf '%s\t%s\n' "$_BASHUNIT_COVERAGE_HITS_FILE_OUT" "$file" + done < <(bashunit::coverage::get_tracked_files) + } >"$manifest" 2>/dev/null || return 1 + + if [ "$tracked" -eq 0 ]; then + return 0 + fi + + local executable hit + while IFS="$(printf '\t')" read -r executable hit file; do + [ -n "$file" ] || continue + bashunit::coverage::_record_file_stats "$file" "$executable" "$hit" + done < <(bashunit::coverage::awk_file_stats "$manifest" 2>/dev/null) + + [ "$_BASHUNIT_COVERAGE_STATS_COUNT" -gt 0 ] +} + # Look up cached stats for a file, returns "executable:hit:pct:class" function bashunit::coverage::get_cached_stats() { local file="$1" diff --git a/tests/unit/coverage/precompute_stats_test.sh b/tests/unit/coverage/precompute_stats_test.sh new file mode 100644 index 00000000..08d50b89 --- /dev/null +++ b/tests/unit/coverage/precompute_stats_test.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash + +# precompute_file_stats fills the cache every renderer reads, and get_file_stats +# computes one file on demand. They must agree exactly: the cache is what the +# report shows, and a file the cache missed falls through to the on-demand path +# mid-report. This pins the two together so the batch path cannot drift (#1088). + +function set_up() { + WORK="$(bashunit::temp_dir)/precompute" + mkdir -p "$WORK" + # The tracked list collapses a doubled slash and the hit blocks are named + # after the recorded path, so the fixture has to record the same spelling the + # tracked list holds -- `/tmp//bashunit` would key its blocks differently and + # read back as zero hits. + local slash="/" + WORK="${WORK//\/\//$slash}" + + # Plain: three executable lines, one comment, one blank. + printf 'function plain() {\n local a=1\n\n # note\n echo "$a"\n}\n' >"$WORK/plain.sh" + # A statement continued over two lines: the hit on the first must carry to + # the second, which is the rule the batch pass has to reproduce (#722). + printf 'function cont() {\n echo "one" \\\n "two"\n echo "three"\n}\n' >"$WORK/cont.sh" + # Never executed at all. + printf 'function cold() {\n echo "never"\n}\n' >"$WORK/cold.sh" + + # The paths have to be in place before init: it is what decides the tracked + # set the report is about. + # shellcheck disable=SC2034 # read by coverage::init and the seeding + BASHUNIT_COVERAGE_PATHS="$WORK" + # shellcheck disable=SC2034 # read by coverage::init and the seeding + BASHUNIT_COVERAGE_EXCLUDE="" + # shellcheck disable=SC2034 # read by coverage::init + BASHUNIT_COVERAGE="true" + bashunit::coverage::init + + { + echo "$WORK/plain.sh:2" + echo "$WORK/plain.sh:2" + echo "$WORK/plain.sh:5" + echo "$WORK/cont.sh:2" + } >>"$_BASHUNIT_COVERAGE_DATA_FILE" + bashunit::coverage::invalidate_hits_aggregation +} + +function test_the_batch_pass_matches_the_per_file_path_for_every_file() { + bashunit::coverage::precompute_file_stats + + local file + for file in "$WORK/plain.sh" "$WORK/cont.sh" "$WORK/cold.sh"; do + assert_same "$(bashunit::coverage::get_file_stats "$file")" \ + "$(bashunit::coverage::get_cached_stats "$file")" + done +} + +function test_the_batch_pass_carries_a_hit_across_a_line_continuation() { + bashunit::coverage::precompute_file_stats + + # `echo "one" \` runs and its continuation counts as run with it, so 2 of the + # 3 executable lines are hit -- the trailing `echo "three"` never ran. + assert_same "3:2:66:medium" "$(bashunit::coverage::get_cached_stats "$WORK/cont.sh")" +} + +function test_a_file_no_test_executed_counts_with_zero_hits() { + bashunit::coverage::precompute_file_stats + + assert_same "1:0:0:low" "$(bashunit::coverage::get_cached_stats "$WORK/cold.sh")" +} + +function test_the_total_percentage_covers_every_tracked_file() { + bashunit::coverage::precompute_file_stats + + # plain 2 executable / 2 hit, cont 3/2, cold 1/0 -> 4 of 6. + assert_same "66" "$(bashunit::coverage::get_percentage)" +}