redboltz / mqtt_cpp

Boost Software License 1.0
434 stars 107 forks source link

Don't shrink the payload_ buffer #228

Closed jonesmz closed 5 years ago

jonesmz commented 5 years ago

In the handle remaining length function, the size of the payload_ vector is changed on every incoming packet.

            payload_.resize(remaining_length_);
            if (remaining_length_ == 0) {
                handle_payload(func);
                return;
            }

Allocating new space like this is expensive.

Instead, we should only change the size of the buffer if more room is needed.

E.g.

if(payload_.size() < remaining_length_){ payload_.resize(remaining_length_);

Then, anywhere payload_ is used, we also use remaininglength to indicate how many bytes in payload_ are actually part of the current packet.

This way, we don't allocate memory for every packet. We only allocate memory when the newly received packet is larger than the buffer.

redboltz commented 5 years ago

I don't think so.

See http://www.cplusplus.com/reference/vector/vector/resize/


Iterator validity

In case the container shrinks, all iterators, pointers and references to elements that have not been removed remain valid after the resize and refer to the same elements they were referring to before the call.

If the container expands, the end iterator is invalidated and, if it has to reallocate storage, all iterators, pointers and references related to this container are also invalidated.


That means memory reallocation doesn't happen at the shrink case.

I wrote checking code. https://wandbox.org/permlink/8yq3ufPVLMsgXTwt (Updated)

jonesmz commented 5 years ago

Ah, you're right. It's already doing the behavior that I described.

We should document that this is what's happening in the code.