Skip to content

feat(rate-limiter): improve performance#2676

Merged
EmrysMyrddin merged 5 commits intomainfrom
feat/rate-limiter/improve-perf
Sep 15, 2025
Merged

feat(rate-limiter): improve performance#2676
EmrysMyrddin merged 5 commits intomainfrom
feat/rate-limiter/improve-perf

Conversation

@EmrysMyrddin
Copy link
Contributor

@EmrysMyrddin EmrysMyrddin commented Sep 11, 2025

Description

To apply the configuration based on type and field name, the rate limiter plugin is using minimatch to allow glob patterns and simplify the configuration.

But minimatch implementation is very unefficient. It uses RegExp under the hood (which is already pretty expensive), but it doesn't even memoize the RegExp instances. This means that a new one is created and compiled for each resolver, for each configured type/field.

Having such heavy process in a hot path like this one can have a big impact of performance.

This PR replaces minimatch by picomatch and builds all relevant matchers ahead of time. This way, only the actual string test is done in the hot path.

It also moves all the logic for replacing the resolver based on configuration to the onSchemaChange hook, to minimize the overhead at execution time.

Realted to graphql-hive/gateway#1485

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Further comments

By reworking the plugin, I also came across some small issues:

  • The directiveName option was not honored.
  • Directives on fields with a matching configuration was silently ignored
  • Configurations matching the same field was silently ignored (only the first matching config was used)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2025

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@envelop/parser-cache 9.0.1-alpha-20250912150608-74bc2fbef267d299dfb47a06509f8d305c4b3160 npm ↗︎ unpkg ↗︎
@envelop/rate-limiter 8.1.0-alpha-20250912150608-74bc2fbef267d299dfb47a06509f8d305c4b3160 npm ↗︎ unpkg ↗︎
@envelop/response-cache 8.0.2-alpha-20250912150608-74bc2fbef267d299dfb47a06509f8d305c4b3160 npm ↗︎ unpkg ↗︎
@envelop/validation-cache 9.0.1-alpha-20250912150608-74bc2fbef267d299dfb47a06509f8d305c4b3160 npm ↗︎ unpkg ↗︎

@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2025

💻 Website Preview

The latest changes are available as preview in: https://784b91a1.envelop.pages.dev

@EmrysMyrddin EmrysMyrddin force-pushed the feat/rate-limiter/improve-perf branch from 89bdd49 to 74bc2fb Compare September 12, 2025 15:05
Copy link
Member

@ardatan ardatan left a comment

Choose a reason for hiding this comment

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

You can also use mapSchema but this is also good

@EmrysMyrddin
Copy link
Contributor Author

Yes, that was my first thought, and then I found out that useOnResolve is not using mapSchema but mutating it in place, so I've done the same :-)

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants