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 Answers 3

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\$
4
  • \$\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\$ Commented Jul 7, 2019 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\$ Commented Jul 7, 2019 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\$ Commented Jul 8, 2019 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\$ Commented Jul 8, 2019 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\$
1
  • 1
    \$\begingroup\$ There are actually attributes [[likely]] and [[unlikely]] coming in C++20. gcc has it in version 9 \$\endgroup\$ Commented Jul 7, 2019 at 13:06

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.