Skip to content

perf: stream image outputs directly into zip entries - #44

Open
orvalvera wants to merge 1 commit into
iii123iii:mainfrom
orvalvera:codex/stream-zip-image-output-2
Open

perf: stream image outputs directly into zip entries#44
orvalvera wants to merge 1 commit into
iii123iii:mainfrom
orvalvera:codex/stream-zip-image-output-2

Conversation

@orvalvera

@orvalvera orvalvera commented Jul 15, 2026

Copy link
Copy Markdown

/claim #2
Fixes #2

What changed

  • Stream generated PDF-to-image pages directly into each ZIP entry instead of buffering every rendered image into a separate byte array first.
  • Stream extracted PDF images directly into each ZIP entry as well.
  • Add service-level ZIP/image decode coverage for both streaming paths and validate unsupported extract-image formats as user-input errors.
  • Keep the same filenames, formats, and ZIP response behavior while avoiding one extra allocation and copy per page/image.

Why

Large PDFs can generate many high-DPI images. Writing each image through an intermediate ByteArrayOutputStream amplifies peak memory usage and adds an unnecessary copy before the ZIP write. Streaming into the ZipOutputStream reduces that work for the image conversion and image extraction paths.

Verification

  • git diff --check
  • JAVA_HOME=/Users/orval/Documents/Codex/2026-07-14/generame-5-dolares-y-haz-algo/.tools/jdk-21.0.11+10/Contents/Home PATH=/Users/orval/Documents/Codex/2026-07-14/generame-5-dolares-y-haz-algo/.tools/jdk-21.0.11+10/Contents/Home/bin:$PATH bash ./gradlew test --tests com.crystalpdf.backend.service.PdfToImageServiceTest --tests com.crystalpdf.backend.service.ExtractImagesServiceTest
    • Result: BUILD SUCCESSFUL (3 tests, 3 passed).

Copilot AI review requested due to automatic review settings July 15, 2026 05:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Improves backend PDF-to-image and image-extraction tooling by writing image encodings directly into ZIP entries, reducing per-image buffering/copy overhead and lowering peak memory usage for large/high-DPI outputs.

Changes:

  • Stream rendered PDF pages (PdfToImageService) directly into ZipOutputStream entries via ImageIO.write(...).
  • Stream extracted embedded images (ExtractImagesService) directly into ZipOutputStream entries via ImageIO.write(...).
  • Add explicit handling for unsupported output formats via the boolean return value of ImageIO.write(...).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
backend/src/main/java/com/crystalpdf/backend/service/PdfToImageService.java Streams rendered page images directly into ZIP entries instead of buffering per-page image bytes.
backend/src/main/java/com/crystalpdf/backend/service/ExtractImagesService.java Streams extracted images directly into ZIP entries and adds explicit unsupported-format handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to +51
if (!ImageIO.write(bimg, imgFormat, zos)) {
throw new IOException("Unsupported image output format: " + imgFormat);
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Handled in the current branch: ExtractImagesService validates png / jpg / jpeg up front and throws IllegalArgumentException for unsupported formats, so GlobalExceptionHandler maps it to a 400. Verified with JAVA_HOME=/Users/orval/Documents/Codex/2026-07-14/generame-5-dolares-y-haz-algo/.tools/jdk-21.0.11+10/Contents/Home sh gradlew test --tests com.crystalpdf.backend.service.PdfToImageServiceTest --tests com.crystalpdf.backend.service.ExtractImagesServiceTest from backend (3/3 passed).

Comment on lines 34 to 39
ZipEntry entry = new ZipEntry(String.format("page_%03d.%s", i + 1, fmt));
zos.putNextEntry(entry);
zos.write(imgOut.toByteArray());
if (!ImageIO.write(image, imageioFormat, zos)) {
throw new IOException("Unsupported image output format: " + imageioFormat);
}
zos.closeEntry();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Covered in PdfToImageServiceTest.convert_returnsZipWithDecodablePageImages: the test opens the returned bytes with ZipInputStream, checks the expected page_001.png / page_002.png entry names, reads each ZIP entry, and verifies ImageIO.read(...) returns a decodable image. Focused Gradle run passed 3/3.

Comment on lines 47 to 52
String entryName = String.format("page%d_img%d.%s", pageIdx + 1, imgIndex++, imgFormat);
zos.putNextEntry(new ZipEntry(entryName));
zos.write(imgBytes.toByteArray());
if (!ImageIO.write(bimg, imgFormat, zos)) {
throw new IOException("Unsupported image output format: " + imgFormat);
}
zos.closeEntry();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Covered in ExtractImagesServiceTest: extractImages_returnsZipWithDecodableImages opens the ZIP, checks the page1_img1.png entry name, and verifies the entry decodes through ImageIO.read(...); extractImages_throwsForUnsupportedFormat locks the invalid-format behavior to IllegalArgumentException. Focused Gradle run passed 3/3.

@orvalvera
orvalvera force-pushed the codex/stream-zip-image-output-2 branch from 584c39c to af08679 Compare July 15, 2026 05:43
@orvalvera

Copy link
Copy Markdown
Author

Addressed the automated review feedback in the latest push: added service-level ZIP/image decode coverage for both PDF-to-image conversion and embedded-image extraction, and changed unsupported extract-image formats to fail as IllegalArgumentException user-input errors. Verification note remains: local backend tests cannot run in this environment because no Java Runtime is installed.

@orvalvera

Copy link
Copy Markdown
Author

Update: I added a local Temurin 21 JDK and reran the focused backend tests. PdfToImageServiceTest and ExtractImagesServiceTest pass: BUILD SUCCESSFUL with 3/3 tests passing. I also updated the PR body verification section.

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.

Fix speed

2 participants