abseil-cpp icon indicating copy to clipboard operation
abseil-cpp copied to clipboard

InlinedVector::clear() should lazily avoid deallocating out-of-line memory

Open mcskatkat opened this issue 4 years ago • 2 comments

abseil-cpp/absl/container/inlined_vector.h

Describe the bug

InlinedVector::clear() method should lazily avoid deallocating out-of-line memory, if already allocated. This would be in line with the behavior of std::vector, for which it is designed to be a drop-in replacement. Deallocation on clear() means that if any number of elements beyond the inline capacity are re-pushed into the container, it will reallocate and re-copy contents unnecessarily. As of this writing, the master branch still has storage_.DeallocateIfAllocated(); in clear()

Steps to reproduce the bug

InlinedVector<int, 8> container;
container.reserve(max_size); // because I KNOW what the maximum required size is, but not necessarily at compile time
for (int count=max_size; count>0; --count) {
#if UGLY_WORKAROUND
    container.erase(container.begin(), container.end());  // does not deallocate
#else
    container.clear();  // deallocates storage :-(
#endif
    for (int i=0; i<count; ++i)
        container.push_back(i);
    do_something_with(container);
}

If called with max_size set to 16, the above code will trigger rellocation+copy at least 8 times. With the workaround (or correct clear() behavior) it will never reallocate/copy.

What version of Abseil are you using? Any, AFAIK. Specifically, lts_2020_02_25 has it.

What operating system and version are you using Irrelevant, but Ubuntu 18.04

What compiler and version are you using? g++ 7.5.0

What build system are you using? None (make)

mcskatkat avatar Sep 04 '20 11:09 mcskatkat

@CJ-Johnson looked into making this change a year ago, but found it broke a lot of code and was unable to make the change.

@CJ-Johnson, are you willing to take another shot at this?

derekmauro avatar Sep 08 '20 13:09 derekmauro

I don't have the cycles at the moment, but it is on my list!

For tracking purposes, the corresponding bug ID is 138307414

CJ-Johnson avatar Oct 19 '20 22:10 CJ-Johnson