Skip to content

perf: improve CPU throughput and default to configurable INT8 - #8

Merged
TorstenDittmann merged 3 commits into
mainfrom
perf/cpu-inference-int8
Sep 7, 2026
Merged

perf: improve CPU throughput and default to configurable INT8#8
TorstenDittmann merged 3 commits into
mainfrom
perf/cpu-inference-int8

Conversation

@TorstenDittmann

@TorstenDittmann TorstenDittmann commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Enable bounded concurrent analysis on one shared ONNX CPU session, defaulting to effective CPU count with one intra-op thread. Lifecycle locking prevents teardown during inference.
  • Reuse bounded input buffers, streamline NRGBA normalization without changing alpha rounding, and remove the redundant output copy.
  • Default to the verified INT8 U²-Net artifact on both amd64 and arm64. MODEL_PRECISION=fp32 selects FP32; an explicit MODEL_PATH overrides precision. Invalid precision values fail startup unless a custom path is supplied.
  • Bundle both models in Docker and log the selected model and concurrency settings.
  • Retain reproducible calibration/evaluation tools, raw results, model provenance/license, and integration/concurrency regressions.

Measured trade-off

On the supplied Intel Xeon, two four-CPU HTTP comparisons measured 4.06–4.31 req/s for INT8 vs 2.95–2.98 for FP32, with 64–65% less peak cgroup memory. These compare precisions with the same concurrent pipeline, not the old serialized service. The throughput gain has not been established on ARM64.

The model was calibrated on 128 ECSSD images and evaluated on 200 separate public images plus nine extra fixtures. Median focal-point shift was 0.10%, p95 0.91%, and maximum 7.4% of an image dimension. One public image lost over five percentage points of foreground coverage in a tested crop shape; none lost over ten. Difficult-scene regressions remain; this is an intentional speed/quality trade-off, not lossless conversion.

Details: tools/quantization/RESULTS-expanded.md.

Validation

  • make model (both artifact checksums)
  • go test -race ./...
  • go vet ./...
  • Real-model race-enabled integration tests in INT8 and FP32 modes
  • Native Docker builds on amd64 and arm64
  • Go formatting, shell/Python syntax, result JSON validation, and crop-scoring unit tests
  • The benchmarks are short runs, not a sustained soak test

Packaging / rollout notes

  • Adds the 42 MiB INT8 binary to git, with its SHA-256 pinned in Makefile and Dockerfile. This keeps clean/release builds independent of a temporary calibration host; raw datasets and user images are not bundled.
  • Docker retains FP32 for rollback without rebuilding: MODEL_PRECISION=fp32.
  • Remove an existing MODEL_PATH override if you want precision selection to take effect.
  • No production deployment or release is performed by this PR.

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR enables concurrent inference through a shared lifecycle-protected ONNX session, reuses preprocessing buffers, and makes the bundled INT8 U²-Net artifact the configurable default.

  • Adds bounded request concurrency and ONNX thread configuration.
  • Optimizes image normalization and output-buffer ownership.
  • Bundles and verifies INT8 and FP32 models with environment-based selection.
  • Expands unit, integration, concurrency, and benchmark coverage.
  • The latest revision removes container runtime validation and the quantization reproduction materials.

Confidence Score: 5/5

The implementation appears safe to merge from a runtime-correctness perspective, with non-blocking gaps in container validation and model reproducibility.

No blocking code defect remains. The unresolved feedback concerns loss of CI coverage for the shipped container and removal of the scripts and raw evidence needed to reproduce the new default model; both are non-blocking quality issues. The prior memory-risk thread was manually resolved without explanation, while the vulnerable Pillow pin and contradictory precision report were eliminated by the latest deletions and documentation updates.

Files Needing Attention: .github/workflows/ci.yml, tools/quantization/README.md

Important Files Changed

Filename Overview
cmd/autogravity/main.go Adds model-precision selection, concurrency configuration, and pooled preprocessing buffers with validation.
internal/saliency/model.go Enables concurrent shared-session inference while preventing teardown during active runtime calls.
internal/imageutil/image.go Adds reusable tensor preparation while preserving NRGBA premultiplication behavior.
Dockerfile Bundles both model variants and defaults the container to INT8, but the resulting image is no longer exercised in CI.
.github/workflows/ci.yml Builds architecture-specific images but removes runtime validation of their packaged configuration.
tools/quantization/README.md Deletes the primary reproduction guide along with the associated quantization tooling and raw evidence.
models/README.md Records model provenance and aggregate evaluation results, but does not replace the removed reproducibility assets.

Comments Outside Diff (1)

  1. .github/workflows/ci.yml, line 68 (link)

    P2 Container Validation Removed

    The Docker job now exports only a build cache and never starts the built image. The removed container test was the only coverage for the image's bundled model paths, working directory, environment defaults, health check, and INT8, FP32, and custom-path startup modes. As a result, container packaging regressions can pass CI even when the direct Go integration tests succeed.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: .github/workflows/ci.yml
    Line: 68
    
    Comment:
    **Container Validation Removed**
    
    The Docker job now exports only a build cache and never starts the built image. The removed container test was the only coverage for the image's bundled model paths, working directory, environment defaults, health check, and INT8, FP32, and custom-path startup modes. As a result, container packaging regressions can pass CI even when the direct Go integration tests succeed.
    
    ---
    
    For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Fix in Claude Code Fix in Codex

Fix all with Greploop Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
.github/workflows/ci.yml:68
**Container Validation Removed**

The Docker job now exports only a build cache and never starts the built image. The removed container test was the only coverage for the image's bundled model paths, working directory, environment defaults, health check, and INT8, FP32, and custom-path startup modes. As a result, container packaging regressions can pass CI even when the direct Go integration tests succeed.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (2): Last reviewed commit: "docs: remove one-off quantization toolin..." | Re-trigger Greptile

Comment thread cmd/autogravity/main.go
Comment thread tools/quantization/requirements.txt Outdated
Comment thread tools/quantization/RESULTS-expanded.md Outdated
@TorstenDittmann
TorstenDittmann merged commit 87164b4 into main Sep 7, 2026
4 checks passed
@TorstenDittmann
TorstenDittmann deleted the perf/cpu-inference-int8 branch September 7, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant