Skip to content

Allow to avoid a string copy in string_response constructor - #201

Merged
etr merged 1 commit into
etr:masterfrom
fechantillac-antidot:string_response_constructor_use_move
Jul 22, 2020
Merged

etr merged 1 commit into
etr:masterfrom
fechantillac-antidot:string_response_constructor_use_move

Conversation

@fechantillac-antidot

@fechantillac-antidot fechantillac-antidot commented Jul 21, 2020 •

Copy link
Copy Markdown
Contributor

Description of the Change

For the string_response constructor, instead of using a const reference, pass the string content by value (hence can be moved into) and move it into the content field. This allow to avoid a copy by doing things like

string_response my_response(std::move(my_string));

Quantitative Performance Benefits

I ran benchmarks with gatling against my own application using libhttpserver as a frontend for the application backed by a mongo DB. Each run is 60 seconds. Used 10k and 10MB responses. Between 10 and 50 gatling clients. Performance gain is 3% to 5% depending on the setup.

If you have a standard way to run benchmarks I would be happy to run those.

Possible Drawbacks

I don't see any drawbacks.

Verification Process

It passes my own test suites. Sorry I was unable to run libhttpserver tests, I got unexpected errors when running make check, e.g undefined reference to httpserver::resource_init...

Release Notes

Slight performances improvement by allowing to skip a copy in string_response constructor.

@codecov

codecov Bot commented Jul 21, 2020 •

Copy link
Copy Markdown

Codecov Report

Merging #201 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #201   +/-   ##
=======================================
  Coverage   95.63%   95.63%           
=======================================
  Files          35       35           
  Lines        3230     3230           
=======================================
  Hits         3089     3089           
  Misses        141      141           
Impacted Files Coverage Δ
src/httpserver/string_response.hpp 75.00% <100.00%> (ø)

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 cbaa3f3...ce387e0. Read the comment docs.

@etr

etr commented Jul 22, 2020

Copy link
Copy Markdown
Owner

No worries for the tests - appveyor and travis have run them.
See: https://travis-ci.org/github/etr/libhttpserver/builds/710289396 and https://ci.appveyor.com/project/etr/libhttpserver/builds/34213707

Thanks for the help!

@etr
etr merged commit e9da6f8 into etr:master Jul 22, 2020
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.

2 participants