Skip to content

registry: add JMH benchmark suite - #590

Open
iamabhilaksh wants to merge 2 commits into
salesforce:mainfrom
iamabhilaksh:feat/registry-benchmark-suite
Open

registry: add JMH benchmark suite#590
iamabhilaksh wants to merge 2 commits into
salesforce:mainfrom
iamabhilaksh:feat/registry-benchmark-suite

Conversation

@iamabhilaksh

Copy link
Copy Markdown
Contributor

Summary

Adds a JMH benchmark suite for the container registry: AbstractRegistryBenchmarkTest (registry-client) plus thin AWS/GCP concretes. Gated by @EnabledIfSystemProperty(named="runBenchmarks", matches="true") — inert in CI.

Swept (4): pullManifest, extractLayers, pullAndExtract, pullMultiArch (self-guards to a no-op unless a multi-arch test image is configured).

Measurement caveat

The pull benchmarks re-pull the same image ref each invocation, so the registry's edge cache and the warm connection pool make these steady-state / cache-hit numbers, not cold-pull latency. Documented in-source.

Testing proof

Run locally against live AWS ECR + GCP Artifact Registry on 2026-08-11, both JMH modes. The 3 real methods produced populated results on both clouds; pullMultiArch reads ~0 until a multi-arch fixture is wired (by design).

Invocation (creds via OS env only):

mvn test -pl registry/registry-aws -Dtest=AwsRegistryBenchmarkTest -DrunBenchmarks=true \
  -DREGISTRY_BENCHMARK_AWS_ENDPOINT=<ecr-endpoint> -DREGISTRY_BENCHMARK_AWS_REGION=us-west-2 \
  -DREGISTRY_BENCHMARK_AWS_SMALL_IMAGE_REF=<image:tag>
mvn test -pl registry/registry-gcp -Dtest=GcpRegistryBenchmarkTest -DrunBenchmarks=true \
  -DREGISTRY_BENCHMARK_GCP_ENDPOINT=<ar-endpoint> -DREGISTRY_BENCHMARK_GCP_SMALL_IMAGE_REF=<image:tag>

Rendered AWS-vs-GCP pages are being published to the benchmark-visualizer.

Run contract

  • registry-aws: REGISTRY_BENCHMARK_AWS_{ENDPOINT,REGION,SMALL_IMAGE_REF} (+ optional _MULTIARCH_IMAGE_REF)
  • registry-gcp: REGISTRY_BENCHMARK_GCP_{ENDPOINT,SMALL_IMAGE_REF} (+ optional _MULTIARCH_IMAGE_REF)

JMH config note

Class-level annotations are the local-run baseline; the chameleon pipeline overrides via its own BenchmarkRunner.

Downstream

Needs a separate vendor-sync PR into sfdc-bazel to run in the pipeline; not automatic.

Merge order

Independent. Recommend the root-pom JMH fix merges first.

GUS: W-23830175

Adds an abstract AbstractRegistryBenchmarkTest in registry-client plus thin
AWS/GCP concretes, gated by @EnabledIfSystemProperty(runBenchmarks=true).
Swept: pullManifest, extractLayers, pullAndExtract, pullMultiArch (self-guards
to a no-op when no multi-arch fixture is configured). Pull benchmarks are
warm-cache steady-state, not cold-pull latency (documented in-source).

Story: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002hP986YAC/view (W-23830175)
@codecov-commenter

codecov-commenter commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.61%. Comparing base (aea85ed) to head (71a14a4).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #590   +/-   ##
=========================================
  Coverage     83.61%   83.61%           
  Complexity      674      674           
=========================================
  Files           215      215           
  Lines         15010    15010           
  Branches       2076     2076           
=========================================
  Hits          12550    12550           
  Misses         1636     1636           
  Partials        824      824           
Flag Coverage Δ
unittests 83.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

