Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallelize @astrojs/image transforms #4626

Merged
merged 1 commit into from
Sep 6, 2022
Merged

Parallelize @astrojs/image transforms #4626

merged 1 commit into from
Sep 6, 2022

Conversation

altano
Copy link
Contributor

@altano altano commented Sep 5, 2022

Changes

This change parallelizes @astrojs/image transforms to get a big performance boost.

@astrojs/image currently processes images serially, one after the other. If it processes 10 images and takes 1 second to process each, the total time will be 10 seconds.

This change makes it process images in batches. The batch size is the # of CPUs on the machine, e.g. 8 on my dev machine.

Testing

I manually put the newly built code in dist/build/ssg.js into my current project (I can't figure out how to get pnpm and npm to let me link across projects). I then did a before and after build.

I then edited astro.config.mts and enabled debug logging:

integrations: [
    image({
      logLevel: "debug",
    }),
...

BEFORE - 92 second build

image

AFTER - 39 second build

image

NOTE: Yes I know a 92s->39s build time is still abysmal. @astrojs/image is very slow at processing animated gifs. I will investigate that separately.

Docs

No docs needed as this is a performance improvement

@changeset-bot
Copy link

changeset-bot bot commented Sep 5, 2022

🦋 Changeset detected

Latest commit: 8156382

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/image Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Sep 5, 2022
@@ -40,6 +40,7 @@
"test": "mocha --exit --timeout 20000 test"
},
"dependencies": {
"@altano/tiny-async-pool": "^1.0.2",
Copy link
Contributor Author

@altano altano Sep 5, 2022

Choose a reason for hiding this comment

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

In case anyone is hesitant to take on @altano/tiny-async-pool, a new dependency:

  • The used export is super tiny: 315B gzipped
  • The package is a fork of async-pool by Rafael Xavier de Souza. I left it MIT licensed and barely modified the code other than to modernize it (typescript, esm export, vitest, etc). The original package gets ~900K weekly downloads so it's mature code and should be fairly bug free.

@altano
Copy link
Contributor Author

altano commented Sep 5, 2022

This seems to be trivially faster for small batches of well-behaved transforms as well, not just the pathological case of misbehaved animated gif transforms:

BEFORE (158ms) AFTER (85ms)
image image

Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

Great work! Want to leave it to @tony-sull to take a look at, but it lgtm.

Copy link
Contributor

@tony-sull tony-sull left a comment

Choose a reason for hiding this comment

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

Very nice!

Just ran it locally against the astro.build site and was impressed to see that sharp didn't blow up Node's memory usage when processing 8 images in parallel 🚀

@tony-sull tony-sull merged commit 494c2b8 into withastro:main Sep 6, 2022
@astrobot-houston astrobot-houston mentioned this pull request Sep 6, 2022
@tony-sull tony-sull self-assigned this Sep 7, 2022
@altano altano deleted the alan/improve-image-processing-perf branch September 8, 2022 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants