Faster scanning: batched reads, readahead, threaded first scan

.github/actions/linux-setup/action.yml

@@ -1,21 +1,48 @@
 name: linux-setup
-description: Set up the Linux VM
+description: Install build deps and configure the tree
+
+inputs:
+  cc:
+    description: Compiler to build with
+    required: false
+    default: gcc
+  cflags:
+    description: Extra CFLAGS passed to configure
+    required: false
+    default: '-O2 -Wall'
+  ldflags:
+    description: Extra LDFLAGS passed to configure
+    required: false
+    default: ''
+  configure-args:
+    description: Extra arguments for ./configure
+    required: false
+    default: '--enable-gui'
 
 runs:
   using: 'composite'
   steps:
-    # Update packages
-    - run: sudo apt --assume-yes update
-      shell: bash
-    # Upgrade packages
-    - run: sudo apt --assume-yes upgrade
+    # apt-get rather than apt: apt says itself that it has no stable CLI for
+    # scripts. noninteractive and a retry count so a slow mirror cannot leave
+    # the step sitting there. There was a run on main that spent six hours in
+    # apt before it got cancelled.
+    # No `apt upgrade` either, it pulls in a lot for no benefit here.
+    - run: sudo apt-get -o Acquire::Retries=3 --assume-yes update
       shell: bash
-    # Install required packages
-    - run: sudo apt --assume-yes install make gcc binutils intltool libtool autotools-dev libreadline-dev python3
+      env:
+        DEBIAN_FRONTEND: noninteractive
+    - run: >
+        sudo apt-get -o Acquire::Retries=3 --assume-yes install
+        make gcc clang binutils intltool libtool libtool-bin
+        autotools-dev autoconf automake libreadline-dev python3
       shell: bash
-    # Run auto-generate
+      env:
+        DEBIAN_FRONTEND: noninteractive
     - run: ./autogen.sh
       shell: bash
-    # Run the configuration
-    - run: ./configure --enable-gui
+    - run: ./configure ${{ inputs.configure-args }}
       shell: bash
+      env:
+        CC: ${{ inputs.cc }}
+        CFLAGS: ${{ inputs.cflags }}
+        LDFLAGS: ${{ inputs.ldflags }}

.github/workflows/01-build.yml

@@ -1,18 +1,68 @@
-name: build-gui
-run-name: Build GUI by ${{ github.actor }}
+name: build
+run-name: Build by ${{ github.actor }}
 
-on: [push,pull_request,workflow_dispatch]
+on: [push, pull_request, workflow_dispatch]
 
 jobs:
-  build-gui:
-    runs-on: ubuntu-22.04
+  build:
+    name: ${{ matrix.cc }}
+    runs-on: ubuntu-24.04
+    timeout-minutes: 30
+    # clang does not fail the run yet. It builds clean but the scans come back
+    # wrong at -O1 and above, and that is not new: upstream 0375cc0 fails the
+    # same way, gcc is fine at every level, and clang -O0 is fine. It went
+    # unnoticed because the old suite asserted nothing about results and CI
+    # only ever built with gcc. Kept in the matrix so the failure stays
+    # visible instead of being dropped.
+    continue-on-error: ${{ matrix.cc == 'clang' }}
+    strategy:
+      fail-fast: false
+      matrix:
+        cc: [gcc, clang]
     steps:
-      - uses: actions/checkout@v3
-      - name: Linux set up
+      - uses: actions/checkout@v4
+      - name: Set up
         uses: ./.github/actions/linux-setup
-      - name: Make the binary
-        run: make CFLAGS='-O2 -fsanitize=address,undefined'
-      - name: Testing - requires `sudo` for `ptrace()`
+        with:
+          cc: ${{ matrix.cc }}
+          cflags: '-O2 -Wall'
+      - name: Build
+        run: make
+      - name: Test (needs sudo for ptrace)
         run: sudo make check VERBOSE=1