* measured time reflects decompress + whiteout-flatten cost, not just the first read.
*/
@Benchmark
public void benchmarkExtractLayers(Blackhole bh) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ benchmarkExtractLayers re-downloads every layer on each invocation, so it isn't measuring decompression

The javadoc at :147-148 describes decompression throughput for a pre-pulled image, but the pre-pull retains only metadata. pull() returns a RemoteImage holding transport, repository, imageRef and a Manifest — digests and descriptors, no bytes — and its own javadoc says as much: "blobs are only downloaded when accessed via Image#getLayers()" (registry-client/src/main/java/com/salesforce/multicloudj/registry/driver/AbstractRegistry.java:95-96).

extract() then calls getLayers() (AbstractRegistry.java:216), and RemoteImage.getLayers() (driver/RemoteImage.java:29-39) builds fresh RemoteLayer objects holding only digests on every call, whose getUncompressed() (driver/RemoteLayer.java:31) issues an unconditional GET through OciHttpTransport.downloadBlob (driver/OciHttpTransport.java:396-442). There is no content cache anywhere on that path — the only two caches in the module (OciHttpTransport:64, AwsRegistry:45) are auth, not content, and neither harness injects a caching HTTP client. LayerExtractor walks every layer (:94-96, :131) and the benchmark drains the full stream (:152-153), so each invocation transfers the entire compressed image.

The clearest symptom: benchmarkExtractLayers and benchmarkPullAndExtract (:165) now differ by exactly one manifest GET, so two benchmarks measure nearly the same thing and neither isolates decompression. Fetching the layer bytes once in @Setup into a reusable buffer would fix it; failing that, renaming to reflect full image materialisation would at least make the number honest. The pipelining objection doesn't rescue it — extraction does run on a background thread over a 64 KB pipe (LayerExtractor:62-108), but layers are processed strictly sequentially and a pipeline is bounded by its slower stage. Note too that no choice of fixture helps: a large image makes it bandwidth-bound, a tiny one RTT-bound, and neither is decompression throughput.

Same defect class one method up at :138. multiArchImageRef comes from optionalEnv and is unset by default on both providers, so benchmarkPullMultiArch early-returns and JMH publishes a timed empty method. I measured that shape with JMH 1.37 under this suite's own class annotations: ~1.5 × 10⁹ ops/s (≈0.6 ns/op) in Throughput against 17.4 ops/s for a 50 ms control call, so the "reads ~0" note in the PR body holds for the SampleTime rows only — in the JSON the Throughput row is the largest number in the file, and the record is indistinguishable from a real measurement. Same suggestion I left on #588: drive it from a capability predicate feeding .exclude() rather than emitting a datapoint for work that never ran.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right on both, and I ran it live against ECR and GAR to confirm rather than just eyeballing the source.

On benchmarkExtractLayers: the pre-pull only holds metadata, so extract() re-fetches the layer bytes on every invocation through RemoteLayer.getUncompressed() with no content cache on the path. The live numbers make it plain — extractLayers tracks a pull-shaped cost, not decompression (AWS: pullAndExtract 1.051 ≈ extractLayers 0.703 + pullManifest 0.398 s/op; GCP: extractLayers 0.794 vs pullAndExtract 0.417 ops/s). Since there's no way to fixture our way out of that, I dropped the benchmark entirely rather than rename it — pullAndExtract already owns the honest full-materialisation number.

On benchmarkPullMultiArch: confirmed the phantom. With the ref unset, AWS published pullMultiArch at 558,215,787 ops/s — literally the largest number in the file, indistinguishable from a real pull. Went with the capability-predicate approach you suggested on #588: a supportsMultiArch() default on the Harness feeding .exclude(".*benchmarkPullMultiArch.*"), so it's suppressed where there's no fixture and still runs where there is (GCP kept it at 0.668 ops/s after the fix, AWS correctly dropped it).

Fix is in the follow-up commit. Thanks for the detailed trace through the transport layer — the auth-vs-content cache distinction is exactly what I verified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants