D23505: added sorting benchmark

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> meven wrote in CMakeLists.txt:42
> Make it optional to build benchmark, perhaps in its own folder.

Why? We already run another benchmark (kfileitemmodelbenchmark) unconditionally.

> dolphinsortingbenchmark.cpp:30
> +    {
> +        QFile f(QFINDTESTDATA("data/usr_bin.txt"));
> +        bool isOpen = f.open(QFile::ReadOnly | QFile::Text);

Use descriptive variable names (e.g. `file`).

> dolphinsortingbenchmark.cpp:31
> +        QFile f(QFINDTESTDATA("data/usr_bin.txt"));
> +        bool isOpen = f.open(QFile::ReadOnly | QFile::Text);
> +        QVERIFY(isOpen);

Missing `const`

> dolphinsortingbenchmark.cpp:34-35
> +        QStringList programs;
> +        while(!f.atEnd())
> +            programs << f.readLine();
> +        QTest::addRow("usr_bin") << programs;

Missing braces

> dolphinsortingbenchmark.cpp:56
> +    }
> +    qDebug() << std::is_sorted(v.begin(), v.end()); // this is intended as an optimization barrier
> +}

Not sure what you mean with optimizazion barrier. Can you explain why there is no QVERIFY here?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D23505

To: fabiank, #dolphin, elvisangelaccio
Cc: meven, kfm-devel, pberestov, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200119/378d838c/attachment.htm>