-      - name: Install Scanmem
+      - name: Show test log on failure
+        if: failure()
+        run: cat test/*.log || true
+      - name: Install
         run: sudo make install
+
+  sanitizers:
+    name: asan+ubsan
+    runs-on: ubuntu-24.04
+    timeout-minutes: 45
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up
+        uses: ./.github/actions/linux-setup
+        with:
+          cc: gcc
+          # float-cast-overflow is not in gcc's default -fsanitize=undefined
+          # set, and it is the one that caught the 64 bit parse bound bug
+          cflags: '-O1 -g -fno-omit-frame-pointer -fsanitize=address,undefined,float-cast-overflow'
+          ldflags: '-fsanitize=address,undefined,float-cast-overflow'
+          configure-args: '--enable-gui'
+      - name: Build
+        run: make
+      # memfake is the process being scanned, not code under test. Linked
+      # against asan it maps a huge shadow address space, which scanmem then
+      # scans, and the run never finishes. Build it clean first so `make check`
+      # finds it up to date and leaves it alone.
+      - name: Build the test target without sanitizers
+        run: make -C test memfake CFLAGS='-std=gnu99 -Wall -O2' LDFLAGS=''
+      - name: Test (needs sudo for ptrace)
+        run: sudo make check VERBOSE=1
+        env:
+          UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
+      - name: Show test log on failure
+        if: failure()
+        run: cat test/*.log || true

.github/workflows/01-coverity-scan.yml

@@ -9,9 +9,10 @@ on:
 
 jobs:
   coverity-scan:
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
+    timeout-minutes: 45
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Linux set up
         uses: ./.github/actions/linux-setup
       - name: Make clean

.gitignore

@@ -29,6 +29,9 @@ stamp-h1
 tags
 test-driver
 
+# Backup files, autoconf leaves configure~ behind
+*~
+
 # GameConqueror
 gui/org.scanmem.gameconqueror.metainfo.xml
 gui/org.scanmem.gameconqueror.desktop

Makefile.am

@@ -7,7 +7,7 @@ if ENABLE_GUI
 endif
 
 # '-O2 -g' are added by `configure`, unless the user overrides CFLAGS
-AM_CFLAGS = -std=gnu99 -Wall
+AM_CFLAGS = -std=gnu11 -Wall $(EXTRA_CFLAGS)
 
 # Utilities library, statically linked in sm and libsm
 noinst_LTLIBRARIES = libutil.la

README.md

@@ -1,6 +1,6 @@
 # ![](https://raw.githubusercontent.com/scanmem/scanmem/main/gui/GameConqueror_72x72.png)scanmem & GameConqueror
 
-[![Build Status](https://travis-ci.org/scanmem/scanmem.svg?branch=main)](https://travis-ci.org/scanmem/scanmem)
+[![Build Status](https://github.com/scanmem/scanmem/actions/workflows/01-build.yml/badge.svg)](https://github.com/scanmem/scanmem/actions/workflows/01-build.yml)
 [![Coverity Status](https://scan.coverity.com/projects/8565/badge.svg?flat=1")](https://scan.coverity.com/projects/scanmem)
 
 scanmem is a debugging utility designed to isolate the address of an arbitrary
@@ -10,6 +10,37 @@ the process and the value of the variable at several different times.
 After several scans of the process, scanmem isolates the position of the
 variable and allows you to modify its value.
 
+## Scan speed
+
+The first scan is the slow one. It has to run the comparison at every byte
+offset of every scanned region, where later scans only revisit addresses that
+already matched. It is split across threads now.
+
+`option threads N` sets the count. The default, `0`, uses one thread per online
+CPU. `1` scans serially.
+
+Measured on a 12 core machine, scanning for a value with 1000 matches in the
+region, best of 3:
+
+| region | serial | 12 threads |
+|--------|--------|------------|
+| 64MB   | 204ms  | 84ms       |
+| 256MB  | 734ms  | 208ms      |
+| 1GB    | 2936ms | 702ms      |
+
+How much this helps depends on how many cores are actually free, and on the
+target being big enough for the split to pay for itself. `threads 1` performs
+the same as the older serial code.
+
+Reads go through `process_vm_readv` where the kernel allows it, falling back to
+`/proc/pid/mem` and then to `ptrace`. On its own that call is roughly 2.8x
+faster than `pread` for this access pattern, but end to end it is worth a few
+percent at most, because the scan is limited by the comparison work rather than
+by reading.
+
+Any thread count returns the same matches. `bench/run.sh` reproduces the numbers
+above and `make check` compares a chunked scan against an unchunked one.
+
 ## GUI
 
 GameConqueror is a GUI front-end for scanmem, providing more features, such as:

bench/BASELINE.md

@@ -0,0 +1,97 @@
+# Baseline, before any optimisation
+
+Recorded at `6a81d1e` with `bench/run.sh`.
+
+    CPU:    AMD Ryzen 5 3600X 6-Core Processor (12 threads)
+    Kernel: 7.0.12+kali-amd64
+    CC:     gcc (Debian 15.3.0-2) 15.3.0
+
+Target is memfake with every slot in the buffer set to the search value, so
+the match set is as large as the buffer allows. That is deliberately the worst
+case for the narrowing scan.
+
+| target | matches | initial scan | narrow | pread64 calls |
+|---|---|---|---|---|
+| 16MB | 2,097,152 | 0.19s | 0.22s | 8,233 |
+| 128MB | 16,777,216 | 1.33s | 1.95s | 65,695 |
+
+Medians of 3, each run against a freshly started target.
+
+## What the syscall count says
+
+The narrowing scan reads every match back through `sm_peekdata`, which
+refills its cache `PEEKDATA_CHUNK` (2048) bytes at a time. 128MB of matches
+divided by 2048 is 65,536, and we measure 65,695 including the initial scan's
+1MB reads and a handful of setup. So the narrowing scan is spending one
+syscall per 2KB of match data regardless of how the matches are laid out.
+
+`process_vm_readv` accepts up to IOV_MAX (1024 here) remote iovecs per call,
+so the same work should collapse to roughly 64 calls instead of 65,536.
+
+# After
+
+Same machine. Each entry is `bench/run.sh` or the equivalent, medians or best
+of 3, against a freshly started target every time.
+
+## Narrowing scan, dense target
+
+The dense case above, 128MB with every slot matching:
+
+| | before | after |
+|---|---|---|
+| initial scan | 1.33s | 0.58s |
+| narrow | 1.95s | 0.48s |
+| read syscalls | 65,695 | ~1,100 |
+
+Two changes account for it. `sm_peekdata` reads ahead while the scan walks
+forwards instead of refilling 2048 bytes at a time, which is where the syscall
+count went. Then `sm_peekdata` and `add_element` got inline fast paths for the
+cases the per byte loop actually hits, a cache hit and a contiguous append,
+because both were out of line calls being made once per scanned byte.
+
+Worth noting the syscalls fell about 30x while the time fell about 4x. The
+scan is limited by the comparison work, not by reading. That is also why
+`process_vm_readv` is only worth a few percent end to end despite being 2.8x
+faster than `pread` on its own.
+
+## Initial scan, threads
+
+Realistic shape this time, 1000 matches rather than every slot, because the
+initial scan cost tracks region size and not match count:
+
+| region | serial | threads=1 | threads=12 |
+|---|---|---|---|
+| 64MB | 204ms | 208ms | 84ms |
+| 256MB | 734ms | 732ms | 208ms |
+| 1GB | 2936ms | 2917ms | 702ms |
+
+Short of linear, and it should be. Reading and comparing are shared across
+cores but the stitch that turns per chunk records back into swaths is serial,
+and so is everything before the scan starts.
+
+A note on measuring this: the first attempt showed threads=1 coming out 10 to
+20 percent behind the old serial path, which looked like real overhead from the
+chunking. It was two orphaned scanmem processes from earlier timed out runs
+sitting at 95% CPU. On an idle machine threads=1 is level with serial. Check
+what else is running before believing a regression this size.
+
+# Known: clang builds scan wrong
+
+Worth writing down because it cost a while to pin down. scanmem built with
+clang at -O1 or higher finds almost nothing. The scan runs, reads the right
+memory, parses the right value and picks the right routine, and still returns
+3 matches where gcc returns 102.
+
+Not introduced by any of the work above. Upstream 0375cc0 fails identically
+once you point an asserting test suite at it, which nothing did before, since
+the old suite only checked that scanmem exited zero and CI only built gcc.
+
+Ruled out so far: strict aliasing (-fno-strict-aliasing does not help),
+vectorisation, signed overflow (-fwrapv), and the extern inline definitions
+(removing them entirely does not help). -fno-inline takes it from 4/23 to
+16/23 checks passing, so inlining is exposing it rather than causing it.
+
+Reproduce:
+
+    ./configure CC=clang CFLAGS='-O2 -Wall' && make
+    sudo make check