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

[FEATURE] add memory cache to @salesforce/pwa-kit-runtime/utils/ssr-config getConfig function #1621

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

luca-izzo
Copy link

@luca-izzo luca-izzo commented Dec 15, 2023

Description

getConfig function inside @salesforce/pwa-kit-runtime/utils/ssr-config/ takes relatively long time to be executed on SSR and when called thousand of times can have a huge impact on the performances.

Steps to reproduce

On both remote and local environments:
Call getConfig function inside @salesforce/pwa-kit-runtime/utils/ssr-config/ a lot of times in your application and observe the impact on the rendering time.

Types of Changes

Added a Map to cache in memory the result of the function based on the provided buildDirectory.

Closes #1620

@luca-izzo luca-izzo requested a review from a team as a code owner December 15, 2023 11:30
Copy link

Thanks for the contribution! Before we can merge this, we need @luca-izzo to sign the Salesforce Inc. Contributor License Agreement.

@luca-izzo luca-izzo changed the title [FEATURE] add memory cache to @salesforce/pwa-kit-runtime/utils/ssr-config getConfig function #1620 [FEATURE] add memory cache to @salesforce/pwa-kit-runtime/utils/ssr-config getConfig function #1620 Dec 15, 2023
@luca-izzo luca-izzo changed the title [FEATURE] add memory cache to @salesforce/pwa-kit-runtime/utils/ssr-config getConfig function #1620 [FEATURE] add memory cache to @salesforce/pwa-kit-runtime/utils/ssr-config getConfig function Dec 15, 2023
@bfeister
Copy link
Collaborator

bfeister commented Dec 15, 2023

@luca-izzo thanks for the contribution! Since you've already done the work of investigating the problem, and I'm sure you've compiled data and metrics that demonstrates the increasing latency with successive calls, I would like you to post those here as part of the PR description. Whenever I think about pull requests, I always think "Will it make sense for me looking back at this PR as a reference in 12 months from now when I've forgotten the discussion?"

The data in your findings (screenshots that show an increasingly slow response time in the dev server in terminal or "network" tab in Chrome are fine) will be important to help us validate both the problem and the solution

@luca-izzo
Copy link
Author

luca-izzo commented Dec 16, 2023

Hello @bfeister,
I've measured one of our HPs where we call this function 694 times.

Sum of execution time of each call to getConfig in ms:
Local before: -750ms
Local after: ~1ms

Remote before: -550ms
Remote after: ~1ms

Chrome network tab:
Local before:
Screenshot 2023-12-16 at 14 39 50

Local after:
Screenshot 2023-12-16 at 14 38 14

Remote before:
Screenshot 2023-12-16 at 17 56 23

Remote After:
Screenshot 2023-12-16 at 17 52 09

@luca-izzo
Copy link
Author

@bfeister ,

Note:
Another breaking change for us was to remove all "URL" objects.
We removed it from all helpers functions together with caching getConfig output and we've seen a drastic performance improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] add memory cache to @salesforce/pwa-kit-runtime/utils/ssr-config getConfig function
2 participants