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

Fix inlineEnvironment perf #9014

Merged
merged 3 commits into from
May 17, 2023
Merged

Fix inlineEnvironment perf #9014

merged 3 commits into from
May 17, 2023

Conversation

mattcompiles
Copy link
Contributor

↪️ Pull Request

Currently, when the inlineEnvironment config in the JS transformer is used with an array value, it does a glob match against every environment variable in the process. This leads to a large drop in performance as it is performed for every JS file in the build.

micromatch, which is used for our glob matching, actually supports many to many matching which is much more performant for this type of case. This PR exposes a new glob API (globMatch) which is now used in the JS transformer.

Just change took a local build completion time from ~140s to ~70s.

While this is good, I do wonder if there's a way to optimize this further as this work should only be done once per build? It is currently done once per file.

🚨 Test instructions

No new tests as functionality hasn't changed.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@mischnic
Copy link
Member

Some alternative ideas (the last two assuming that the proportion of assets using env variables is tiny):

  • As you've said, something like loadConfig that runs once per build for projectRoot-level settings.
  • Don't pass everything to Rust but instead use a JS callback to get used variables on demand
  • Pass all env vars (as in the else case) and config?.inlineEnvironment to Rust, and make the glob matching there (also on demand)

@mattcompiles
Copy link
Contributor Author

As discussed, I'm merging this as is and will follow up with global config loading for transformers.

@mattcompiles mattcompiles merged commit 585ea26 into v2 May 17, 2023
@mattcompiles mattcompiles deleted the fix-inline-environment-perf branch May 17, 2023 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants