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

DelayedOutputStream and Ok.chunked performance #5

Closed
CassOnMars opened this issue Sep 1, 2015 · 2 comments
Closed

DelayedOutputStream and Ok.chunked performance #5

CassOnMars opened this issue Sep 1, 2015 · 2 comments

Comments

@CassOnMars
Copy link

In my fork I have already removed DelayedOutputStream and substituted it with just a ByteArrayOutputStream, so that I could shrink the handle method in CxfController down to

val delayedOutput = new ByteArrayOutputStream
val replyPromise: Promise[Message] = Promise.apply()
dispatchMessage(extractMessage, delayedOutput, replyPromise)

replyPromise.future.map { outMessage =>
  Ok(delayedOutput.toByteArray) withHeaders(
    Message.CONTENT_TYPE -> outMessage.get(Message.CONTENT_TYPE).asInstanceOf[String]
  )
}

So far, I haven't found any problems in doing so, and the response time improved by nearly 200ms in my test environment. I'm curious if there was a reason in using the DelayedOutputStream rather than the direct dump of the byte array? I would gladly set up a pull request if this change is acceptable.

@kustra
Copy link
Contributor

kustra commented Sep 14, 2015

Using DelayedOutputStream is supposed to avoid storing the whole response in memory before returning it to the client, assuming that Apache CXF (or Play) doesn't store the whole response in memory either.
This could mean a difference with large response bodies.
(I haven't tested it in practice so it might not work as intended just yet.)

I understand it's not needed for everyone, but I wouldn't just remove it. If you were to abstract this logic out of CxfHandler and make it changeable by the programmer, I would be glad to accept a pull request. I guess some pluggable "CXF Message to Play Result" converter trait and its two implementations (Delayed and ByteArray) could do the trick.

@CassOnMars
Copy link
Author

I'll look further into how the DelayedOutputStream is behaving, and reformulate the pull request accordingly.

Thank you for your comments.

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

No branches or pull requests

2 participants