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

Improve performance and reduce memory usage of /openapi/tree #1141

Merged
merged 4 commits into from
Aug 15, 2022

Conversation

darrelmiller
Copy link
Contributor

@darrelmiller darrelmiller commented Aug 13, 2022

Overview

This PR changes the way the /openapi/tree action returns the response payload to minimize memory usage.

Notes

This change takes advantage of the fact that the ASP.NET Response object provides access to a BodyWriter instance that can be passed to the constructor of Urtf8JsonWriter. This enables the server to construct the JSON response and have it written directly to the network stream without having to have the entire document buffered in memory.

A further enhancement could be made to cache the entire UrlNodeTree rather than reconstructing on every request.

One downside to this approach is that if an exception occurs whilst writing the tree, the HTTP response will already have returned a 200 response.

Testing Instructions

Calling https://localhost:5001/openapi/tree?graphVersions=v1.0 from multiple concurrent threads shows reduced memory usage on the server and shorter time to first byte.

@darrelmiller darrelmiller changed the title Dm/open api tree perf Improve performance and reduce memory usage of /openapi/tree Aug 13, 2022
@darrelmiller darrelmiller linked an issue Aug 13, 2022 that may be closed by this pull request
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
@baywet baywet enabled auto-merge (squash) August 15, 2022 12:55
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@baywet baywet merged commit 8b1167e into dev Aug 15, 2022
@baywet baywet deleted the dm/OpenApiTreePerf branch August 15, 2022 13:02
@irvinesunday
Copy link
Contributor

Thank you @darrelmiller for this fix. From my local tests the tree doesn't render on the browser. The expectation is to be able to render the tree on the browser as well, as is currently in prod.

image

@baywet
Copy link
Member

baywet commented Aug 15, 2022

I do get the tree to show up once I fixed the launch configuration and made sure an exception pause wasn't being raised (I don't have the connection strings to the storage account)
image

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.

Resolve OutOfMemory server errors causing 500 responses
3 participants