From 19e019a57b4c669fd24db5a1bc87b1ac236ae00c Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Wed, 22 Mar 2023 03:17:01 +0100 Subject: [PATCH] add PR comments that show build statistics, speed & RAM usage (#735) --- .github/workflows/pr_info_intro.yml | 24 +++++ .github/workflows/pr_info_post.yml | 49 +++++++++++ .github/workflows/pr_info_untrusted.yml | 67 ++++++++++++++ ci/request_time_stats.d | 109 +++++++++++++++++++++++ ci/summary_comment.sh | 60 +++++++++++++ ci/summary_comment_diff.sh | 111 ++++++++++++++++++++++++ src/dcd/server/main.d | 9 ++ tests/run_tests.sh | 60 ++++++++++++- 8 files changed, 485 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/pr_info_intro.yml create mode 100644 .github/workflows/pr_info_post.yml create mode 100644 .github/workflows/pr_info_untrusted.yml create mode 100644 ci/request_time_stats.d create mode 100755 ci/summary_comment.sh create mode 100755 ci/summary_comment_diff.sh diff --git a/.github/workflows/pr_info_intro.yml b/.github/workflows/pr_info_intro.yml new file mode 100644 index 0000000..c9ef855 --- /dev/null +++ b/.github/workflows/pr_info_intro.yml @@ -0,0 +1,24 @@ +name: PR Info (pre-comment) + +on: + # NOTE: high probability for security vulnerabilities if doing ANYTHING in + # this file other than commenting something! + pull_request_target: + branches: + - master + +jobs: + intro_comment: + name: Make intro comment + runs-on: ubuntu-20.04 + steps: + - name: 'Prepare sticky comment' + # commit of v2.5.0 + # same one used again at the bottom of the file to update the comment. + uses: marocchino/sticky-pull-request-comment@3d60a5b2dae89d44e0c6ddc69dd7536aec2071cd + with: + message: | + Thanks for your Pull Request and making D better! + + This comment will automatically be updated to summarize some statistics in a few minutes. + only_create: true diff --git a/.github/workflows/pr_info_post.yml b/.github/workflows/pr_info_post.yml new file mode 100644 index 0000000..c3a0914 --- /dev/null +++ b/.github/workflows/pr_info_post.yml @@ -0,0 +1,49 @@ +name: PR Info (comment) + +on: + workflow_run: + workflows: ["PR Info"] + types: + - completed + +jobs: + comment: + name: PR Info + runs-on: ubuntu-20.04 + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + steps: + # from https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ + - name: 'Download artifact' + uses: actions/github-script@v3.1.0 + with: + script: | + var artifacts = await github.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: ${{github.event.workflow_run.id }}, + }); + var matchArtifact = artifacts.data.artifacts.filter((artifact) => { + return artifact.name == "pr" + })[0]; + var download = await github.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: matchArtifact.id, + archive_format: 'zip', + }); + var fs = require('fs'); + fs.writeFileSync('${{github.workspace}}/pr.zip', Buffer.from(download.data)); + - run: unzip pr.zip + + - name: Set variable + run: | + PR_ID=$(cat ./NR) + echo "PR_ID=$PR_ID" >> $GITHUB_ENV + + - name: Update GitHub comment + uses: marocchino/sticky-pull-request-comment@3d60a5b2dae89d44e0c6ddc69dd7536aec2071cd + with: + path: ./comment.txt + number: ${{ env.PR_ID }} diff --git a/.github/workflows/pr_info_untrusted.yml b/.github/workflows/pr_info_untrusted.yml new file mode 100644 index 0000000..f72c0de --- /dev/null +++ b/.github/workflows/pr_info_untrusted.yml @@ -0,0 +1,67 @@ +name: PR Info + +# This workflow builds the whole project once and: +# - comments build deprecations/warnings (highlighting new ones since last tested PR) + +on: + pull_request: + branches: + - master + +jobs: + pr_info: + name: PR Info + runs-on: ubuntu-latest + steps: + # we first create a comment thanking the user in pr_info_intro.yml + # (separate step due to needing GITHUB_TOKEN access) + + # Compiler to test with + - name: Prepare compiler + uses: dlang-community/setup-dlang@v1 + with: + compiler: dmd-latest + + - name: Prepare compiler + uses: dlang-community/setup-dlang@v1 + with: + compiler: ldc-latest + + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Checkout old stuff, with new comment script + run: | + git checkout ${{ github.base_ref }} + git checkout ${{ github.sha }} -- ./ci/summary_comment.sh ./ci/summary_comment_diff.sh + + # first dump old info + + - name: Check pre-PR status + run: ./ci/summary_comment.sh | tee ../OLD_OUTPUT.txt + + - name: Checkout PR target + run: | + git checkout ${{ github.sha }} + git clean -fd + git reset --hard + + - name: Evaluate PR + run: ./ci/summary_comment.sh | tee ../NEW_OUTPUT.txt + + - name: Generate comment + run: ./ci/summary_comment_diff.sh ../OLD_OUTPUT.txt ../NEW_OUTPUT.txt | tee comment.txt + + - name: Prepare comment for upload + run: | + mkdir -p ./pr + mv comment.txt pr + echo ${{ github.event.number }} > ./pr/NR + + - name: upload comment to high-trust action making the comment + uses: actions/upload-artifact@v2 + with: + name: pr + path: pr/ diff --git a/ci/request_time_stats.d b/ci/request_time_stats.d new file mode 100644 index 0000000..d146e87 --- /dev/null +++ b/ci/request_time_stats.d @@ -0,0 +1,109 @@ +import core.time; +import std.algorithm; +import std.conv; +import std.format; +import std.stdio; +import std.string; + +void main() +{ + long[] shortRequests, longRequests; + + foreach (line; stdin.byLine) + { + auto index = line.countUntil("Request processed in "); + if (index == -1) + { + stderr.writeln("Warning: skipping unknown line for stats: ", line); + continue; + } + index += "Request processed in ".length; + auto dur = line[index .. $].parseDuration; + if (dur != Duration.init) + { + if (dur >= 10.msecs) + longRequests ~= dur.total!"hnsecs"; + else + shortRequests ~= dur.total!"hnsecs"; + } + } + + if (shortRequests.length > 0) + { + writeln("STAT:short requests: (", shortRequests.length, "x)"); + summarize(shortRequests); + } + writeln("STAT:"); + if (longRequests.length > 0) + { + writeln("STAT:long requests over 10ms: (", longRequests.length, "x)"); + summarize(longRequests); + } +} + +void summarize(long[] hnsecs) +{ + hnsecs.sort!"a&1 || echo "DCD BUILD FAILED" +dub build --build=release --config=server --compiler=ldc2 --force 2>&1 || echo "DCD BUILD FAILED" +end=`date +%s` +build_time=$( echo "$end - $start" | bc -l ) + +strip bin/dcd-server +strip bin/dcd-client + +echo "STAT:statistics (-before, +after)" +echo "STAT:client size=$(wc -c bin/dcd-client)" +echo "STAT:server size=$(wc -c bin/dcd-server)" +echo "STAT:rough build time=${build_time}s" +echo "STAT:" + +cd tests +./run_tests.sh --time-server + +echo "STAT:DCD run_tests.sh $(grep -F 'Elapsed (wall clock) time' stderr.txt)" +echo "STAT:DCD run_tests.sh $(grep -F 'Maximum resident set size (kbytes)' stderr.txt)" + +echo "STAT:" +grep -E 'Request processed in .*' stderr.txt | rdmd ../ci/request_time_stats.d +echo "STAT:" + +# now rebuild server with -profile=gc +cd .. +rm -rf .dub bin/dcd-server +dub build --build=profile-gc --config=server --compiler=dmd 2>&1 || echo "DCD BUILD FAILED" + +cd tests +./run_tests.sh + +echo "STAT:top 5 GC sources in server:" +if [ ! -f "profilegc.log" ]; then + echo 'Missing profilegc.log file!' + echo 'Tail for stderr.txt:' + tail -n50 stderr.txt +fi +head -n6 profilegc.log | sed 's/^/STAT:/g' diff --git a/ci/summary_comment_diff.sh b/ci/summary_comment_diff.sh new file mode 100755 index 0000000..43707ef --- /dev/null +++ b/ci/summary_comment_diff.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash + +set -u + +EMPTY=1 + +ADDED=$(diff --new-line-format='%L' --old-line-format='' --unchanged-line-format='' "$1" "$2") +REMOVED=$(diff --new-line-format='' --old-line-format='%L' --unchanged-line-format='' "$1" "$2") +TOTAL=$(cat "$2") + +STATS_OLD=$(grep -E '^STAT:' "$1" | sed -E 's/^STAT://') +STATS_NEW=$(grep -E '^STAT:' "$2" | sed -E 's/^STAT://') + +STATS_DIFFED=$(diff --new-line-format='+%L' --old-line-format='-%L' --unchanged-line-format=' %L' <(echo "$STATS_OLD") <(echo "$STATS_NEW")) + +ADDED_DEPRECATIONS=$(grep -Pi '\b(deprecation|deprecated)\b' <<< "$ADDED") +REMOVED_DEPRECATIONS=$(grep -Pi '\b(deprecation|deprecated)\b' <<< "$REMOVED") +ADDED_WARNINGS=$(grep -Pi '\b(warn|warning)\b' <<< "$ADDED") +REMOVED_WARNINGS=$(grep -Pi '\b(warn|warning)\b' <<< "$REMOVED") + +DEPRECATION_COUNT=$(grep -Pi '\b(deprecation|deprecated)\b' <<< "$TOTAL" | wc -l) +WARNING_COUNT=$(grep -Pi '\b(warn|warning)\b' <<< "$TOTAL" | wc -l) + +if [ -z "$ADDED_DEPRECATIONS" ]; then + # no new deprecations + true +else + echo "⚠️ This PR introduces new deprecations:" + echo + echo '```' + echo "$ADDED_DEPRECATIONS" + echo '```' + echo + EMPTY=0 +fi + +if [ -z "$ADDED_WARNINGS" ]; then + # no new deprecations + true +else + echo "⚠️ This PR introduces new warnings:" + echo + echo '```' + echo "$ADDED_WARNINGS" + echo '```' + echo + EMPTY=0 +fi + +if grep "DCD BUILD FAILED" <<< "$TOTAL"; then + echo '❌ Basic `dub build` failed! Please check your changes again.' + echo +else + if [ -z "$REMOVED_DEPRECATIONS" ]; then + # no removed deprecations + true + else + echo "✅ This PR fixes following deprecations:" + echo + echo '```' + echo "$REMOVED_DEPRECATIONS" + echo '```' + echo + EMPTY=0 + fi + + if [ -z "$REMOVED_WARNINGS" ]; then + # no removed warnings + true + else + echo "✅ This PR fixes following warnings:" + echo + echo '```' + echo "$REMOVED_WARNINGS" + echo '```' + echo + EMPTY=0 + fi + + if [ $EMPTY == 1 ]; then + echo "✅ PR OK, no changes in deprecations or warnings" + echo + fi + + echo "Total deprecations: $DEPRECATION_COUNT" + echo + echo "Total warnings: $WARNING_COUNT" + echo +fi + +if [ -z "$STATS_DIFFED" ]; then + # no statistics? + true +else + echo "Build statistics:" + echo + echo '```diff' + echo "$STATS_DIFFED" + echo '```' + echo +fi + +echo '
' +echo +echo 'Full build output' +echo +echo '```' +echo "$TOTAL" +echo '```' +echo +echo '
' diff --git a/src/dcd/server/main.d b/src/dcd/server/main.d index e1ca9bd..5514330 100644 --- a/src/dcd/server/main.d +++ b/src/dcd/server/main.d @@ -45,6 +45,15 @@ import dcd.server.server; int main(string[] args) { + version (D_ProfileGC) + { + import core.runtime; + + // make sure profilegc.log is written to cwd and not to `/` + // (since we `chdir` to `/` later) + profilegc_setlogfilename(buildPath(getcwd, "profilegc.log")); + } + try { return runServer(args); diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 59cca6b..3bcf7fe 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -5,6 +5,32 @@ YELLOW="\033[33m" NORMAL="\033[0m" IMPORTS=$(pwd)/imports export IMPORTS +SOCKETMODES="unix tcp" +TIME_SERVER=0 + +# `--arguments` must come before test dirs! +while (( "$#" )); do + if [[ "$1" == "--tcp-only" ]]; then + # only test TCP sockets + SOCKETMODES="tcp" + elif [[ "$1" == "--unix-only" ]]; then + # only test unix domain sockets + SOCKETMODES="unix" + elif [[ "$1" == "--time-server" ]]; then + # --time-server runs dcd-server through /usr/bin/time, for statistics + # implies `--unix-only` (since we only want a single mode to time) + # socket mode can still be overriden with `--tcp-only` + TIME_SERVER=1 + SOCKETMODES="unix" + elif [[ "$1" =~ ^-- ]]; then + echo "Unrecognized test argument: $1" + exit 1 + else + break + fi + + shift +done if [ -z "${1:-}" ]; then @@ -18,22 +44,46 @@ pass_count=0 client="../bin/dcd-client" server="../bin/dcd-server" tcp="" +server_pid="" function startServer() { - "$server" "$tcp" --ignoreConfig -I $IMPORTS 2>stderr.txt > stdout.txt & - server_pid=$! + if [[ "$TIME_SERVER" == "1" ]]; then + /usr/bin/time -v "$server" "$tcp" --ignoreConfig -I $IMPORTS 2>stderr.txt > stdout.txt & + server_pid=$! + else + "$server" "$tcp" --ignoreConfig -I $IMPORTS 2>stderr.txt > stdout.txt & + server_pid=$! + fi sleep 1 } +function waitShutdown() +{ + if [[ -z "$server_pid" ]]; then + sleep 0.5 # not owned by us + else + ( sleep 15 ; echo 'Waiting for shutdown timed out'; kill $server_pid ) & + killerPid=$! + + wait $server_pid + status=$? + (kill -0 $killerPid && kill $killerPid) || true + + server_pid="" + + return $status + fi +} + # Make sure that the server is shut down echo "Shutting down currently-running server..." "$client" --shutdown 2>/dev/null > /dev/null "$client" --shutdown --tcp 2>/dev/null > /dev/null -for socket in unix tcp; do +for socket in $SOCKETMODES; do # supported: unix tcp # allow some time for server to shutdown - sleep 0.5; + waitShutdown if [[ $socket == "tcp" ]]; then tcp="--tcp" @@ -88,6 +138,8 @@ for socket in unix tcp; do echo "Shutting down server..." "$client" --shutdown "$tcp" 2>/dev/null > /dev/null + waitShutdown + # Report if [[ $fail_count -eq 0 ]]; then echo -e "${GREEN}${pass_count} tests passed and ${fail_count} failed.${NORMAL}"