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

fields_base::set avoiding temporaries when possible #113

Open
ashtum opened this issue Dec 6, 2024 · 3 comments
Open

fields_base::set avoiding temporaries when possible #113

ashtum opened this issue Dec 6, 2024 · 3 comments

Comments

@ashtum
Copy link
Collaborator

ashtum commented Dec 6, 2024

Temporary strings are sometimes required to call fields_base::set. For example:

request.set(field::host, url.authority().encoded_host_and_port().decode());
std::string field = cookie_jar->make_field(url); // temporary 
field.append(explicit_cookies); // ideally append in place
request.set(field::cookie, field);

It would be useful to provide an interface that enables this.

@vinniefalco
Copy link
Member

I'm not quite sure what you are asking. request::set does append to the internal string, and it does so in a single call to resize the buffer if needed. Or do you mean that you want some kind of way to write the result of cookie_jar::make_field directly into the destination container? That's an interesting idea but could be an example of going too far. Temporary calculations with strings are something that the computer is supposed to just do. I think it is better to just write it the normal way and worry about optimizing later. If we find that this is a bottleneck, we might prefer instead to use some flavor of SmallString such as the one here:
https://llvm.org/doxygen/classllvm_1_1SmallString.html

@ashtum
Copy link
Collaborator Author

ashtum commented Dec 7, 2024

I'm uncertain about the interface, but something that eliminates the need for a temporarily allocated string would be ideal.
Just to clarify what I mean:

auto host_and_port = url.authority().encoded_host_and_port();
request.set(
    field::host,
    host_and_port.decoded_size(),
    [&](std::span<char> buffer)
    {
        // copy_to is a string_token
        host_and_port.decode({}, copy_to(buffer));
    });

But I agree this is not something we need to be concerned about at the moment, especially considering the allocated temporary will be deallocated immediately afterward.

@vinniefalco
Copy link
Member

too much try-hard

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