Skip to content
This repository was archived by the owner on Jun 30, 2023. It is now read-only.

Improve memory usage by serializing the response object directly#159

Merged
tangiel merged 2 commits intocloudendpoints:masterfrom
AODocs:stream_response
Aug 20, 2018
Merged

Improve memory usage by serializing the response object directly#159
tangiel merged 2 commits intocloudendpoints:masterfrom
AODocs:stream_response

Conversation

@clementdenis
Copy link
Contributor

When serializing big responses as JSON (tens of MBs), Cloud Endpoints can easily trigger OOMEs, even on the relatively "big" F4 App Engine instances. This is caused by the creation of an intermediate String object containing the whole JSON response.

This change does not create the intermediate String anymore, and streams the JSON output directly to the response (even if it's not real streaming on App Engine).
As a side effect, the Content-Length header is not set by default, as it's not necessary anyway on App Engine (removed or changed by front-end servers).
The header can be reenabled if necessary with the servlet init parameter addContentLength=true, with a negligible performance penalty (requires the object to be serialized twice). However, I don't see any obvious use case for it.

After this change, F2 instance can reach the maximum response size of 32MB without OOME. Previously, OOME started to happen around 22MB reponse size.
It also provides a significant latency improvement (3.5s vs. 4.5s on a F2 instance for a quite complex 12MB JSON reponse).

Note: if you intend to perform performance tests, you should not map the EndpointsServlet to the default /_ah/api/* path, as the request would be going through the legacy Endpoints v1 servers, that add a very significant latency to big responses (even if Endpoints v1 is officially shut down since August 2, 2018)

Clément Denis added 2 commits August 16, 2018 18:24
- Prevents an intermediate String object from being created
- Content-Length header is not set calculated by default anymore
- Content-Length can be reenabled with addContentLength=true init param
@codecov-io
Copy link

codecov-io commented Aug 16, 2018

Codecov Report

Merging #159 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #159      +/-   ##
============================================
+ Coverage        80%   80.05%   +0.04%     
- Complexity     1680     1681       +1     
============================================
  Files           156      156              
  Lines          5597     5601       +4     
  Branches        731      733       +2     
============================================
+ Hits           4478     4484       +6     
+ Misses          839      837       -2     
  Partials        280      280
Impacted Files Coverage Δ Complexity Δ
...pi/server/spi/handlers/EndpointsMethodHandler.java 100% <ø> (ø) 10 <0> (ø) ⬇️
...rver/spi/response/ServletResponseResultWriter.java 94.73% <100%> (+2.73%) 17 <3> (ø) ⬇️
.../server/spi/response/RestResponseResultWriter.java 100% <100%> (ø) 5 <1> (ø) ⬇️
...pi/server/spi/ServletInitializationParameters.java 96% <100%> (+0.25%) 18 <0> (+1) ⬆️
...c/main/java/com/google/api/server/spi/EnvUtil.java 70% <0%> (ø) 8% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2a26bd...acad93c. Read the comment docs.

@tangiel
Copy link
Contributor

tangiel commented Aug 16, 2018

Hmm, do you happen to be using API management? I have a hunch that it was programmed this way specifically because of API management. We may need to make adjustments to accommodate this change.

@clementdenis
Copy link
Contributor Author

I don't use it on production projects, but I performed tests both on Flex with ESP and Standard with ServiceManagementConfigFilter + GoogleAppEngineControlFilter, and everything seems to work fine.

Also, it seems like the Flex + ESP behaves mostly like Standard + GSE:

  • When possible, response is automatically gzipped and chunked
  • If the client does not accept gzip, Content-Length is automatically added, even when not provided by backend

So at least for App Engine (Flex or Standard), the Content-Length header is never needed.
I don't know for other kind of deployments (GCE, GKE, local ESP, etc.).

What part of the changes do you think could cause issues with API management?

@tangiel
Copy link
Contributor

tangiel commented Aug 20, 2018

I don't remember, it's mostly dejavu feelings in my head :) since you've tested it, I think it's no worry.

@tangiel tangiel merged commit c69273d into cloudendpoints:master Aug 20, 2018
@clementdenis clementdenis deleted the stream_response branch February 21, 2022 13:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants