Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<deque>: Needs a major performance overhaul #147

Open
StephanTLavavej opened this issue Sep 30, 2019 · 3 comments
Open

<deque>: Needs a major performance overhaul #147

StephanTLavavej opened this issue Sep 30, 2019 · 3 comments
Labels
performance Must go faster vNext Breaks binary compatibility

Comments

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Sep 30, 2019

deque has long-standing performance problems, including but not limited to its choice of a very small block size. Unlike all other containers, deque::iterator always uses the proxy object, further adding to its complexity. When we can break binary compatibility in the vNext release, we need to eliminate deque's proxy (as we've already done for all other containers in debug mode, except vector<bool>), retune the block size, and generally rethink deque from scratch.

Also tracked by Microsoft-internal VSO-102760 / AB#102760.

vNext note: Resolving this issue will require breaking binary compatibility. We won't be able to accept pull requests for this issue until the vNext branch is available. See #169 for more information.

@StephanTLavavej StephanTLavavej added enhancement Something can be improved vNext Breaks binary compatibility performance Must go faster labels Sep 30, 2019
@StephanTLavavej StephanTLavavej removed the enhancement Something can be improved label Feb 6, 2020
@bubnikv
Copy link

bubnikv commented Apr 6, 2022

The current block size is tragically small: For more than two int32 values or for a single int64 the deque degenerates to a vector of pointers. Thus for all practical purposes, on MSVC deque IS a vector of pointers, which defeats its purpose of reducing allocator load and memory fragmentation.

https://github.com/microsoft/STL/blob/main/stl/inc/deque#L561

while GCC 4.6.3 allocates blocks of 512 bytes:

#define _GLIBCXX_DEQUE_BUF_SIZE 512
inline size_t
  __deque_buf_size(size_t __size)
 { return (__size < _GLIBCXX_DEQUE_BUF_SIZE
     ? size_t(_GLIBCXX_DEQUE_BUF_SIZE / __size) : size_t(1)); }

https://gcc.gnu.org/onlinedocs/gcc-4.6.3/libstdc++/api/a01049_source.html

boost::deque uses a similar logic and block size as the glibc implementation above.

bubnikv added a commit to prusa3d/PrusaSlicer that referenced this issue Apr 11, 2022
its purpose of reducing allocator load and memory fragmentation.
microsoft/STL#147 (comment)
Slic3r::deque<> compiles to boost::container::deque<> on Windows,
to std::deque<> on other systems.
SeamPlacer newly uses Slic3r::deque<>.
Godrak pushed a commit to prusa3d/PrusaSlicer that referenced this issue Apr 14, 2022
its purpose of reducing allocator load and memory fragmentation.
microsoft/STL#147 (comment)
Slic3r::deque<> compiles to boost::container::deque<> on Windows,
to std::deque<> on other systems.
SeamPlacer newly uses Slic3r::deque<>.
Godrak pushed a commit to prusa3d/PrusaSlicer that referenced this issue Apr 20, 2022
its purpose of reducing allocator load and memory fragmentation.
microsoft/STL#147 (comment)
Slic3r::deque<> compiles to boost::container::deque<> on Windows,
to std::deque<> on other systems.
SeamPlacer newly uses Slic3r::deque<>.
Godrak pushed a commit to prusa3d/PrusaSlicer that referenced this issue Apr 25, 2022
its purpose of reducing allocator load and memory fragmentation.
microsoft/STL#147 (comment)
Slic3r::deque<> compiles to boost::container::deque<> on Windows,
to std::deque<> on other systems.
SeamPlacer newly uses Slic3r::deque<>.
@frederick-vs-ja
Copy link
Contributor

Currently, both checked and unchecked iterators of MSVC STL's deque add one more layer of indirection. It seems that we can add simpler iterator types that hold the _Mapptr and the offset before vNext and use them internally. I don't know whether we can directly change unchecked iterators now.

@Seneral
Copy link

Seneral commented Dec 7, 2023

The tiny block size of MSVC's implementation is just utterly baffling.
Whoever made that choice actively contributed to make the world that much harder for devs, and should be ashamed.
Please fix this - in my limited knowledge I don't see how this could possibly be an ABI breaking change, but you had plenty choice to fix this even if it was, and you didn't.
For now, I gotta keep using my own implementation, or just accept that windows users get a less optimised experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster vNext Breaks binary compatibility
Projects
None yet
Development

No branches or pull requests

4 participants