D8059: KDevelop: abstractfilemanagerimport benchmark

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  the commit message should include some output numbers and document how to run this test with different backends. I.e. something like the following should be the desired commit message _and_ also shows the desired output from the benchmark itself:
  
    On my (linux) machine, running the benchmark on the kdevelop master repo, I measure the following:
    
    KDIRWATCH_METHOD=inotify ./kdevplatform/project/tests/abstractfilemanagerpluginimportbenchmark ~/projects/kf5/src/extragear/kdevelop/kdevelop/
    KDirWatch backend: Inotify
    Benchmark set-up took 0.152 seconds
    Starting import of project "/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop"
    creating dirwatcher took 0.021 seconds
    imported project 0 with 5611 elements (XXX files & YYY folders) took 0.062 seconds
    Done in 0.236 seconds total (excluding dirwatcher destruction)
    Closing project
    Done in XYZ seconds total
    
    KDIRWATCH_METHOD=QFSWatch ./kdevplatform/project/tests/abstractfilemanagerpluginimportbenchmark ~/projects/kf5/src/extragear/kdevelop/kdevelop/
    KDirWatch backend: QFSWatch
    Benchmark set-up took 0.181 seconds
    Starting import of project "/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop"
    creating dirwatcher took 0.14 seconds
    imported project 0 with 5611 elements (XXX files & YYY folders) took 0.066 seconds
    Done in 0.388 seconds total (excluding dirwatcher destruction)
    Closing project
    Done in XYZ seconds total
  
  Note that the number of calls to `addDir` is completely useless output (it's an implementation detail). What is interesting, is the total time only.
  
  Note also how for me, the QSFWatch dirwatcher creation only takes 0.14s for the kdevelop project, which is still pretty fast. Using FAM takes ages though. Please do add the numbers for your mac machine, too.

INLINE COMMENTS

> projectwatcher.cpp:39
> +{
> +    if (m_filter->isValid(Path(path), true, m_project) && !contains(path)) {
> +        KDirWatch::addDir(path, watchModes);

this filter check here is conceptually different functionality compared to before. I'm not saying it's a bad change, but it makes this patch do more than just adding a benchmark. Please split your patches up to make it easier to review _and_ to keep the history sane.

Splitting up this patch that way will also allow us to assess the impact of this change.

> abstractfilemanagerpluginimportbenchmark.cpp:44
> +
> +class AFMPBenchmark : public QObject
> +{

Name the class after the file: `AbstractFileManagerPluginImportBenchmark`

> abstractfilemanagerpluginimportbenchmark.cpp:58
> +        m_projectNumber = s_numBenchmarksRunning++;
> +        qInfo() << "Starting import of project" << m_project->path();
> +        m_timer.start();

here and below: don't use qInfo, it's usually not enabled. Instead, use `qDebug` or better yet use a `QTextStream m_out` member and initialize it with `m_out(stdout)`.

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171019/3d9447e2/attachment-0001.html>