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

Remove schema data from client message to devtool #1362

Merged
merged 2 commits into from
May 9, 2024

Conversation

hehaijin
Copy link
Contributor

@hehaijin hehaijin commented May 9, 2024

Schema data is requested by 'IntrospectionQuery'
for showing documentation in dev tool for the explorer tab.

It is removed from queries and cache tab as no need to show there.
However, for each message to sync client data to dev tools,
the schema data is still included in each message, and needs to be
stringified/parsed/processed/GCed for each message.

This creates big performance issue when schema is big.
by removing it I see a big reduction in memory and cpu usage.

@hehaijin hehaijin requested a review from a team as a code owner May 9, 2024 19:01
Copy link

relativeci bot commented May 9, 2024

#327 Bundle Size — 916.35KiB (+0.28%).

66ea5f6(current) vs 2fe2682 main#322(baseline)

Warning

Bundle contains 32 duplicate packages – View duplicate packages

Bundle metrics  Change 5 changes Regression 2 regressions
                 Current
#327
     Baseline
#322
Regression  Initial JS 878.95KiB(+0.29%) 876.4KiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 1.31% 0%
No change  Chunks 5 5
No change  Assets 12 12
Change  Modules 693(+1.46%) 683
Regression  Duplicate Modules 45(+28.57%) 35
Change  Duplicate Code 5.48%(+20.18%) 4.56%
No change  Packages 99 99
No change  Duplicate Packages 31 31
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#327
     Baseline
#322
Regression  JS 878.95KiB (+0.29%) 876.4KiB
No change  IMG 35.85KiB 35.85KiB
No change  HTML 810B 810B
No change  Other 778B 778B

Bundle analysis reportBranch hehaijin:remove-schema-from-mess...Project dashboard

@hehaijin
Copy link
Contributor Author

hehaijin commented May 9, 2024

in my case rough estimate in this test is that all other queries/mutations are around 100KB
while our schema is > 10MB.
So the effect is very obvious when removing the schema.
before
image
after
image

@hehaijin
Copy link
Contributor Author

hehaijin commented May 9, 2024

@jerelmiller could you take a look?

@hehaijin hehaijin force-pushed the remove-schema-from-message branch 2 times, most recently from e3cc3be to 6771365 Compare May 9, 2024 20:26
@jerelmiller
Copy link
Member

jerelmiller commented May 9, 2024

Hey @hehaijin this is a really great find! The reasoning here makes sense. The only thing that uses that introspection query is the explorer since it uses the apollo client instance to send queries. No reason we need to hang onto that.

Would you mind adding a changeset? I'll be happy to merge and get this released as soon as you do 🙂. Thanks so much for looking into this! I'm sure this will be super helpful for you app. That memory and CPU savings is nuts!

@hehaijin hehaijin force-pushed the remove-schema-from-message branch from 6771365 to 80b5b7c Compare May 9, 2024 20:55
@hehaijin
Copy link
Contributor Author

hehaijin commented May 9, 2024

@jerelmiller Thanks for review. Added change set.

Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

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

🔥 Thanks again! Looking forward to getting this one out.

@jerelmiller jerelmiller merged commit 72b181e into apollographql:main May 9, 2024
9 checks passed
@github-actions github-actions bot mentioned this pull request May 9, 2024
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

2 participants