Static partition forward ranges correctly#1753
Conversation
In DevCom-1326903 the user reports that foward ranges, specifically std::map in their case, produce incorrect results from std::equal. After reducing to forward_list and debugging through, the problem is that when processing "unchunked items" we aren't giving those intitial chunks their extra item, so the std::equal call fails fast whenever there are unchunked items. Testing before didn't catch this because the values I chose of being around the thread count didn't account for the underlying infrastructure using _Oversubscription_multiplier which meant that the partitioning edge cases were not exercised (in most cases the parallel algorithms tests were testing only 1 element per chunk). <execution> Add the missing +1. Note that this +1 is the same +1 in the random-access version on line 907. $/tests/std/include/parallel_algorithms_utilities.hpp: Adjust the N constants to account for _Oversubscription_multiplier.
BillyONeal
force-pushed
the
forward_algorithm_partitioning
branch
from
ca234d7 to
6814762
Compare
BillyONeal
force-pushed
the
forward_algorithm_partitioning
branch
from
6814762 to
b32df54
Compare
|
FYI @mnatsuhara, I pushed minor changes after you approved. |
BillyONeal
force-pushed
the
forward_algorithm_partitioning
branch
from
5a75db6 to
9d688d2
Compare
CaseyCarter
changed the title
|
Thanks for this involved fix, Billy-3! |
I’ll be sure to leave in lots of typos to get credit for fixing 🥲 At least it isn’t something like, I don’t know, not writing a copy ctor this time |
Everyone writes bugs - that's normal. Not everyone is willing to donate their personal time to fix said bugs when it's no longer their day job. That said, there's also a chunk of bonus credit here since it would presumably take someone who didn't originally author the code twice as long to locate the problem and develop a clean fix.
I'd take your "missing copy constructor" bug over my use-after-free |
I wasn't getting paid for this?
Bug off! Bug off! Bug off!.... |
4 participants
In DevCom-1326903 the user reports that forward ranges, specifically
std::mapin their case, produce incorrect results fromstd::equal. After reducing toforward_listand debugging through, the problem is that when processing "unchunked items" we aren't giving those initial chunks their extra item, so thestd::equalcall fails fast whenever there are unchunked items.Testing before didn't catch this because the values I chose of being around the thread count didn't account for the underlying infrastructure using
_Oversubscription_multiplierwhich meant that the partitioning edge cases were not exercised (in most cases the parallel algorithms tests were testing only 1 element per chunk).<execution>Add the missing +1. Note that this +1 is the same +1 in the iterator-returning version on line 907.$/tests/std/include/parallel_algorithms_utilities.hpp: Adjust the N constants to account for_Oversubscription_multiplier.Resolves DevCom-1326903/VSO-1273957/AB#1273957.