Major Architecture Refactor & Performance Improvements#3
Open
karimElmougi wants to merge 41 commits intonicoburniske:mainfrom
Open
Major Architecture Refactor & Performance Improvements#3karimElmougi wants to merge 41 commits intonicoburniske:mainfrom
karimElmougi wants to merge 41 commits intonicoburniske:mainfrom
Conversation
At 100-200 images per volume, having multiple in memory at once and fighting for threads is too much contention.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of all, I would like to apologize for the gigantic PR. I could not find a way to split it up without ending in conflict-hell rewriting the history.
This PR represents a comprehensive refactoring of
comically, restructuring the project into a workspace with three separate crates, reorganizing the image processing pipeline, and implementing significant performance optimizations. The result is a ~2.6x speedup in processing time.Architecture Changes
Workspace Restructure
Split the monolithic crate into a workspace with three focused crates:
comically- Core library with image processing, archive handling, and format conversion logiccomically-tui- Terminal UI application (the original user interface)comically-cli- New command-line interface for automation and scriptingThe CLI made performance profiling significantly easier, and this structure would allow for a GUI version of
comicallyto reuse the core logic.Module Organization
Within the
comicallylibrary crate, the code is now organized into focused modules:image/- Image processing pipeline split into:decode.rs- Image decoding from various formatsencode.rs- Compression and encoding (JPEG/PNG/WebP)transform.rs- Cropping, resizing, splitting, rotation, gamma correctionmod.rs- High-level batch processing APIdevice.rs- Device presets and dimension handling (extracted from config)comic.rs- Core types (ComicFile,ComicConfig,ProcessedImage)archive.rs- Archive file reading (CBZ/CBR/ZIP/RAR)epub.rs- EPUB generation (now builds in memory)cbz.rs- CBZ generationmobi.rs- MOBI conversion (via kindlegen)Performance Optimizations
Image Processing (~2.6x overall speedup)
Switched to
fast_image_resize(fromimageproc::imageops::resize)imageproc::resize, but visual quality is equivalent or betterGamma lookup table
Zero-copy image transformations
CroppedImagetype for zero-copy cropping operationsMemory allocation optimizations
arrayvecfor split images (1-3 images) to avoid heap allocationVec::with_capacitythroughout to reduce reallocationsParallelism Strategy Refinement
The rationale is that, with 100-200 images per manga volume, having multiple volumes in memory simultaneously creates excessive thread contention and memory pressure. I noticed significantly degraded performance when trying to process multiple volumes at the same time.
Archive Processing
In-Memory EPUB Building
Build EPUB archives entirely in memory instead of using temporary files, reducing I/O overhead and improving performance.
Performance Results
Processing all the Naruto volumes (17.5 GiB) on my M2 Macbook Air:
I noticed it took around 3.5-4.2s per volume.
Output Compatibility
Until the
fast_image_resizeoptimization:After the
fast_image_resizeoptimization:imageproc::resizeoutputFiles Worthy of Attention
New Crate:
comically-cli/src/main.rs(355 lines, new file)Complete command-line interface for batch processing comics. Includes:
clapCore Processing:
comically-tui/src/pipeline.rs(292 lines, new file)Orchestrates the processing pipeline for the TUI:
comicallylibraryProgress Reporting:
comically-tui/src/tui/progress.rs(139 lines, modified)Improved progress tracking:
Core Library:
comically/(entire crate, ~1,500 lines)Complete rewrite/reorganization of the core processing logic:
comically/src/image/mod.rs(178 lines)High-level image processing API:
process_batch()- Parallel batch processingprocess_batch_with_progress()- With progress callbacksSplit<T>- Stack-allocated container for split images (arrayvec)comically/src/image/transform.rs(584 lines)Core image transformations:
fast_image_resizecomically/src/image/encode.rs(141 lines)Image compression:
comically/src/image/decode.rs(12 lines)Simple image decoding wrapper around
imageproc.comically/src/comic.rs(172 lines)Core types and configuration:
ComicFile- Represents a comic archive fileComicConfig- Processing configurationProcessedImage- Output image dataSplitStrategy,OutputFormatenumscomically/src/device.rs(163 lines)E-reader device presets:
comically/src/epub.rs(288 lines)In-memory EPUB generation:
comically/src/archive.rs(32 lines, modified)Archive file reading:
comically/src/cbz.rs(33 lines)CBZ archive generation.
comically/src/mobi.rs(12 lines, modified)MOBI conversion coordination (delegates to kindlegen).
Dependencies Added
fast_image_resize(5.0) - High-performance image resizingarrayvec(0.7) - Stack-allocated vectors for split imagesnum_enum(0.7) - Enum derives for device presetsparking_lot(0.12) - Better mutex implementationstrum(0.27) - Enum utilitiesTesting
All changes have been tested with: