Skip to content

Fix inlineEnvironment perf#9014

Merged
mattcompiles merged 3 commits into
v2from
fix-inline-environment-perf
May 17, 2023
Merged

Fix inlineEnvironment perf#9014
mattcompiles merged 3 commits into
v2from
fix-inline-environment-perf

Conversation

@mattcompiles

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

3 participants