Skip to content

Issue SAM3 remote requests concurrently instead of one frame at a time - #2886

Draft
rafel-roboflow wants to merge 1 commit into
mainfrom
worktree-sam3-concurrent-remote
Draft

Issue SAM3 remote requests concurrently instead of one frame at a time#2886
rafel-roboflow wants to merge 1 commit into
mainfrom
worktree-sam3-concurrent-remote

Conversation

@rafel-roboflow

Copy link
Copy Markdown
Contributor

What does this PR do?

The SAM3 concept-segmentation blocks receive a batch of frames for remote execution, then loop over it and block on one HTTP round trip per frame. Video preview runs pay that serially for the whole video.

Measured on a staging video-preview run (sam3 + mask/label visualization, 264 frames):

serverless calls 264 for 264 frames (1 request(s) with 1 frame(s) each)
per-call latency median 244ms, p90 283ms, p99 519ms
wall clock in calls 67.8s of the 67.8s run
throughput 3.9 fps

A detection workflow on the same worker, same endpoint, issued its requests in parallel and got 31ms/frame.

This PR hands the whole batch to the SDK in a single call, so execute_requests_packages fans the per-image requests out concurrently instead of the block awaiting them one by one. The image-independent prompt building moves out of the loop; the loop now only maps responses back onto their images.

max_batch_size stays pinned at 1 deliberately. /sam3/concept_segment takes a single image (Sam3SegmentationRequest.image: InferenceRequestImage), and any larger batch size makes the SDK's inject_images_into_payload put a list into that field, which the endpoint would reject. Concurrency is the safe lever here, and it is exactly what the serial loop was giving up.

Applies to all six variants — v1/v2/v3, plain and _tensor — since ENABLE_TENSOR_DATA_REPRESENTATION decides which pair loads.

Related Issue(s): Found while investigating slow video previews in the streaming preview worker.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

  • I have tested this change locally
  • I have added/updated tests for this change

Test details:

New tests/workflows/unit_tests/core_steps/models/foundation/test_segment_anything3_remote_concurrency.py, parametrized over all six block variants (24 tests):

  • a 4-frame batch reaches the SDK as one call carrying all four images, not four calls
  • the client is configured with max_batch_size == 1 and max_concurrent_requests > 1
  • results stay aligned one-per-image
  • a single-image batch still works (the SDK unwraps a one-element list to a bare dict)

Every test fails on main for the right reason and passes here. Also run: the whole tests/workflows/unit_tests suite (5996 passed) and tests/workflows/unit_tests/core_steps/dependent_resources (225 passed), plus a block-loader import check with the tensor flag both off and on (235 blocks each). The 17 failures in that suite (zone geometry, rle compact, detections mismatch/stitch, semantic segmentation) reproduce unchanged on the base commit and are untouched by this change. black and isort are clean.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable) — N/A

Additional Context

What this does not change: the 244ms per call itself. That is one SAM3 forward pass plus network, plus an extra proxy hop on the hosted path. What changes is that independent frames no longer wait in line for each other — expected gain is roughly the concurrency factor (WORKFLOWS_REMOTE_EXECUTION_MAX_STEP_CONCURRENT_REQUESTS, 8 in the streaming preview deployment), bounded in practice by how the serverless GPU fleet handles the parallelism. Worth watching fleet behaviour when this ships.

Left alone deliberately: run_via_request has the same serial shape, but it posts directly with requests rather than through the SDK, so making it concurrent needs its own thread pool. It is only reachable with SAM3_EXEC_MODE=remote (default is local), and it is not the path the measurements above came from. Happy to do it as a follow-up.

The SAM3 concept-segmentation blocks received a batch of frames but looped
over it, blocking on one HTTP round trip per frame. A staging video preview
run showed the cost: 264 serverless calls for a 264-frame job, 244ms median
each, with the whole 67.8s of wall clock spent inside those calls (3.9 fps).
A detection workflow on the same worker issued its requests in parallel and
managed 31ms/frame.

Hand the whole batch to the SDK in a single call so it fans the per-image
requests out concurrently, and hoist the image-independent prompt building
out of the loop. The loop now only maps responses back onto their images.

max_batch_size stays pinned at 1: the /sam3/concept_segment endpoint takes a
single `image`, and a larger batch size would make the SDK put a list in that
field. Concurrency is the part that is safe to raise, and it is what the
serial loop was giving up.

Applies to all six variants (v1/v2/v3, plain and tensor). run_via_request has
the same serial shape but a different transport, and is only reachable with
SAM3_EXEC_MODE=remote; left alone here.
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely.

🚧 Right now this is a draft, so automated Claude review is paused — nothing is being spent yet. Mark it Ready for review to trigger it.

Warning

💸 The Claude reviewer bills in credits, not vibes

Automated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared.

Draft when unsure, Ready when you mean it:

  • 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
  • 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

  • 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
  • Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
  • 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
  • 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

  • Prefer to skip automated review entirely? Add the skip-claude-review label.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Claude review started at commit c4f0b5a511cae747750382b4f9a8510a7680867c.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

@rafel-roboflow
rafel-roboflow marked this pull request as draft August 27, 2026 20:45
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