4
\$\begingroup\$

I wanted a buffer like std::vector<uint8_t> which is very fast at resizing, and it does not require byte initialization (it resizes to 16K, then to 2-4 bytes, then goes back to 16K, and repeat, a lot. std::vector zeroes all the bytes every time it grows back to 16K, I thought I could avoid the zeroing-overhead with a custom implementation)

I came up with this:

// buffer which should be faster than std::vector<uint8_t> when resizing a lot because it does not do byte initialization when resizing
class uint8_fast_buffer
{
public:
    uint8_fast_buffer(const size_t initial_size)
    {
        if(initial_size == 0)
        {
            // .. i don't really like the idea of buf being nullptr, this avoids that issue.
            this->internal_reserve(1);
        }
        else
        {
            this->internal_reserve(initial_size);
            this->buf_size=initial_size;
        }
    }
    ~uint8_fast_buffer() noexcept
    {
        free(this->buf);
    }
    size_t size(void) noexcept
    {
        return this->buf_size;
    }
    void reserve(const size_t reserve_size)
    {
        if(reserve_size > this->buf_size_real)
        {
            this->internal_reserve(reserve_size);
        }
    }
    // this function is supposed to be very fast when newlen is smaller than the biggest it has ever been before.
    void resize(const size_t newlen)
    {
        if(__builtin_expect(newlen > this->buf_size_real,0))
        {
            this->internal_reserve(newlen);
        }
        this->buf_size=newlen;
    }
    void append(const uint8_t *data, const size_t len){
        const size_t pos=this->size();
        const size_t new_pos=this->size()+len;
        this->resize(new_pos);
        memcpy(&this->buf[pos],data,len);
    }
    void reset(void) noexcept
    {
        this->buf_size=0;
    }
    bool empty(void) noexcept
    {
        return (this->buf_size==0);
    }
    uint8_t* data(void) noexcept
    {
        return this->buf;
    }
    uint8_t& at(const size_t pos)
    {
        if(__builtin_expect(pos >= this->size(),0))
        {
            throw std::out_of_range(std::to_string(pos)+std::string(" >= ")+std::to_string(this->size()));
        }
        return this->buf[pos];
    }
    uint8_t& operator[](const size_t pos) noexcept
    {
        return this->buf[pos];
    }
private:
    void internal_reserve(const size_t reserve_size)
    {
        uint8_t *newbuf=(uint8_t*)realloc(this->buf,reserve_size);
        if(__builtin_expect(newbuf == nullptr,0))
        {
            throw std::bad_alloc();
        }
        this->buf_size_real=reserve_size;
        this->buf=newbuf;
    }
    size_t buf_size=0;
    size_t buf_size_real=0;
    uint8_t *buf=nullptr;
};
\$\endgroup\$
3
\$\begingroup\$
// buffer which should be faster than std::vector<uint8_t> when resizing a lot because it does not do byte initialization when resizing

This comment line is really long. You should wrap the text so that there is no horizontal scrollbar anymore.

class uint8_fast_buffer
{
public:
    uint8_fast_buffer(const size_t initial_size)

Undeclared identifier size_t. You forgot to post the #include lines; they are required for a good code review.

    {
        if(initial_size == 0)

There's a space missing between the if and the open parenthesis.

        {
            // .. i don't really like the idea of buf being nullptr, this avoids that issue.
            this->internal_reserve(1);
        }
        else
        {
            this->internal_reserve(initial_size);
            this->buf_size=initial_size;
        }
    }

The above constructor can be written in a shorter way, like this:

    uint8_fast_buffer(const size_t initial_size)
    {
        this->buf_size = initial_size;
        this->internal_reserve(std::max(initial_size, 1));
    }

Continuing with your code:

    ~uint8_fast_buffer() noexcept
    {
        free(this->buf);
    }
    size_t size(void) noexcept

The void is not necessary inside the parentheses. Remove it.

