Optimize memory usage for large PDFs via chunked processing #10

2025-11-22T19:13:32-05:00

GionnyBearThaRealHe commented

2025-11-22 19:13:32 -05:00

(Migrated from github.com)

Key Changes:

Chunked Processing: Pages are now processed in batches of 50. Each batch is compressed and stored temporarily, preventing the browser from holding thousands of raw canvas elements in memory simultaneously.
Memory Management: Added explicit cleanup for canvas contexts and PDF page resources (page.cleanup()) after processing each page.
Optimized Preview: The DOM preview is now limited to the first 20 pages to keep the UI responsive, while still converting the entire document in the background.
Garbage Collection: Canvas buffers are explicitly cleared for pages not shown in the preview to assist the browser's garbage collector.
Results:

Significantly reduced RAM consumption (preventing the
~15GB spikes previously observed).

Eliminated browser crashes on large files.
Maintained high-quality output (scale 3) without sacrificing resolution for performance.

Key Changes: Chunked Processing: Pages are now processed in batches of 50. Each batch is compressed and stored temporarily, preventing the browser from holding thousands of raw canvas elements in memory simultaneously. Memory Management: Added explicit cleanup for canvas contexts and PDF page resources (page.cleanup()) after processing each page. Optimized Preview: The DOM preview is now limited to the first 20 pages to keep the UI responsive, while still converting the entire document in the background. Garbage Collection: Canvas buffers are explicitly cleared for pages not shown in the preview to assist the browser's garbage collector. Results: Significantly reduced RAM consumption (preventing the ~15GB spikes previously observed). Eliminated browser crashes on large files. Maintained high-quality output (scale 3) without sacrificing resolution for performance.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-11-23 02:05:17 -05:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR optimizes memory usage for large PDF conversions by implementing chunked processing to prevent browser crashes from excessive RAM consumption (previously reaching ~15GB spikes).

Key Changes:

Implemented chunked processing with batches of 50 pages, creating temporary PDF documents per chunk
Limited DOM preview to first 20 pages while processing entire document in background
Added explicit memory cleanup with canvas buffer clearing and PDF page resource cleanup (page.cleanup())

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

## Pull request overview This PR optimizes memory usage for large PDF conversions by implementing chunked processing to prevent browser crashes from excessive RAM consumption (previously reaching ~15GB spikes). **Key Changes:** - Implemented chunked processing with batches of 50 pages, creating temporary PDF documents per chunk - Limited DOM preview to first 20 pages while processing entire document in background - Added explicit memory cleanup with canvas buffer clearing and PDF page resource cleanup (`page.cleanup()`) --- 💡 <a href="/chizkiyahu/pdf-dark-mode-converter/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

index.html

					
				@ -258,2 +258,3 @@

				            const pdfDoc = await PDFLib.PDFDocument.create();

				            // Initialize PDF document later or in chunks

				            const pdf = await pdfjsLib.getDocument({ data: pdfData }).promise;

The comment says "Initialize PDF document later or in chunks" but this line of code was removed. The comment is now orphaned and should be removed or rewritten to explain why the PDF document initialization was deferred.

The comment says "Initialize PDF document later or in chunks" but this line of code was removed. The comment is now orphaned and should be removed or rewritten to explain why the PDF document initialization was deferred. ```suggestion ```

index.html

					
				@ -293,0 +302,4 @@

				                    await page.render(renderContext).promise;

				                    if (renderId !== currentRenderId) return;

[nitpick] The CHUNK_SIZE magic number (50) should be defined as a constant at the top of the file or function scope for better maintainability. Consider adding a comment explaining why 50 was chosen as the optimal chunk size.

[nitpick] The `CHUNK_SIZE` magic number (50) should be defined as a constant at the top of the file or function scope for better maintainability. Consider adding a comment explaining why 50 was chosen as the optimal chunk size.

index.html

					
				@ -351,1 +396,4 @@

				            if (renderId !== currentRenderId) return;

				            progressContainer.style.display = 'none';

