Skip to content

Conversation

@benminer
Copy link
Contributor

@benminer benminer commented Apr 18, 2025

Performance Optimization: Map-based Caches for Metadata Class Building

Problem

Our schema generation process was becoming exponentially slower as our schema grew, taking over a minute to complete in some cases. With our large schema containing ClassMetadata arrays ranging from 3,000 to 15,000 objects, the repeated .find() and .filter() operations within forEach loops were creating significant performance bottlenecks.

Solution

Implemented map-based "caches" during the metadata class build process to replace inefficient array operations. These caches are added as new variables on the MetadataStorage class, while preserving list-based attributes where required by existing components.

Impact

This optimization has dramatically improved performance as demonstrated by the following benchmarks:
before (using O(n) lookups)

buildClassMetadata: 31.353s
buildFieldResolverMetadata: 313.714ms
buildResolversMetadata: 1.429s

after (using O(1) lookups)

buildClassMetadata: 268.772ms
buildFieldResolverMetadata: 38.593ms
buildResolversMetadata: 126.305ms

Implementation Notes

  • Maintained backward compatibility with components expecting list attributes
  • Primary focus was eliminating costly .find() and .filter() operations within loop contexts
  • Maps provide O(1) lookup time compared to O(n) for array operations

Open with Devin

@benminer benminer requested a review from MichalLytek as a code owner April 18, 2025 16:44
@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community labels Apr 19, 2025
@benminer
Copy link
Contributor Author

benminer commented Apr 22, 2025

@MichalLytek Thanks for taking a look at this, tests should be fixed. The root issue was a lack of resetting the new cache state in the global metadata-storage instance, and I also ran into some weirdness of needing to reset the state in beforeEach blocks rather than beforeAll - state was bleeding into other tests leading to failures, but would pass when run in isolation.

@benminer
Copy link
Contributor Author

benminer commented May 6, 2025

@MichalLytek Bumping this, would love to get it merged 😄

@MichalLytek
Copy link
Owner

I promise I will try to find some time to review it 😉

@codecov
Copy link

codecov bot commented May 7, 2025

Codecov Report

❌ Patch coverage is 99.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.59%. Comparing base (be33fd4) to head (220f60f).
⚠️ Report is 93 commits behind head on master.

Files with missing lines Patch % Lines
src/metadata/metadata-storage.ts 99.24% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1779      +/-   ##
==========================================
+ Coverage   95.50%   95.59%   +0.09%     
==========================================
  Files         113      114       +1     
  Lines        1847     1931      +84     
  Branches      364      397      +33     
==========================================
+ Hits         1764     1846      +82     
- Misses         83       85       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@benminer
Copy link
Contributor Author

Hey @MichalLytek, bumping this again! Let me know if you get a chance to take a look. Thanks in advance.

@benminer
Copy link
Contributor Author

benminer commented Sep 4, 2025

@MichalLytek Bumping this once more :)

@benminer
Copy link
Contributor Author

benminer commented Sep 29, 2025

For anyone keeping up with this PR, I've published my fork to NPM under @scope3/type-graphql. More detailed instructions in the top of the README. Please open up any issues on the fork that you may come across!

https://www.npmjs.com/package/@scope3/type-graphql

@MichalLytek
Copy link
Owner

@benminer
I've rebased the branch on top of latest master and made a few adjustments.
I need to push it back here to GitHub, so please make sure to allow edits from maintainers 🙏

The rest looks good, we can merge this PR then 🎉

@MichalLytek
Copy link
Owner

@benminer bumping this! 🙌

@benminer
Copy link
Contributor Author

benminer commented Jan 5, 2026

@benminer bumping this! 🙌

Hey @MichalLytek - great to hear from you! Apologies for the delay, was on holiday break. Doing this now!

@benminer
Copy link
Contributor Author

benminer commented Jan 5, 2026

@benminer I've rebased the branch on top of latest master and made a few adjustments. I need to push it back here to GitHub, so please make sure to allow edits from maintainers 🙏

