drogonframework / drogon

Drogon: A C++14/17/20 based HTTP web application framework running on Linux/macOS/Unix/Windows
MIT License
11.53k stars 1.11k forks source link

Passing binary arrays directly to a resp->setBody() #2139

Open RomanSpies opened 2 months ago

RomanSpies commented 2 months ago

It's disheartening to dance on eggshells when trying to pass a large binary array to drogon's setBody method. Nothing works. No shared_ptr, no string_view, nothing. The only way to pass a binary array right now seems to be using the list constructor of std::string by calling std::string fullResponse(char myData, size_t myDataSize); and then calling std::move to hand it over to the setBody method, which involves a wholly unnecessary copy operation. Yes, compared to the compression, serialization, its not really THAT relevant. Nonetheless, what I would like is a method overload which accepts a char and a size_t. The same goes for the send() method of the async stream.

RomanSpies commented 2 months ago

I've taken a further look at the code and it seems while a httpResponse's setBody method can and will accept a r-value, taking advantage of a std::move operation on a std::string, the async-streams send method will not do the same and probably incur yet another copy of the data when passing back the std::string via send, even when encapsulating it in a std::move. Personally, I'd suggest having both methods accept a shared or unique pointer with a custom destructor, so this may be possible:

char* myCompressedBinaryData = static_cast<char*>(mi_malloc(maxBinarySize))
*ZSTD compression op outputting data into myCompressedBinaryData *
auto responseBody = std::shared_ptr<char>(myCompressedBinaryData , [](char* ptr) {
        mi_free(ptr);  
    }); // maybe unique_ptr due to lower overhead ? 

auto resp = drogon::HttpResponse::newHttpResponse();
resp->setContentTypeCode(CT_APPLICATION_OCTET_STREAM);
resp->setBody(responseBody, compressed_size);
callback(resp);
co_return;
fwsGonzo commented 1 month ago

Agreed. I'm having the same issues with copies.

mcopik commented 1 month ago

Same problem here. Could we maybe support at least string_view?

mcopik commented 1 month ago

@RomanSpies @fwsGonzo I found that there is already internal support for using string_view as the response body. However, it is used in a private method of the HttpResponse class through the private setBody(char*, size_t) function. It is only exposed to the user through a function that accepts a constant-size array:

template <int N>                                                                                                                                                                                                                                                     
void setBody(const char (&body)[N])                                                                                                                                                                                                                                  
{                                                                                                                                                                                                                                                                    
  assert(strnlen(body, N) == N - 1);                                                                                                                                                                                                                               
  setBody(body, N - 1);                                                                                                                                                                                                                                            
} 

This triggers the following function in lib/src/HttpResponseImpl.h: https://github.com/drogonframework/drogon/blob/13d714876485a1a55969bed99cf5fa437dced0c3/lib/src/HttpResponseImpl.h#L456

The created object of type HttpMessageStringViewBody creates a string_view, so it neither copies data nor takes ownership of it.

By adding a single function in lib/inc/drogon/HttpResponse.h, we can use it for a generic zero-copy response where the user is responsible for the lifetime of the response while benefiting from a zero-copy response:

    void setViewBody(const char* body, int N)                                                                                           
    {                                                                                                                                   
        setBody(body, N);                                                                                                               
    }  

I'm happy to open a PR if you even this is a good direction.

fwsGonzo commented 1 month ago

Unfortunately, we don't know when the response completes, or stops needing to read from it. Something with a callback or ref-counting without atomics (preventing use of shared_ptr) would be needed to make it truly low-latency, when the data has a free function (depending on which arena it came from). Even simple bump allocators that drop all allocations after a request is barred using this method. Unless I missed something.

For anything with static storage, string_view is indeed perfect, but I hope that we can do better and support dynamic response data too.