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

Specifying content encoding when not necessary #7987

Closed
benmccann opened this issue Nov 2, 2017 · 4 comments
Closed

Specifying content encoding when not necessary #7987

benmccann opened this issue Nov 2, 2017 · 4 comments

Comments

@benmccann
Copy link
Contributor

I'm wondering if we should stop specifying the charset and let utf-8 be assumed? TechEmpower is suggesting we would do better on there benchmarks that way. I'm not sure if there are other considerations

Content encoding found in "application/json; charset=utf-8" where "application/json" is acceptable.
Additional response bytes may negatively affect benchmark performance.
See http://frameworkbenchmarks.readthedocs.org/en/latest/Project-Information/Framework-Tests/#specific-test-requirements

@gmethvin
Copy link
Member

gmethvin commented Nov 2, 2017

I'm pretty sure we are already doing that. We used to send charset in the past but I think that was removed. Is there a specific test you can point me to?

@benmccann
Copy link
Contributor Author

I saw it in the Travis logs for my PR TechEmpower/FrameworkBenchmarks#3034 to upgrade one of the projects to Play 2.6.6
https://travis-ci.org/TechEmpower/FrameworkBenchmarks/jobs/296489436

@gmethvin
Copy link
Member

gmethvin commented Nov 2, 2017

Actually, you're right. There is different logic for the Java sendJson: https://github.com/playframework/playframework/blob/2.6.7/framework/src/play/src/main/java/play/mvc/StatusHeader.java#L356. We really should be using a constant there.

I also found it a bit odd that that project is using a controller annotation for something that could be done in a filter: https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/Java/play2-java/play2-java/app/utils/Headers.java. Not sure if that affects the performance.

@mkurz
Copy link
Member

mkurz commented Nov 13, 2017

@gmethvin

I also found it a bit odd that that project is using a controller annotation for something that could be done in a filter: https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/Java/play2-java/play2-java/app/utils/Headers.java.

Turns out there is no need anymore to set the Server and Date header via a controller annotation: The Server header can simply be set via play.server.akka.server-header = ... in application.conf now and the Date header will be send for each response by Play anyway - maybe that wasn't the case in older Play releases when these benchmarks tests were written.
So I removed the @Headers annotation in TechEmpower/FrameworkBenchmarks#3061

Not sure if that affects the performance.

The play.mvc.Filter implementation has a serious performance issue, see #8010. However, it seems like action composition is slightly faster than java filters (when extending EssentialFilter instead of the faulty Filter class), however I am not sure if that was just the case for the benchmarks I ran. I think it's almost save to say the performance of filters vs. action composition is almost the same...

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

3 participants