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

perf: reduce memory usage and improve speed during dev, by caching payload config & sanitization #9501

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

AlessioGr
Copy link
Member

@AlessioGr AlessioGr commented Nov 25, 2024

Previously, during dev, the payload config would be loaded and sanitized between every single page transition twice. This lead to higher memory usage during dev. This PR makes sure it only happens once when payload is started.

This PR does not change the behavior in prod, as it was already cached nicely there. It will only affect dev

How to use

If you don't change anything, everything will still work and you will get no performance improvements. => Backwards-compatible and non-breaking.

To benefit from improved performance and lower memory usage, simply wrap your payload config argument in a function:

- buildConfig({
+ buildConfig(() => ({
   // Config here
- })
+ }))

Todo

Get HMR to work

AlessioGr and others added 25 commits November 22, 2024 13:34
…emoving deep copying and building client config from the ground up instead
…ly runs once, and does not re-run when refreshing the page or navigating. HMR still works despite caching in dev
… comes back, we should run it in the background
…nd clientSchemaMap, introduce clientSchemaMap
…ayload and cache it. This now only happens once
…xists, if already-queried document is published
…t editor config is properly cached and only sanitized once for all lexical fields, instead of once per lexical field, as previously this was not cached until the sanitization function finished
Comment on lines 10 to 36
export async function buildConfig(
_config: (() => Config) | Config,
): Promise<(() => Promise<SanitizedConfig>) | SanitizedConfig> {
let configFn: (() => Config) | null = null

if (typeof _config !== 'function') {
console.warn(
'For optimal performance, buildConfig should be called with a function that returns a config object. Otherwise, you will notice increased memory usage and decreased performance during development.',
)
configFn = () => _config
// We could still return a function that returns the sanitized config here,
// so that it's cached properly and not loaded multiple times after every page transition.
// However, in order for this to be backwards compatible, we return the sanitized config directly, the old way.
// Otherwise, the imported config would suddenly be a function when imported, which may break standalone scripts
return await loadAndSanitizeConfig(configFn)
} else {
configFn = _config
}

if (process.env.NODE_ENV === 'production') {
return await loadAndSanitizeConfig(configFn)
} else {
return async () => {
return await loadAndSanitizeConfig(configFn)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't the performance benefits be made automatic without changing the API by using a cache?
I'm thinking something like this:

export async function buildConfig(
  _config: Config,
): Promise<SanitizedConfig> {
  // Create a wrapper function that maintains a single instance in dev
  let cachedConfig: SanitizedConfig | null = null
  let cachedConfigFn: (() => Promise<SanitizedConfig>) | null = null

  const getConfigFn = () => async (): Promise<SanitizedConfig> => {
    // In development, cache the sanitized config
    if (process.env.NODE_ENV === 'development') {
      if (!cachedConfig) {
        cachedConfig = await loadAndSanitizeConfig(() => _config)
      }
      return cachedConfig
    }
    // In production, just process normally since it's already optimized
    return await loadAndSanitizeConfig(() => _config)
  }

  // In development, return a function that returns the cached result
  if (process.env.NODE_ENV === 'development') {
    if (!cachedConfigFn) {
      cachedConfigFn = getConfigFn()
    }
    return await cachedConfigFn()
  }

  // In production, just process normally
  return await loadAndSanitizeConfig(() => _config)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

If we cache it on the module scope then might work yea. But if the config is not passed through as a function returning the config, code in there will still execute despite the config being cached.

E.g. say your config does this:

CleanShot 2024-11-25 at 08 35 26@2x

then yes, the config would only be sanitized once - however, even if the config is cached, it would still execute the lexicalEditor and feature functions + spread defaultEditorFeatures every single time.

Base automatically changed from perf/clientconfig to main November 26, 2024 21:31
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.

3 participants