fix(memory): use staging folder to avoid keeping everything in memory

Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Fixes #203

internal/cbz/cbz_creator.go

@@ -3,6 +3,7 @@ package cbz
 import (
 	"archive/zip"
 	"fmt"
+	"io"
 	"os"
 	"time"
 
@@ -56,7 +57,7 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) error {
 			Bool("is_splitted", page.IsSplitted).
 			Uint16("split_part", page.SplitPartIndex).
 			Str("filename", fileName).
-			Int("size", len(page.Contents.Bytes())).
+			Uint64("size", page.Size).
 			Msg("Writing page to CBZ archive")
 
 		// Create a new file in the ZIP archive
@@ -70,17 +71,30 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) error {
 			return fmt.Errorf("failed to create file in .cbz: %w", err)
 		}
 
-		// Write the page contents to the file
-		bytesWritten, err := fileWriter.Write(page.Contents.Bytes())
+		// Stream the page contents into the archive. This transparently
+		// handles pages staged on disk (see manga.Page.TempFilePath) so we
+		// never need to hold the whole chapter's contents in memory at once.
+		pageReader, err := page.Open()
+		if err != nil {
+			log.Error().Str("output_path", outputFilePath).Str("filename", fileName).Err(err).Msg("Failed to open page contents")
+			return fmt.Errorf("failed to open page contents: %w", err)
+		}
+
+		bytesWritten, err := io.Copy(fileWriter, pageReader)
+		closeErr := pageReader.Close()
 		if err != nil {
 			log.Error().Str("output_path", outputFilePath).Str("filename", fileName).Err(err).Msg("Failed to write page contents")
 			return fmt.Errorf("failed to write page contents: %w", err)
 		}
+		if closeErr != nil {
+			log.Error().Str("output_path", outputFilePath).Str("filename", fileName).Err(closeErr).Msg("Failed to close page contents reader")
+			return fmt.Errorf("failed to close page contents reader: %w", closeErr)
+		}
 
 		log.Debug().
 			Str("output_path", outputFilePath).
 			Str("filename", fileName).
-			Int("bytes_written", bytesWritten).
+			Int64("bytes_written", bytesWritten).
 			Msg("Page written successfully")
 	}
 

internal/manga/chapter.go

@@ -1,6 +1,10 @@
 package manga
 
