dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kcoredirlister_benchmark.cpp:244
> + uint hash=qHash(url);
> + auto it = std::lower_bound(lstItems.cbegin(), lstItems.cend(), hash,
> lessThanHash);
> + if (it != lstItems.cend() && it->item->url() == url) {
As discussed in https://phabricator.kde.org/D10742, this doesn't handle the
case of hash collisions (this code assumes each URL gets a unique hash). So
this commit needs to be updated as well, possibly by just removing this class
if making it right is too much effort, for too slow code in the end?
> kcoredirlister_benchmark.cpp:329
> +
> +template <class T> void createFiles(int number)
> +{
`number` is always 50 so you could remove it as a parameter in all these
methods and just use a static const int for instance.
It's more dangerous to have it as a method parameter: if one caller passes a
different number, the benchmarks won't be comparable anymore...
> kcoredirlister_benchmark.cpp:376
> + QBENCHMARK {
> + for (int i=0; i<5000; i++) {
> + data.findByUrl(u1);
QBENCHMARK takes care of repeating as many times as necessary, so this for loop
isn't needed, is it?
> kcoredirlister_benchmark.cpp:384
> +
> QUrl::fromLocalFile(QLatin1String("/home/user/Folder1/SubFolder2/a4505.txt")));
> +
> QCOMPARE(data.findByUrl(QUrl::fromLocalFile(QLatin1String("/home/user/Folder1/SubFolder2/a4505.txt")))->url(),
> +
> QUrl::fromLocalFile(QLatin1String("/home/user/Folder1/SubFolder2/a4505.txt")));
copy/paste errors? This is looking up the same URL three times.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D12945
To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
