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.

Sign up for GitHub

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

Jump to bottom

perf(gatsby-plugin-sharp): change approach to concurrency for image processing #28575

Merged
merged 2 commits into from
Jan 5, 2021
Merged

perf(gatsby-plugin-sharp): change approach to concurrency for image processing #28575

merged 2 commits into from
Jan 5, 2021

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Dec 10, 2020 •
edited
Loading

Description

My tests for lazy images (#27603) showed that increasing sharp queue concurrency results in a better overall image transformation performance. Tested on two image-heavy sites.

First one (based on this starter):

Before this PR:

success Generating image thumbnails - 311.836s - 532/532 1.71/s
info Done building in 317.3029042 sec

After this PR:

success Generating image thumbnails - 174.163s - 532/532 3.05/s
info Done building in 179.6475083 sec

Second site:

Before this PR:

Generating image thumbnails - 436.176s

After this PR:

Generating image thumbnails - 259.491s

Note that for this site the build has failed for me with unrelated SSR error - for both checks. Just don't have time to figure it out now - my fork is pretty outdated. But image processing was significantly faster this way.

I also see similar good effect on lazy images - images load about 30-40% faster.

The best explanation I have is that the number of transformations for a single image is smaller than the number of cores and so sharp underutilizes the CPU.

CC @pvdz @wardpeet

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 10, 2020
@vladar vladar added topic: sharp and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 10, 2020
@vladar vladar marked this pull request as draft December 10, 2020 14:09
@wardpeet wardpeet mentioned this pull request Dec 11, 2020
Closed
@vladar vladar marked this pull request as ready for review January 5, 2021 11:46
pvdz
pvdz approved these changes Jan 5, 2021
View reviewed changes
Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

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

Massive win, much hype!

@vladar vladar added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jan 5, 2021
@gatsbybot gatsbybot merged commit 930e6b6 into master Jan 5, 2021
@gatsbybot gatsbybot deleted the vladar/sharp-perf branch January 5, 2021 13:44
vladar added a commit that referenced this pull request Jan 5, 2021
@vladar
perf(gatsby-plugin-sharp): change approach to concurrency for image p…
381ed0e 
…rocessing (#28575)

Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
(cherry picked from commit 930e6b6)
@gatsbybot gatsbybot mentioned this pull request Jan 5, 2021
Merged
gatsbybot pushed a commit that referenced this pull request Jan 5, 2021
@vladar
perf(gatsby-plugin-sharp): change approach to concurrency for image p…
afac774 
…rocessing (#28575) (#28862)

Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
(cherry picked from commit 930e6b6)

Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com>
@vladar vladar mentioned this pull request Mar 31, 2021
Merged
@saurabharch saurabharch mentioned this pull request Nov 4, 2022
Open
@nidhi42 nidhi42 mentioned this pull request Nov 4, 2022
Open
@ajesse11x ajesse11x mentioned this pull request Nov 4, 2022
Open
@nidhi42 nidhi42 mentioned this pull request Nov 5, 2022
Open
This was referenced Nov 5, 2022
Open
Open
@nidhi42 nidhi42 mentioned this pull request Nov 5, 2022
Open
@kaocher82 kaocher82 mentioned this pull request Nov 5, 2022
Open
This was referenced Nov 5, 2022
Open
Open
@ajesse11x ajesse11x mentioned this pull request Nov 5, 2022
Open
@kaocher82 kaocher82 mentioned this pull request Nov 5, 2022
Open
@0xSebin 0xSebin mentioned this pull request Nov 5, 2022
Open
@ajesse11x ajesse11x mentioned this pull request Nov 5, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Nov 6, 2022
Open
@saurabharch saurabharch mentioned this pull request Dec 25, 2022
Open
@nidhi42 nidhi42 mentioned this pull request Dec 25, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Dec 25, 2022
Open
@ajesse11x ajesse11x mentioned this pull request Dec 25, 2022
Open
@kaocher82 kaocher82 mentioned this pull request Dec 25, 2022
Open
@0xSebin 0xSebin mentioned this pull request Dec 25, 2022
Open
@ajesse11x ajesse11x mentioned this pull request Dec 25, 2022
Open
@nidhi42 nidhi42 mentioned this pull request Dec 25, 2022
Open
This was referenced Dec 25, 2022
Open
Open
@nidhi42 nidhi42 mentioned this pull request Dec 25, 2022
Open
@kaocher82 kaocher82 mentioned this pull request Dec 26, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Dec 26, 2022
Open
@0xSebin 0xSebin mentioned this pull request Dec 26, 2022
Open
@kaocher82 kaocher82 mentioned this pull request Dec 27, 2022
Open
@nidhi42 nidhi42 mentioned this pull request Jun 21, 2023
Open
@ajesse11x ajesse11x mentioned this pull request Jun 21, 2023
Open
@saurabharch saurabharch mentioned this pull request Jun 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvdz pvdz pvdz approved these changes

Assignees
No one assigned
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vladar @pvdz @gatsbybot