On 01/09/20 14:06 +0200, François Dumont wrote:
Hi
No chance to review this small patch ?
I did review it, and I wasn't convinced it was a good change. It only
helps a particular usage pattern, and might hurt in other cases.
I don't agree with your assertion that you use std::deque when you
only use push_front() and you use std::list if you need both
push_front() and push_back().
Ideally we'd keep the most recently reallocated node around for reuse,
and then in the situation you describe the first push_front would
allocate a new node, but if you immediately do pop_back() we wouldn't
deallocate the node. But I haven't figured out a way to do that
caching without an ABI break.
The patch also has no tests. Are our existing tests sufficient to
cover this case? Do we want a test that verifies that we don't
allocate a new node if doing push_front() into an empty deque?
I would like to see a test doing something like:
std::deque<int, CustomAlloc<int>> d;
const int count = CustomAlloc::number_of_allocations;
d.push_front(1);
VERIFY( CustomAlloc::number_of_allocations == count );
d.pop_front();
d.push_back(1);
VERIFY( CustomAlloc::number_of_allocations == count );