-import "time"
+import (
+	"fmt"
+	"os"
+	"time"
+)
 
 type Chapter struct {
 	// FilePath is the path to the chapter's directory.
@@ -13,6 +17,11 @@ type Chapter struct {
 	IsConverted bool
 	// ConvertedTime is a pointer to a time.Time object that indicates when the chapter was converted. Nil mean not converted.
 	ConvertedTime time.Time
+	// TempDir, when non-empty, is a staging temp folder holding converted
+	// page contents on disk (see Page.TempFilePath) rather than fully in
+	// memory. It should be removed via Cleanup once the chapter has been
+	// written out (or is no longer needed).
+	TempDir string
 }
 
 // SetConverted sets the IsConverted field to true and sets the ConvertedTime field to the current time.
@@ -20,3 +29,16 @@ func (chapter *Chapter) SetConverted() {
 	chapter.IsConverted = true
 	chapter.ConvertedTime = time.Now()
 }
+
+// Cleanup removes the chapter's staging temp folder (if any), releasing any
+// page contents that were staged to disk during conversion.
+func (chapter *Chapter) Cleanup() error {
+	if chapter.TempDir == "" {
+		return nil
+	}
+	if err := os.RemoveAll(chapter.TempDir); err != nil {
+		return fmt.Errorf("failed to remove staging temp folder: %w", err)
+	}
+	chapter.TempDir = ""
+	return nil
+}

internal/manga/page.go

@@ -1,6 +1,13 @@
 package manga
 
-import "bytes"
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/rs/zerolog/log"
+)
 
 type Page struct {
 	// Index of the page in the chapter.
@@ -9,10 +16,71 @@ type Page struct {
 	Extension string `json:"extension" jsonschema:"description=Extension of the page image."`
 	// Size of the page in bytes
 	Size uint64 `json:"-"`
-	// Contents of the page
+	// Contents of the page. Nil when the page contents have been staged to
+	// disk (see TempFilePath) to bound memory usage.
 	Contents *bytes.Buffer `json:"-"`
+	// TempFilePath, when non-empty, points to a file on disk (in a staging
+	// temp folder) holding the page contents instead of keeping them fully
+	// in memory. Use Open() to transparently read the page contents
+	// regardless of where they are stored.
+	TempFilePath string `json:"-"`
 	// IsSplitted tell us if the page was cropped to multiple pieces
 	IsSplitted bool `json:"is_cropped" jsonschema:"description=Was this page cropped."`
 	// SplitPartIndex represent the index of the crop if the page was cropped
 	SplitPartIndex uint16 `json:"crop_part_index" jsonschema:"description=Index of the crop if the image was cropped."`
 }
+
+// Open returns a reader for the page contents, transparently handling
+// whether the contents are held in memory (Contents) or staged on disk
+// (TempFilePath). The caller is responsible for closing the returned
+// reader.
+func (page *Page) Open() (io.ReadCloser, error) {
+	if page.TempFilePath != "" {
+		file, err := os.Open(page.TempFilePath)
+		if err != nil {
+			return nil, fmt.Errorf("failed to open staged page contents: %w", err)
+		}
+		return file, nil
+	}
+	if page.Contents == nil {
+		return nil, fmt.Errorf("page has no contents: neither TempFilePath nor Contents is set")
+	}
+	return io.NopCloser(bytes.NewReader(page.Contents.Bytes())), nil
+}
+
+// Stage writes the given content to a file in tempDir instead of keeping it
+// in memory, updating Extension, Size and TempFilePath accordingly and
+// clearing Contents. This is used after converting a page so that only the
+// pages currently being processed are held in memory, bounding memory usage
+// for chapters with many/large pages.
+func (page *Page) Stage(tempDir string, content *bytes.Buffer, extension string) error {
+	file, err := os.CreateTemp(tempDir, "page-*.tmp")
+	if err != nil {
+		return fmt.Errorf("failed to create staging file: %w", err)
+	}
+	fileName := file.Name()
+
+	written, writeErr := file.Write(content.Bytes())
+	closeErr := file.Close()
+	if writeErr == nil && closeErr == nil && written == content.Len() {
+		page.TempFilePath = fileName
+		page.Extension = extension
+		page.Size = uint64(written)
+		page.Contents = nil
+		return nil
+	}
+
+	// Something went wrong: remove the incomplete/partial staging file
+	// rather than leaving corrupted data behind on disk.
+	if removeErr := os.Remove(fileName); removeErr != nil {
+		log.Warn().Str("file", fileName).Err(removeErr).Msg("Failed to remove incomplete staging file")
+	}
+
+	if writeErr != nil {
+		return fmt.Errorf("failed to write staging file: %w", writeErr)
+	}
+	if closeErr != nil {
+		return fmt.Errorf("failed to close staging file: %w", closeErr)
+	}
+	return fmt.Errorf("short write to staging file: wrote %d of %d bytes", written, content.Len())
+}

internal/utils/optimize.go

@@ -91,6 +91,15 @@ func Optimize(options *OptimizeOptions) error {
 		return fmt.Errorf("failed to convert chapter")
 	}
 
+	// Clean up the staging temp folder (if any) used to hold converted page
+	// contents on disk once we're done with the chapter, regardless of
+	// success or failure below.
+	defer func() {
+		if cleanupErr := convertedChapter.Cleanup(); cleanupErr != nil {
+			log.Warn().Str("file", chapter.FilePath).Err(cleanupErr).Msg("Failed to remove staging temp folder")
+		}
+	}()
+
 	log.Debug().
 		Str("file", chapter.FilePath).
 		Int("original_pages", len(chapter.Pages)).

internal/utils/optimize_integration_test.go

@@ -51,6 +51,13 @@ func TestOptimizeIntegration(t *testing.T) {
 		if err != nil {
 			return err
 		}
+		// Skip the "large" fixtures directory: it holds the Git LFS-tracked
+		// fixture used exclusively by TestOptimizeIntegration_LargeFile,
+		// which may only be a small LFS pointer file if the content wasn't
+		// fetched, and shouldn't be exercised by this generic test.
+		if info.IsDir() && filepath.Base(path) == "large" {
+			return filepath.SkipDir
+		}
 		if !info.IsDir() {
 			fileName := strings.ToLower(info.Name())
 			if (strings.HasSuffix(fileName, ".cbz") || strings.HasSuffix(fileName, ".cbr")) && !strings.Contains(fileName, "converted") {

internal/utils/optimize_large_file_integration_test.go

@@ -0,0 +1,121 @@
+package utils
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/belphemur/CBZOptimizer/v2/internal/utils/errs"
+	"github.com/belphemur/CBZOptimizer/v2/pkg/converter"
+	"github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant"
+	"github.com/rs/zerolog/log"
+)
+
+// largeTestFile is a ~1GB synthetic CBZ fixture stored via Git LFS (see
+// .gitattributes). It is used to exercise the optimize pipeline with a
+// chapter large enough to make in-memory-only handling impractical, and to
+// validate that converted pages are streamed to/from a staging temp folder
+// (see manga.Page.TempFilePath / manga.Chapter.TempDir) instead of blowing
+// up memory usage.
+const largeTestFile = "../../testdata/large/large_chapter.cbz"
+
+// TestOptimizeIntegration_LargeFile is opt-in (set CBZ_RUN_LARGE_FILE_TEST=1)
+// since it processes a ~1GB fixture and can take a while to run. It is
+// automatically skipped if the fixture is unavailable (e.g. Git LFS content
+// wasn't fetched, leaving only a pointer file) or in short mode.
+func TestOptimizeIntegration_LargeFile(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping large file integration test in short mode")
+	}
+	if os.Getenv("CBZ_RUN_LARGE_FILE_TEST") == "" {
+		t.Skip("Skipping large file integration test; set CBZ_RUN_LARGE_FILE_TEST=1 to run it")
+	}
+
+	info, err := os.Stat(largeTestFile)
+	if err != nil {
+		t.Skipf("large test fixture not found: %v", err)
+	}
+	// If Git LFS content wasn't fetched (e.g. `actions/checkout` without
+	// `lfs: true`), the file on disk is just a small pointer text file
+	// rather than the real ~1GB fixture. Detect and skip gracefully instead
+	// of failing the whole suite.
+	const minExpectedSize = 500 * 1024 * 1024 // 500MB
+	if info.Size() < minExpectedSize {
+		t.Skipf("large test fixture looks like a Git LFS pointer (size=%d), skipping; run `git lfs pull`", info.Size())
+	}
+
+	tempDir, err := os.MkdirTemp("", "test_optimize_large_file")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer errs.CaptureGeneric(&err, os.RemoveAll, tempDir, "failed to remove temporary directory")
+
+	converterInstance, err := converter.Get(constant.WebP)
+	if err != nil {
+		t.Skip("WebP converter not available, skipping large file integration test")
+	}
+	if err := converterInstance.PrepareConverter(); err != nil {
+		t.Skip("Failed to prepare WebP converter, skipping large file integration test")
+	}
+
+	cbzFile := filepath.Join(tempDir, "large_chapter.cbz")
+	if err := copyFile(largeTestFile, cbzFile); err != nil {
+		t.Fatal(err)
+	}
+
+	var memBefore, memAfter runtime.MemStats
+	runtime.GC()
+	runtime.ReadMemStats(&memBefore)
+
+	options := &OptimizeOptions{
+		ChapterConverter: converterInstance,
+		Path:             cbzFile,
+		Quality:          85,
+		Override:         false,
+		Split:            true,
+	}
+
+	err = Optimize(options)
+	if err != nil {
+		t.Fatalf("failed to optimize large chapter: %v", err)
+	}
+
+	runtime.GC()
+	runtime.ReadMemStats(&memAfter)
+	log.Info().
+		Uint64("heap_alloc_before", memBefore.HeapAlloc).
+		Uint64("heap_alloc_after", memAfter.HeapAlloc).
+		Int64("input_size", info.Size()).
+		Msg("Large file integration test memory usage")
+
+	outputFile := strings.TrimSuffix(cbzFile, ".cbz") + "_converted.cbz"
+	if _, err := os.Stat(outputFile); err != nil {
+		t.Fatalf("expected converted output file %s to exist: %v", outputFile, err)
+	}
+}
+
+// copyFile copies src to dst using streaming file I/O so that the whole
+// file content is never held in memory at once, which matters for the
+// large fixture used by this test.
+func copyFile(src, dst string) (err error) {
+	in, err := os.Open(src)
+	if err != nil {
+		return fmt.Errorf("failed to open source file: %w", err)
+	}
+	defer errs.Capture(&err, in.Close, "failed to close source file")
+
+	out, err := os.Create(dst)
+	if err != nil {
+		return fmt.Errorf("failed to create destination file: %w", err)
+	}
+	defer errs.Capture(&err, out.Close, "failed to close destination file")
+
+	if _, err := io.Copy(out, in); err != nil {
+		return fmt.Errorf("failed to copy file contents: %w", err)
+	}
+	return nil
+}