Skip to content

Don't queue full-resolution originals when preprocessing on the image queue - #256

Closed
putdanil wants to merge 1 commit into
agentmorris:mainfrom
putdanil:image-queue-uint8
Closed

putdanil wants to merge 1 commit into
agentmorris:mainfrom
putdanil:image-queue-uint8

Conversation

@putdanil

Copy link
Copy Markdown
Contributor

With --use_image_queue --preprocess_on_image_queue, each loader puts the whole preprocess_image() dict on the queue. Besides the letterboxed image, that dict holds the original image twice, as a numpy array and as a PIL image: about 2/3 of the pickled bytes for each image (≈21 MB for a 5 MP frame). The consumer process unpickles all of it, and as a result the preprocessing queue is currently slower than the default path.

Inference only needs the original image's shape. This PR:

  • records img_original_shape in PTDetector.preprocess_image() and uses it in the batch path instead of img_original.shape;
  • has loaders set img_original_pil and img_original to None before queueing, unless the consumer needs them (--include_image_size, --include_image_timestamp, --include_exif_tags).

Only dicts that carry img_original_shape are changed, i.e. those from PTDetector. RF-DETR runs inference on the original image, so its dicts are queued exactly as before.

Nothing changes when the queue is off or preprocessing is not done on the queue.

Measurements

MDv1000-redwood, run_detector_batch CLI, 500 images from LILA (mixed resolutions), an NVIDIA GPU + 8 vCPU (Xeon Gold 6248R VM). Images/s excluding start-up (from runs on 100 and 500 images):

configuration main this PR this PR + #255
--use_image_queue --preprocess_on_image_queue 5.3 12.6 16.5
same with --batch_size 8 8.0 24.3 34.0
same with --use_threads_for_queue (whole run, 500 images) 2.9 6.5

For comparison, main without the queue: 6.9 (default), 10.3 (--batch_size 8). Single runs; run-to-run spread on this machine is a few percent. multiprocessing.JoinableQueue pickles items for threads too, which is why the thread-based queue also benefits.

Equivalence

The images list of the output file is identical to main's for: --use_image_queue --preprocess_on_image_queue at batch size 1 and 8, with --use_threads_for_queue, and with --include_image_size --include_image_timestamp --include_exif_tags all (including the width, height, datetime, and EXIF fields).

With --batch_size 8 on the queue, main is not deterministic run to run: batches are formed in the order images arrive from the loaders, and two runs of main differ on 22 of 500 images (confidence in the last digit, 0.001; no box changes). The patched outputs differ from main by exactly that much.

Tests

Run on main and with all three of my PRs applied (#255, #256, #257), on the same machine:

  • pytest -vv: 24 passed, 4 skipped, on both.
  • python -m megadetector.utils.md_tests --max_coord_error 0.01 --max_conf_error 0.01 --skip_download_tests --warning_mode (GPU, CPU, and video tests): all 35 steps run on both, and the comparison lines against the expected results are the same. 40 of the 41 result files are identical to main's; the remaining one (worker-side preprocessing with batched inference, the non-deterministic case above) differs on 1 of 47 images by 0.001 in confidence.

--warning_mode is there because, independently of these PRs, md_tests fails on main at "Running MD on a folder of videos": DSCF0028.MP4 is reported with a confidence error of 0.667, although every box above 0.5 matches the expected results at IoU ≥ 0.998 with confidence within 0.021 (and the assertion message names two boxes with confidence 0.005). Presumably the frames decode slightly differently here: opencv-python 5.0.0 (FFmpeg avcodec 62.28.101), torch 2.14.1+cu130, NVIDIA driver 580.105.08, Ubuntu 22.04, Python 3.11.14.


This came out of benchmarking run_detector_batch end to end (related to the TODO item "Performance evaluation for batching, preprocessing"); happy to share the full numbers. Prepared with help from Claude Code; I reviewed the diff and the measurements.

… queue

With --use_image_queue --preprocess_on_image_queue, each loader put the whole
preprocess_image() dict on the queue, including the original image as both a
numpy array and a PIL image (about 2/3 of the pickled bytes).  Inference only
needs the original image's shape, so PTDetector.preprocess_image() now records
img_original_shape, the batch path uses it, and loaders drop both copies unless
image size, timestamp, or EXIF output was requested.  Only dicts that carry
img_original_shape are changed: RF-DETR runs inference on the original image,
so its dicts are queued as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@agentmorris

Copy link
Copy Markdown
Owner

Thanks! You're right that it's silly to pass the original images around like this when they've already been preprocessed; this is a historical artifact. But I'd rather not fix it by just sometimes not passing them; the whole path where two copies of the image get passed around should be dropped. This also should get handled for video and images at the same time. So, closing this PR without merging, but I've added a TODO for this, if you'd like to take a shot at this, go for it.

Thanks for pointing this out!

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.

2 participants