[nitpick] The preview page limit (20) is a magic number that should be defined as a named constant (e.g., MAX_PREVIEW_PAGES) for better maintainability. This value is used in two places (lines 334 and 358) and having it as a constant would make it easier to adjust and maintain.

[nitpick] The preview page limit (20) is a magic number that should be defined as a named constant (e.g., `MAX_PREVIEW_PAGES`) for better maintainability. This value is used in two places (lines 334 and 358) and having it as a constant would make it easier to adjust and maintain.

index.html

Missing error handling for the canvas.toBlob() promise. If the blob conversion fails or the blob is null, blob.arrayBuffer() will throw an error. Consider adding explicit error handling:

const imgBytes = await new Promise((resolve, reject) => {
    canvas.toBlob(blob => {
        if (blob) {
            blob.arrayBuffer().then(resolve).catch(reject);
        } else {
            reject(new Error('Failed to convert canvas to blob'));
        }
    }, 'image/png');
});

                    const imgBytes = await new Promise((resolve, reject) => {
                        canvas.toBlob(blob => {
                            if (blob) {
                                blob.arrayBuffer().then(resolve).catch(reject);
                            } else {
                                reject(new Error('Failed to convert canvas to blob'));
                            }
                        }, 'image/png');
                    });

Missing error handling for the `canvas.toBlob()` promise. If the blob conversion fails or the blob is null, `blob.arrayBuffer()` will throw an error. Consider adding explicit error handling: ```javascript const imgBytes = await new Promise((resolve, reject) => { canvas.toBlob(blob => { if (blob) { blob.arrayBuffer().then(resolve).catch(reject); } else { reject(new Error('Failed to convert canvas to blob')); } }, 'image/png'); }); ``` ```suggestion const imgBytes = await new Promise((resolve, reject) => { canvas.toBlob(blob => { if (blob) { blob.arrayBuffer().then(resolve).catch(reject); } else { reject(new Error('Failed to convert canvas to blob')); } }, 'image/png'); }); ```

index.html

[nitpick] The delay of 10ms may not be sufficient for garbage collection to occur, and setTimeout doesn't guarantee GC will run. Modern browsers typically perform GC automatically based on memory pressure. Consider removing this artificial delay or increasing it if UI responsiveness is truly impacted. The comment should also clarify that this is primarily for UI updates, not GC.

                // Optional: small delay to allow UI updates (not for garbage collection).
                await new Promise(resolve => setTimeout(resolve, 50)); // Increase to 50ms if UI is lagging; remove if unnecessary.

[nitpick] The delay of 10ms may not be sufficient for garbage collection to occur, and `setTimeout` doesn't guarantee GC will run. Modern browsers typically perform GC automatically based on memory pressure. Consider removing this artificial delay or increasing it if UI responsiveness is truly impacted. The comment should also clarify that this is primarily for UI updates, not GC. ```suggestion // Optional: small delay to allow UI updates (not for garbage collection). await new Promise(resolve => setTimeout(resolve, 50)); // Increase to 50ms if UI is lagging; remove if unnecessary. ```

index.html

[nitpick] Setting chunks[i] = null may not effectively release memory. The chunk bytes are already loaded into chunkDoc and the array still holds the reference until the loop completes. Consider using chunks.length = 0 or chunks = [] after the loop to fully release the array, or avoid storing chunks altogether by merging them immediately after creation.

            }
            // Release all chunk references
            chunks.length = 0;

[nitpick] Setting `chunks[i] = null` may not effectively release memory. The chunk bytes are already loaded into `chunkDoc` and the array still holds the reference until the loop completes. Consider using `chunks.length = 0` or `chunks = []` after the loop to fully release the array, or avoid storing chunks altogether by merging them immediately after creation. ```suggestion } // Release all chunk references chunks.length = 0; ```

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

chizkiyahu/pdf-dark-mode-converter!10

Rows
Columns