Skip to content

Commit

Permalink
QtConcurrent: filter/map reduction without default ctor
Browse files Browse the repository at this point in the history 
Previously a default constructor was required for the result type
of mappedReduced() and filteredReduced(), even if a default value
was provided.

This patch fixes the problem.

The issue was in the ResultReporter type, that was calling
QList::resize() to adjust the size of expected reported results.
A default-value parameter was added to the class, so that
a corresponding overload of QList::resize could be invoked.

Task-number: QTBUG-88452
Change-Id: I51113753e314d76aa74d201b5a7e327a6ca75f47
Reviewed-by: Sona Kurazyan <[email protected]>
  • Loading branch information
@isolovev
isolovev committed Dec 4, 2020
1 parent 0c19e3f commit 3d780c0
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 34 deletions.
20 changes: 9 additions & 11 deletions src/concurrent/qtconcurrentfilterkernel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,21 +160,19 @@ template <typename ReducedResultType,
typename qValueType<Iterator>::value_type> >
class FilteredReducedKernel : public IterateKernel<Iterator, ReducedResultType>
{
ReducedResultType reducedResult;
ReducedResultType &reducedResult;
KeepFunctor keep;
ReduceFunctor reduce;
Reducer reducer;
typedef IterateKernel<Iterator, ReducedResultType> IterateKernelType;

public:
template <typename Keep = KeepFunctor, typename Reduce = ReduceFunctor>
FilteredReducedKernel(QThreadPool *pool,
Iterator begin,
Iterator end,
Keep &&_keep,
Reduce &&_reduce,
ReduceOptions reduceOption)
: IterateKernelType(pool, begin, end), reducedResult(), keep(std::forward<Keep>(_keep)),
template<typename Keep = KeepFunctor, typename Reduce = ReduceFunctor>
FilteredReducedKernel(QThreadPool *pool, Iterator begin, Iterator end, Keep &&_keep,
Reduce &&_reduce, ReduceOptions reduceOption)
: IterateKernelType(pool, begin, end),
reducedResult(this->defaultValue.value),
keep(std::forward<Keep>(_keep)),
reduce(std::forward<Reduce>(_reduce)),
reducer(pool, reduceOption)
{ }
Expand All @@ -183,8 +181,8 @@ class FilteredReducedKernel : public IterateKernel<Iterator, ReducedResultType>
FilteredReducedKernel(QThreadPool *pool, Iterator begin, Iterator end, Keep &&_keep,
Reduce &&_reduce, ReducedResultType &&initialValue,
ReduceOptions reduceOption)
: IterateKernelType(pool, begin, end),
reducedResult(std::forward<ReducedResultType>(initialValue)),
: IterateKernelType(pool, begin, end, std::forward<ReducedResultType>(initialValue)),
reducedResult(this->defaultValue.value),
keep(std::forward<Keep>(_keep)),
reduce(std::forward<Reduce>(_reduce)),
reducer(pool, reduceOption)
Expand Down
87 changes: 76 additions & 11 deletions src/concurrent/qtconcurrentiteratekernel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,22 @@ template <typename T>
class ResultReporter
{
public:
ResultReporter(ThreadEngine<T> *_threadEngine)
:threadEngine(_threadEngine)
ResultReporter(ThreadEngine<T> *_threadEngine, T &_defaultValue)
: threadEngine(_threadEngine), defaultValue(_defaultValue)
{

}

void reserveSpace(int resultCount)
{
currentResultCount = resultCount;
vector.resize(qMax(resultCount, vector.count()));
resizeList(qMax(resultCount, vector.count()));
}

void reportResults(int begin)
{
const int useVectorThreshold = 4; // Tunable parameter.
if (currentResultCount > useVectorThreshold) {
vector.resize(currentResultCount);
resizeList(currentResultCount);
threadEngine->reportResults(vector, begin);
} else {
for (int i = 0; i < currentResultCount; ++i)
Expand All @@ -125,6 +124,17 @@ class ResultReporter
int currentResultCount;
ThreadEngine<T> *threadEngine;
QList<T> vector;

private:
void resizeList(qsizetype size)
{
if constexpr (std::is_default_constructible_v<T>)
vector.resize(size);
else
vector.resize(size, defaultValue);
}

T &defaultValue;
};

template <>
Expand All @@ -137,6 +147,22 @@ class ResultReporter<void>
inline void * getPointer() { return nullptr; }
};

template<typename T>
struct DefaultValueContainer
{
template<typename U = T>
DefaultValueContainer(U &&_value) : value(std::forward<U>(_value))
{
}

T value;
};

template<>
struct DefaultValueContainer<void>
{
};

inline bool selectIteration(std::bidirectional_iterator_tag)
{
return false; // while
Expand All @@ -160,11 +186,41 @@ class IterateKernel : public ThreadEngine<T>
public:
typedef T ResultType;

template<typename U = T, std::enable_if_t<std::is_same_v<U, void>, bool> = true>
IterateKernel(QThreadPool *pool, Iterator _begin, Iterator _end)
: ThreadEngine<U>(pool),
begin(_begin),
end(_end),
current(_begin),
iterationCount(selectIteration(IteratorCategory()) ? std::distance(_begin, _end) : 0),
forIteration(selectIteration(IteratorCategory())),
progressReportingEnabled(true)
{
}

template<typename U = T, std::enable_if_t<!std::is_same_v<U, void>, bool> = true>
IterateKernel(QThreadPool *pool, Iterator _begin, Iterator _end)
: ThreadEngine<T>(pool), begin(_begin), end(_end), current(_begin)
, iterationCount(selectIteration(IteratorCategory()) ? std::distance(_begin, _end) : 0)
, forIteration(selectIteration(IteratorCategory()))
, progressReportingEnabled(true)
: ThreadEngine<U>(pool),
begin(_begin),
end(_end),
current(_begin),
iterationCount(selectIteration(IteratorCategory()) ? std::distance(_begin, _end) : 0),
forIteration(selectIteration(IteratorCategory())),
progressReportingEnabled(true),
defaultValue(U())
{
}

template<typename U = T, std::enable_if_t<!std::is_same_v<U, void>, bool> = true>
IterateKernel(QThreadPool *pool, Iterator _begin, Iterator _end, U &&_defaultValue)
: ThreadEngine<U>(pool),
begin(_begin),
end(_end),
current(_begin),
iterationCount(selectIteration(IteratorCategory()) ? std::distance(_begin, _end) : 0),
forIteration(selectIteration(IteratorCategory())),
progressReportingEnabled(true),
defaultValue(std::forward<U>(_defaultValue))
{
}

Expand Down Expand Up @@ -199,7 +255,7 @@ class IterateKernel : public ThreadEngine<T>
ThreadFunctionResult forThreadFunction()
{
BlockSizeManager blockSizeManager(ThreadEngineBase::threadPool, iterationCount);
ResultReporter<T> resultReporter(this);
ResultReporter<T> resultReporter = createResultsReporter();

for(;;) {
if (this->isCanceled())
Expand Down Expand Up @@ -252,7 +308,7 @@ class IterateKernel : public ThreadEngine<T>
if (iteratorThreads.testAndSetAcquire(0, 1) == false)
return ThreadFinished;

ResultReporter<T> resultReporter(this);
ResultReporter<T> resultReporter = createResultsReporter();
resultReporter.reserveSpace(1);

while (current != end) {
Expand Down Expand Up @@ -283,6 +339,14 @@ class IterateKernel : public ThreadEngine<T>
return ThreadFinished;
}

private:
ResultReporter<T> createResultsReporter()
{
if constexpr (!std::is_same_v<T, void>)
return ResultReporter<T>(this, defaultValue.value);
else
return ResultReporter<T>(this);
}

public:
const Iterator begin;
Expand All @@ -294,6 +358,7 @@ class IterateKernel : public ThreadEngine<T>
const int iterationCount;
const bool forIteration;
bool progressReportingEnabled;
DefaultValueContainer<ResultType> defaultValue;
};

} // namespace QtConcurrent
Expand Down
26 changes: 14 additions & 12 deletions src/concurrent/qtconcurrentmapkernel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ template <typename ReducedResultType,
QtPrivate::MapResultType<Iterator, MapFunctor>>>
class MappedReducedKernel : public IterateKernel<Iterator, ReducedResultType>
{
ReducedResultType reducedResult;
ReducedResultType &reducedResult;
MapFunctor map;
ReduceFunctor reduce;
Reducer reducer;
Expand All @@ -102,20 +102,22 @@ class MappedReducedKernel : public IterateKernel<Iterator, ReducedResultType>
public:
typedef ReducedResultType ReturnType;

template <typename F1 = MapFunctor, typename F2 = ReduceFunctor>
MappedReducedKernel(QThreadPool *pool, Iterator begin, Iterator end, F1 &&_map,
F2 &&_reduce, ReduceOptions reduceOptions)
: IterateKernel<Iterator, ReducedResultType>(pool, begin, end), reducedResult(),
map(std::forward<F1>(_map)), reduce(std::forward<F2>(_reduce)),
template<typename F1 = MapFunctor, typename F2 = ReduceFunctor>
MappedReducedKernel(QThreadPool *pool, Iterator begin, Iterator end, F1 &&_map, F2 &&_reduce,
ReduceOptions reduceOptions)
: IterateKernel<Iterator, ReducedResultType>(pool, begin, end),
reducedResult(this->defaultValue.value),
map(std::forward<F1>(_map)),
reduce(std::forward<F2>(_reduce)),
reducer(pool, reduceOptions)
{ }

template <typename F1 = MapFunctor, typename F2 = ReduceFunctor>
MappedReducedKernel(QThreadPool *pool, Iterator begin, Iterator end, F1 &&_map,
F2 &&_reduce, ReducedResultType &&initialValue,
ReduceOptions reduceOptions)
: IterateKernel<Iterator, ReducedResultType>(pool, begin, end),
reducedResult(std::forward<ReducedResultType>(initialValue)),
template<typename F1 = MapFunctor, typename F2 = ReduceFunctor>
MappedReducedKernel(QThreadPool *pool, Iterator begin, Iterator end, F1 &&_map, F2 &&_reduce,
ReducedResultType &&initialValue, ReduceOptions reduceOptions)
: IterateKernel<Iterator, ReducedResultType>(pool, begin, end,
std::forward<ReducedResultType>(initialValue)),
reducedResult(this->defaultValue.value),
map(std::forward<F1>(_map)),
reduce(std::forward<F2>(_reduce)),
reducer(pool, reduceOptions)
Expand Down
76 changes: 76 additions & 0 deletions tests/auto/concurrent/qtconcurrentfiltermapgenerated/tst_qtconcurrent_selected_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,3 +294,79 @@ void tst_QtConcurrentFilterMapGenerated::moveOnlyReductionItem()
QCOMPARE(result, expected_result);*/
}

void tst_QtConcurrentFilterMapGenerated::noDefaultConstructorItemMapped()
{
/* test for
template<typename typename ResultType, typename Sequence, typename MapFunctor, typename
ReduceFunctor, typename reductionitemtype> ResultType blockingMappedReduced(QThreadPool* pool,
const Sequence & sequence, MapFunctor function, ReduceFunctor reduceFunction, reductionitemtype
&& initialValue, ReduceOptions);
with
inputsequence=standard
inputsequencepassing=lvalue
inputitemtype=standard
maptype=same
mappeditemtype=standard
reductiontype=different
reductionitemtype=noconstruct
mapfunction=functor
mapfunctionpassing=lvalue
reductionfunction=function
reductionfunctionpassing=lvalue
reductioninitialvaluepassing=lvalue
reductionoptions=unspecified
*/

QThreadPool pool;
pool.setMaxThreadCount(1);
auto input_sequence = []() {
std::vector<SequenceItem<tag_input>> result;
result.push_back(SequenceItem<tag_input>(1, true));
result.push_back(SequenceItem<tag_input>(2, true));
result.push_back(SequenceItem<tag_input>(3, true));
result.push_back(SequenceItem<tag_input>(4, true));
result.push_back(SequenceItem<tag_input>(5, true));
result.push_back(SequenceItem<tag_input>(6, true));
return result;
}();

auto map = MyMap<SequenceItem<tag_input>, SequenceItem<tag_input>> {};
auto reductor = myReduce<SequenceItem<tag_input>, NoConstructSequenceItem<tag_reduction>>;
auto initialvalue = NoConstructSequenceItem<tag_reduction>(0, true);

auto result =
QtConcurrent::blockingMappedReduced(&pool, input_sequence, map, reductor, initialvalue);

auto expected_result = NoConstructSequenceItem<tag_reduction>(42, true);

QCOMPARE(result, expected_result);
}

void tst_QtConcurrentFilterMapGenerated::noDefaultConstructorItemFiltered()
{
QThreadPool pool;
pool.setMaxThreadCount(1);
auto input_sequence = []() {
std::vector<SequenceItem<tag_input>> result;
result.push_back(SequenceItem<tag_input>(1, true));
result.push_back(SequenceItem<tag_input>(2, true));
result.push_back(SequenceItem<tag_input>(3, true));
result.push_back(SequenceItem<tag_input>(4, true));
result.push_back(SequenceItem<tag_input>(5, true));
result.push_back(SequenceItem<tag_input>(6, true));
return result;
}();

auto filter = MyFilter<SequenceItem<tag_input>> {};
auto reductor = myReduce<SequenceItem<tag_input>, NoConstructSequenceItem<tag_reduction>>;
auto initialvalue = NoConstructSequenceItem<tag_reduction>(0, true);

auto result = QtConcurrent::blockingFilteredReduced(&pool, input_sequence, filter, reductor,
initialvalue);

auto expected_result = NoConstructSequenceItem<tag_reduction>(9, true);

QCOMPARE(result, expected_result);
}
2 changes: 2 additions & 0 deletions tests/auto/concurrent/qtconcurrentfiltermapgenerated/tst_qtconcurrentfiltermapgenerated.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ private slots:
void moveOnlyReduceObject();
void functorAsReduction();
void moveOnlyReductionItem();
void noDefaultConstructorItemMapped();
void noDefaultConstructorItemFiltered();
// START_GENERATED_SLOTS (see generate_tests.py)
void test1();
// END_GENERATED_SLOTS (see generate_tests.py)
Expand Down

0 comments on commit 3d780c0

Please sign in to comment.