Improve image optimization run-time and memory usage #457

Merged
a258wang merged 8 commits from amy-image-optimization-quick-fix into main 10 months ago
Owner

This PR aims to resolve the issues surrounding the optimize-images script, at least in the short term, in order to unblock other work.

Problems: Our optimize-images script was taking a very long time to run (~7 minutes in CI, when successful). This led to two problems:

  1. It was near impossible to run the script locally.
  2. CI jobs would often get killed on the optimize-images step.

Solutions:

  1. Resize the images in images/events prior to optimizing them, similar to what is done for the images in images/team. This solution on its own reduced the run-time of the script to ~30 seconds locally on Amy's laptop, which is comparable to the run-time of the script back when it was originally written.
  2. EDIT: Copy/resize/optimize the images in batches of 32 at a time. The reason why the CI job was being killed is because the script would run out of memory, however this change should resolve that while also keeping build times reasonable (~30 sec locally/when deploying on caffeine, ~3 min in CI). As a temporary fix, this PR also replaces the images in images/events with their resized + optimized versions. (For some unknown reason, Solution 1 is not sufficient to solve Problem 2.)

This PR also adds some logging to the script so we can get a (slightly) better sense of where the script is getting stuck.

Related issue: #456

This PR aims to resolve the issues surrounding the optimize-images script, at least in the short term, in order to unblock other work. **Problems:** Our optimize-images script was taking a very long time to run (~7 minutes in CI, when successful). This led to two problems: 1. It was near impossible to run the script locally. 2. CI jobs would often get killed on the optimize-images step. **Solutions:** 1. Resize the images in `images/events` prior to optimizing them, similar to what is done for the images in `images/team`. This solution on its own reduced the run-time of the script to ~30 seconds locally on Amy's laptop, which is comparable to the run-time of the script back when it was originally written. 2. EDIT: Copy/resize/optimize the images in batches of 32 at a time. The reason why the CI job was being killed is because the script would run out of memory, however this change should resolve that while also keeping build times reasonable (~30 sec locally/when deploying on caffeine, ~3 min in CI). ~~As a temporary fix, this PR also replaces the images in `images/events` with their resized + optimized versions. (For some unknown reason, Solution 1 is not sufficient to solve Problem 2.)~~ This PR also adds some logging to the script so we can get a (slightly) better sense of where the script is getting stuck. Related issue: #456
a258wang added 1 commit 10 months ago
19c5aecb07 Resize events images and add logging
a258wang requested review from snedadah 10 months ago
a258wang added 1 commit 10 months ago
2a62f0b508 Add resize logging
a258wang removed review request for snedadah 10 months ago
a258wang added 1 commit 10 months ago
6a3b151ed2 Replace events images with optimized images
a258wang added 2 commits 10 months ago
49c272efa3 Fix brackets
a258wang changed title from Improve image optimization run-time to WIP: Improve image optimization run-time (quick fix) 10 months ago
a258wang added 1 commit 10 months ago
84eba39f7a Revert "Revert "Replace events images with optimized images""
a258wang changed title from WIP: Improve image optimization run-time (quick fix) to Improve image optimization run-time (quick fix) 10 months ago
a258wang requested review from snedadah 10 months ago
a258wang changed title from Improve image optimization run-time (quick fix) to Improve image optimization run-time - quick fix 10 months ago
a3thakra approved these changes 10 months ago
snedadah approved these changes 10 months ago
snedadah left a comment
Owner

Nice catch!! Makes sense why those specific images were taking so long now… this solution is great!

I think this solution is good enough to solve our problems for now. In the future, we should consider an alternative image storage mechanism anyway...

Nice catch!! Makes sense why those specific images were taking so long now… this solution is great! I think this solution is good enough to solve our problems for now. In the future, we should consider an alternative image storage mechanism anyway...
void optimizeImages();
export async function optimizeImages() {
const startTime = Date.now();
Owner

By the way, JavaScript has a built in console.time()

https://developer.mozilla.org/en-US/docs/Web/API/console/time

By the way, JavaScript has a built in `console.time()` https://developer.mozilla.org/en-US/docs/Web/API/console/time
a258wang added 2 commits 10 months ago
a258wang changed title from Improve image optimization run-time - quick fix to Improve image optimization run-time 10 months ago
a258wang changed title from Improve image optimization run-time to Improve image optimization run-time and memory usage 10 months ago
a258wang requested review from a3thakra 10 months ago
a258wang requested review from snedadah 10 months ago
Poster
Owner

Re-requested reviews because I made a pretty significant change - see PR description for update

Re-requested reviews because I made a pretty significant change - see PR description for update
snedadah approved these changes 10 months ago
snedadah left a comment
Owner

"Revert "Revert "Revert :)

`"Revert "Revert "Revert` :)
a258wang merged commit 443925190e into main 10 months ago
a258wang deleted branch amy-image-optimization-quick-fix 10 months ago

Reviewers

a3thakra was requested for review 10 months ago
snedadah approved these changes 10 months ago
continuous-integration/drone/push Build is passing
The pull request has been merged as 443925190e.
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#457
Loading…
There is no content yet.