GNOME Bugzilla – Bug 659255
SoupHTTPInputStream: use a list of SoupBuffers instead of a buffer for leftover data
Last modified: 2011-11-07 18:15:19 UTC
SoupHTTPInputStream currently uses its own buffer to store received data that is pending to be read. Instead we can just store a list of buffers pending to be read. That would save us several realloc() and memcpy() if the client does not read from the stream quickly enough.
Created attachment 196735 [details] [review] Patch
Comment on attachment 196735 [details] [review] Patch sorry it took me so long to get back to this... basically good, but you can simplify a bit and get rid of one more memcpy: >+ g_queue_push_head (priv->leftover_queue, soup_buffer_copy (chunk_buffer)); >+ priv->leftover_offset = chunk_buffer->length - chunk_size; instead of soup_buffer_copy(chunk_buffer), do: soup_buffer_new_subbuffer(chunk_buffer, chunk_buffer->length - chunk_size, chunk_size); so you're only "copying" the part of the buffer you haven't used yet. And then you don't need priv->leftover_offset at all. And then in read_from_leftover(), you can remove priv->leftover_offset, and in the else case, instead of memcpy()ing data around in the existing SoupBuffer, just make another new subbuffer, and replace the queue head with that.
(In reply to comment #2) > (From update of attachment 196735 [details] [review]) > sorry it took me so long to get back to this... > > basically good, but you can simplify a bit and get rid of one more memcpy: Cool, didn't know about the subbuffer stuff. Will provide a new patch.
(In reply to comment #2) > (From update of attachment 196735 [details] [review]) > sorry it took me so long to get back to this... > > basically good, but you can simplify a bit and get rid of one more memcpy: [...] > And then in read_from_leftover(), you can remove priv->leftover_offset, and in > the else case, instead of memcpy()ing data around in the existing SoupBuffer, > just make another new subbuffer, and replace the queue head with that. I understand what you mean except the reference to getting rid of a memcpy. Even if we do what you propose we still have at the end to copy the contents from the soup buffer to the users' buffer (which is not a SoupBuffer).
oops, I misread the code; I thought it was moving data around inside the SoupBuffer at the end. So yeah, you won't actually get rid of a memcpy().
Maybe we can change the API later and force the caller to pass a SoupBuffer instead of a generic pointer.
sure. I still think it's worth using the subbuffers anyway though. it will at least get rid of leftover_offset
Created attachment 198944 [details] [review] Patch using subbuffers
This was committed some time ago 399c6105b7eda992d521cb55c96ededbbb456187