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

Performance: reserve body to avoid frequent reallocations and copies #1788

Closed
wants to merge 1 commit into from

Conversation

ArnaudBienner
Copy link
Contributor

@ArnaudBienner ArnaudBienner commented Feb 29, 2024

The append I'm trying to avoid is the one at line 7507:
res.body.append(buf, n);

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ArnaudBienner
Copy link
Contributor Author

Sorry for the mess with the other PR.

Hopefully this will make more sense now :) This patch, but also the alternative approach I made in #1782.

@yhirose
Copy link
Owner

yhirose commented Mar 10, 2024

@ArnaudBienner same as #1781 (comment), I don't think it improve performance since std::string::append won't be called after your code.

@ArnaudBienner
Copy link
Contributor Author

@ArnaudBienner same as #1781 (comment), I don't think it improve performance since std::string::append won't be called after your code.

Isn't it called as part of out which is passed to detail::read_content along with res and used to read the data?

@yhirose
Copy link
Owner

yhirose commented Apr 12, 2024

@ArnaudBienner sorry for the late reply. You are right. res.body.append gets called after your code. But the code shouldn't be called when a content receiver is provided, because res.body should be empty in this case.

    auto res = cli.Get("/hi", [&](const char *data, size_t data_length) {
      ...
      return true;
    });

@yhirose yhirose closed this in 6cdd349 Sep 1, 2024
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.

None yet

2 participants