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

Branch

speedupConvolution

Lint

No Linters Available

Unit

No Unit Test Coverage

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.

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()

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

			Path	Packages
M			libs/image/kis_convolution_worker_fft.h (89 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	4060	f0cbffc		May 30 2016, 9:34 AM	★	★
Diff 2	4077	5f02b6c	rKRITA535cb0e5f6100b85bd0504dd0803067f7b4c42ff	May 30 2016, 3:35 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
23fb664e1c22	3219410b8082	f0cbffcb6bdd	Friedrich W. H. Kossebau	Speedup KisConvolutionWorkerFFT: avoid QVector detaching, use iterators (Show More…)	May 30 2016, 9:25 AM

Diff 4060

View Options

libs/image/kis_convolution_worker_fft.h