The rest looks good, we can merge this PR then 🎉

Neither me or owners of our org are seeing the optional presented in the documentation here.
Screenshot 2026-01-05 at 2 11 38 PM

Not sure if deprecated, or what. Another option is giving you write access to my forked repository, where you could push changes to the master branch.

@benminer
Copy link
Contributor Author

benminer commented Jan 5, 2026

@MichalLytek I've invited you as a collaborator with write access to the forked repo. Keep in mind, the default branch there is main where I made additional changes to support internal publishing of the forked library. This is also the protected branch. The core optimization changes are still isolated in the master branch, which you should be able to push to.

Copy link
Owner

@MichalLytek MichalLytek left a comment

Choose a reason for hiding this comment

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

LGTM!

@MichalLytek MichalLytek merged commit 58d187e into MichalLytek:master Jan 13, 2026
9 checks passed
@MichalLytek MichalLytek deleted the master branch January 13, 2026 08:17
@MichalLytek
Copy link
Owner

@benminer Thank you very much for your contribution! ❤️

And sorry it took so long to merge this 😞

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

Comment on lines +112 to +126
private static objectTypesInfoMap = new Map<Function, ObjectTypeInfo>();

private static inputTypesInfo: InputObjectTypeInfo[] = [];

private static inputTypesInfoMap = new Map<Function, InputObjectTypeInfo>();

private static interfaceTypesInfo: InterfaceTypeInfo[] = [];

private static interfaceTypesInfoMap = new Map<Function, InterfaceTypeInfo>();

private static enumTypesInfo: EnumTypeInfo[] = [];

private static unionTypesInfo: UnionTypeInfo[] = [];

private static unionTypesInfoMap = new Map<symbol, UnionTypeInfo>();

Choose a reason for hiding this comment

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

🟡 Static SchemaGenerator cache Maps are never cleared between schema builds, causing memory leak

The new static Maps (objectTypesInfoMap, inputTypesInfoMap, interfaceTypesInfoMap, unionTypesInfoMap) on SchemaGenerator accumulate entries across successive calls to generateFromMetadata and are never reset.

Root Cause and Impact

In generateFromMetadata (src/schema/schema-generator.ts:132-164), at the end of each build, only usedInterfaceTypes is cleared:

BuildContext.reset();
this.usedInterfaceTypes = new Set<Function>();

The corresponding arrays (objectTypesInfo, interfaceTypesInfo, etc.) are fine because they are fully replaced via reassignment in buildTypesInfo (e.g., this.objectTypesInfo = this.metadataStorage.objectTypes.map(...) at line 272). However, the new Maps are only ever written to via .set() — entries from previous builds are never removed.

For example, if Schema A is built with types [X, Y] and then Schema B is built with types [Z], the objectTypesInfoMap will contain {X → infoA_X, Y → infoA_Y, Z → infoB_Z} — stale entries for X and Y persist. This is a memory leak that grows with each generateFromMetadata call using different types, which is common in test suites and applications that dynamically rebuild schemas.

Impact: Memory leak in long-running processes or test suites that build multiple schemas. While stale entries are unlikely to cause incorrect lookups (current-build thunks reference current-build targets), the unbounded growth of these Maps is a resource management issue.

Prompt for agents
In src/schema/schema-generator.ts, the static Maps objectTypesInfoMap, inputTypesInfoMap, interfaceTypesInfoMap, and unionTypesInfoMap need to be cleared at the start of buildTypesInfo() or at the end of generateFromMetadata(). The simplest fix is to add clearing statements at the beginning of buildTypesInfo (around line 205), before the maps are repopulated:

  this.objectTypesInfoMap = new Map();
  this.interfaceTypesInfoMap = new Map();
  this.inputTypesInfoMap = new Map();
  this.unionTypesInfoMap = new Map();

Alternatively, add these lines alongside the existing cleanup in generateFromMetadata (near line 154-155) where usedInterfaceTypes is already being reset.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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

Labels

Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants