Differential D1712

Speedup KisConvolutionWorkerFFT: avoid QVector detaching, use iterators
ClosedPublic
Actions

Authored by kossebau on May 30 2016, 9:34 AM.

Details

Reviewers
dkazakov
rempt
Group Reviewers
Krita
Commits
R37:535cb0e5f610: Speedup KisConvolutionWorkerFFT: avoid QVector detaching, use iterators
R37:b9e89e0f8777: Speedup KisConvolutionWorkerFFT: avoid QVector detaching, use iterators
Summary

Iterators for speed up for the usual reasons compared to explicit data access by [].

Due to the implicit sharing logic of QVector the old code resulted
in creation of new memory objects on each loop run:

QVector d;
for-loop
  QVector c(d); // implicit sharing of data
  modify-d(); // copy-on-write -> new memory object allocated for d

This has been changed to

QVector d;
QVector c;
for-loop
  copy-data-from-c-to-d;
  modify-d(); // nothing shared, no new memory object

Diff Detail

Repository
R37 Krita
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau updated this revision to Diff 4060.May 30 2016, 9:34 AM
kossebau retitled this revision from to Speedup KisConvolutionWorkerFFT: avoid QVector detaching, use iterators.
kossebau updated this object.
kossebau edited the test plan for this revision. (Show Details)
kossebau added reviewers: Krita, rempt, dkazakov.
kossebau added a comment.May 30 2016, 9:38 AM

Old numbers of KisConvolutionPainterTest (just one snapshot, times have the usual jitter over multiple runs):

QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 1 time: 425
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 3 time: 730
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 5 time: 1059
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 7 time: 691
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 9 time: 692
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 11 time: 649
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 19 time: 683
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 27 time: 715
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 35 time: 668
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 43 time: 706
PASS   : KisConvolutionPainterTest::benchmarkConvolution()
QDEBUG : KisConvolutionPainterTest::testGaussianSpatial() krita.general: Elapsed time: 2758 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSpatial() krita.general: Elapsed time: 3154 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSpatial() krita.general: Elapsed time: 3401 ms
PASS   : KisConvolutionPainterTest::testGaussianSpatial()
QDEBUG : KisConvolutionPainterTest::testGaussianFFTW() krita.general: Elapsed time: 3603 ms
QDEBUG : KisConvolutionPainterTest::testGaussianFFTW() krita.general: Elapsed time: 3920 ms
QDEBUG : KisConvolutionPainterTest::testGaussianFFTW() krita.general: Elapsed time: 3974 ms
PASS   : KisConvolutionPainterTest::testGaussianFFTW()
QDEBUG : KisConvolutionPainterTest::testGaussianSmallSpatial() krita.general: Elapsed time: 13 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallSpatial() krita.general: Elapsed time: 19 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallSpatial() krita.general: Elapsed time: 24 ms
PASS   : KisConvolutionPainterTest::testGaussianSmallSpatial()
QDEBUG : KisConvolutionPainterTest::testGaussianSmallFFTW() krita.general: Elapsed time: 28 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallFFTW() krita.general: Elapsed time: 28 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallFFTW() krita.general: Elapsed time: 29 ms
PASS   : KisConvolutionPainterTest::testGaussianSmallFFTW()
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsSpatial() krita.general: Elapsed time: 13337 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsSpatial() krita.general: Elapsed time: 15957 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsSpatial() krita.general: Elapsed time: 15096 ms
PASS   : KisConvolutionPainterTest::testGaussianDetailsSpatial()
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsFFTW() krita.general: Elapsed time: 13514 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsFFTW() krita.general: Elapsed time: 13126 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsFFTW() krita.general: Elapsed time: 13831 ms
PASS   : KisConvolutionPainterTest::testGaussianDetailsFFTW()

New numbers of KisConvolutionPainterTest:

QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 1 time: 438
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 3 time: 784
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 5 time: 1127
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 7 time: 516
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 9 time: 501
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 11 time: 459
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 19 time: 556
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 27 time: 664
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 35 time: 456
QDEBUG : KisConvolutionPainterTest::benchmarkConvolution() krita.general: Diameter: 43 time: 512
PASS   : KisConvolutionPainterTest::benchmarkConvolution()
QDEBUG : KisConvolutionPainterTest::testGaussianSpatial() krita.general: Elapsed time: 3092 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSpatial() krita.general: Elapsed time: 3688 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSpatial() krita.general: Elapsed time: 3650 ms
PASS   : KisConvolutionPainterTest::testGaussianSpatial()
QDEBUG : KisConvolutionPainterTest::testGaussianFFTW() krita.general: Elapsed time: 2588 ms
QDEBUG : KisConvolutionPainterTest::testGaussianFFTW() krita.general: Elapsed time: 2615 ms
QDEBUG : KisConvolutionPainterTest::testGaussianFFTW() krita.general: Elapsed time: 2470 ms
PASS   : KisConvolutionPainterTest::testGaussianFFTW()
QDEBUG : KisConvolutionPainterTest::testGaussianSmallSpatial() krita.general: Elapsed time: 14 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallSpatial() krita.general: Elapsed time: 19 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallSpatial() krita.general: Elapsed time: 32 ms
PASS   : KisConvolutionPainterTest::testGaussianSmallSpatial()
QDEBUG : KisConvolutionPainterTest::testGaussianSmallFFTW() krita.general: Elapsed time: 25 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallFFTW() krita.general: Elapsed time: 29 ms
QDEBUG : KisConvolutionPainterTest::testGaussianSmallFFTW() krita.general: Elapsed time: 18 ms
PASS   : KisConvolutionPainterTest::testGaussianSmallFFTW()
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsSpatial() krita.general: Elapsed time: 12880 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsSpatial() krita.general: Elapsed time: 15797 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsSpatial() krita.general: Elapsed time: 16079 ms
PASS   : KisConvolutionPainterTest::testGaussianDetailsSpatial()
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsFFTW() krita.general: Elapsed time: 8851 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsFFTW() krita.general: Elapsed time: 8559 ms
QDEBUG : KisConvolutionPainterTest::testGaussianDetailsFFTW() krita.general: Elapsed time: 9339 ms
PASS   : KisConvolutionPainterTest::testGaussianDetailsFFTW()
rempt accepted this revision.May 30 2016, 10:43 AM
rempt edited edge metadata.

Looks good, and I've tested the usual things that use convolution and I couldn't see a difference, and nothing crashed :-)

This revision is now accepted and ready to land.May 30 2016, 10:43 AM
Closed by commit R37:535cb0e5f610: Speedup KisConvolutionWorkerFFT: avoid QVector detaching, use iterators (authored by kossebau). · Explain WhyMay 30 2016, 3:36 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 4077

View Options

libs/image/kis_convolution_worker_fft.h

Loading...