    {
        return this->buf_size;
    }
    void reserve(const size_t reserve_size)
    {
        if(reserve_size > this->buf_size_real)
        {
            this->internal_reserve(reserve_size);
        }
    }
    // this function is supposed to be very fast when newlen is smaller than the biggest it has ever been before.
    void resize(const size_t newlen)
    {
        if(__builtin_expect(newlen > this->buf_size_real,0))

There's a space missing after the if, and after the comma.

        {
            this->internal_reserve(newlen);
        }
        this->buf_size=newlen;
    }
    void append(const uint8_t *data, const size_t len){

There's a newline missing before the {. You should let your IDE format the source code, to avoid these inconsistencies.

        const size_t pos=this->size();
        const size_t new_pos=this->size()+len;
        this->resize(new_pos);
        memcpy(&this->buf[pos],data,len);

Therearespacesmissingallovertheplaceespeciallyaroundtheoperators.

    }
    void reset(void) noexcept
    {
        this->buf_size=0;
    }
    bool empty(void) noexcept
    {
        return (this->buf_size==0);
    }

The parentheses are not necessary.

    uint8_t* data(void) noexcept
    {
        return this->buf;
    }
    uint8_t& at(const size_t pos)
    {
        if(__builtin_expect(pos >= this->size(),0))
        {
            throw std::out_of_range(std::to_string(pos)+std::string(" >= ")+std::to_string(this->size()));
        }
        return this->buf[pos];
    }
    uint8_t& operator[](const size_t pos) noexcept
    {
        return this->buf[pos];
    }
private:
    void internal_reserve(const size_t reserve_size)
    {
        uint8_t *newbuf=(uint8_t*)realloc(this->buf,reserve_size);
        if(__builtin_expect(newbuf == nullptr,0))
        {
            throw std::bad_alloc();
        }
        this->buf_size_real=reserve_size;
        this->buf=newbuf;
    }
    size_t buf_size=0;
    size_t buf_size_real=0;

To clearly distinguish buf_size and buf_size_real, the Go programming language uses buf_len and buf_cap (for capacity). The word real is rather confusing.

    uint8_t *buf=nullptr;
};

Summary

On a high level, the code looks good. It works, takes care of exceptional situations and defines a minimal and appropriate API.

On the syntactic level, there are many small details that can be improved, like consistent formatting and variable names.

Let your IDE take care of the formatting, write some unit tests, and you're fine.

\$\endgroup\$
  • \$\begingroup\$ thanks, i'll make sure to properly format the code and add #includes before going here in the future. also buf_cap is a much better name than buf_size_real (which could be confused with a real-typed version of the (size_t-typed) buf_size instead of the buffer capacity. Thanks for taking the time! \$\endgroup\$ – hanshenrik Jul 7 at 19:12
  • 2
    \$\begingroup\$ I'm generally uncomfortable with telling people to drop "unnecessary" consts. Although it's commonly neglected, especially in function parameters, it is still considered good practice to use const for values that are not supposed to change. E.g. github.com/isocpp/CppCoreGuidelines/blob/master/… \$\endgroup\$ – Josiah Jul 7 at 22:21
  • \$\begingroup\$ @Josiah thank you for providing this link, I removed that suggestion. I was still under the influence of Sun Studio CC from about 2006, which generated different mangled names whether or not an int parameter was const. \$\endgroup\$ – Roland Illig Jul 8 at 5:09
  • \$\begingroup\$ I'm guessing that size_t was intended to be std::size_t (from <cstdint>), but without any includes, who knows? Anything is possible! \$\endgroup\$ – Toby Speight Jul 8 at 11:02
3
\$\begingroup\$

Using this-> to refer to class members

Ditch this. It's unnecessary unless you need to have to refer to inherited members or disambiguate any other variables or parameters.

\$\endgroup\$
3
\$\begingroup\$

I prefer using the likely() macros over __builtin* directly because they may be more portable and they also reduce clutter.

\$\endgroup\$
  • \$\begingroup\$ There are actually attributes [[likely]] and [[unlikely]] coming in C++20. gcc has it in version 9 \$\endgroup\$ – Incomputable Jul 7 at 13:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.