-
Notifications
You must be signed in to change notification settings - Fork 2.8k
minor refactor #1163
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
minor refactor #1163
Conversation
|
Review app for commit 6864cb9 deployed to Heroku: https://hge-ci-pull-1163.herokuapp.com |
|
Review app for commit 6c2ac32 deployed to Heroku: https://hge-ci-pull-1163.herokuapp.com |
|
Review app for commit aa10de2 deployed to Heroku: https://hge-ci-pull-1163.herokuapp.com |
|
Review app for commit 0274aad deployed to Heroku: https://hge-ci-pull-1163.herokuapp.com |
|
Review app for commit fc69e1f deployed to Heroku: https://hge-ci-pull-1163.herokuapp.com |
|
Review app for commit f0076e1 deployed to Heroku: https://hge-ci-pull-1163.herokuapp.com |
|
Review app https://hge-ci-pull-1163.herokuapp.com is deleted |
…sponses (#1163) <!-- The PR description should answer 2 important questions: --> ### What Fixes: https://linear.app/hasura/issue/ENG-1073/figure-out-whats-causing-the-slowdown-in-response-processing This removes the accidentally-quadratic behavior of repeated nested `from_value`/`to_value` calls where the assumption seems to have been that ndc model fields containing Value would be untouched (similar to the behavior of aeson). ### How We work with `Value` directly instead of using `serde` machinery to get a `RowSet`. #### Benchmarks The large result query shows modest latency improvement:  A query with a response of the same size as above, but with deep nesting to trigger quadratic behavior shows significant latency improvement (~12ms to ~2ms for `process_response` span)  Small responses (common, historically) don't show any improvement, as they don't exhibit the issue. V3_GIT_ORIGIN_REV_ID: f709e4f8a370b46c53f533eef2fef7e772b5cb72
Description
What component does this PR affect?
Requires changes from other components? If yes, please mark the components:
Related Issue
Solution and Design
Type
Checklist: