Skip to content

BUGFIX: File Codec on Windows#507

Merged
easymodo merged 3 commits into
easymodo:devfrom
zymelaii:fix/file_codec
Jan 2, 2024
Merged

BUGFIX: File Codec on Windows#507
easymodo merged 3 commits into
easymodo:devfrom
zymelaii:fix/file_codec

Conversation

@zymelaii
Copy link
Copy Markdown

current method to resolve path is not corret due to encoding issues,
simply using QDir/QFileInfo to replace std::filesystem and std::string.
1. reserve parent dir entry
2. exclude current dir entry
3. exclude hidden entry on non-windows os
@easymodo
Copy link
Copy Markdown
Owner

Hi
Looks like this implementation is quite a bit slower, up to 4x
Here's a directory with 9k files

master branch
sorting: date

DirectoryManager::addEntriesFromDirectory() 59 ms.
DirectoryManager::sortEntryLists() 0 ms.

sorting: name

DirectoryManager::addEntriesFromDirectory() 57 ms.
DirectoryManager::sortEntryLists() 40 ms.

your PR
sorting: date

DirectoryManager::addEntriesFromDirectory() 113 ms.
DirectoryManager::sortEntryLists() 110 ms.

sorting: name

DirectoryManager::addEntriesFromDirectory() 111 ms.
DirectoryManager::sortEntryLists() 53 ms.

I'll poke it around more once I get home

@zymelaii
Copy link
Copy Markdown
Author

This is indeed a problem, let me see if I can find a better solution.

Thanks for your profiling, my dear easymodo.

@zymelaii
Copy link
Copy Markdown
Author

Hi Looks like this implementation is quite a bit slower, up to 4x Here's a directory with 9k files

master branch sorting: date

DirectoryManager::addEntriesFromDirectory() 59 ms.
DirectoryManager::sortEntryLists() 0 ms.

sorting: name

DirectoryManager::addEntriesFromDirectory() 57 ms.
DirectoryManager::sortEntryLists() 40 ms.

your PR sorting: date

DirectoryManager::addEntriesFromDirectory() 113 ms.
DirectoryManager::sortEntryLists() 110 ms.

sorting: name

DirectoryManager::addEntriesFromDirectory() 111 ms.
DirectoryManager::sortEntryLists() 53 ms.

I'll poke it around more once I get home

I run a benchmark on the two cases as below, with 10000 empty files named after random uuids under D:\\foobar.dir, it shows little difference between the master and my pr.

----------------------------------------------------------------------
Benchmark                            Time             CPU   Iterations
----------------------------------------------------------------------
OLD_addEntriesFromDirectory   21220853 ns     21354167 ns           30
NEW_addEntriesFromDirectory   19721743 ns     19425676 ns           37
OLD_sortByName                21681330 ns     20833333 ns           30
NEW_sortByName                19022935 ns     19301471 ns           34
OLD_sortByDate                21413357 ns     21354167 ns           30
NEW_sortByDate                19097041 ns     18841912 ns           34
#include <benchmark/benchmark.h>
#include <QRegularExpressionMatch>
#include <QDateTime>
#include <QDir>
#include <QCollator>
#include <vector>
#include <filesystem>

namespace fs = std::filesystem;

struct FSEntryOld {
    QString            path, name;
    std::uintmax_t     size;
    fs::file_time_type modifyTime;
    bool               isDirectory;
};

struct FSEntryNew {
    QString        path, name;
    std::uintmax_t size;
    QDateTime      modifyTime;
    bool           isDirectory;
};

void loadEntryList__old(
    std::vector<FSEntryOld>& fileEntryVec,
    std::vector<FSEntryOld>& dirEntryVec,
    QString                  directoryPath) {
    QRegularExpressionMatch match;
    for (const auto& entry :
         fs::directory_iterator(directoryPath.toStdWString())) {
        QString name =
            QString::fromStdString(entry.path().filename().generic_string());
        QString path = QString::fromStdString(entry.path().generic_string());
        if (entry.is_directory()) { // this can still throw std::bad_alloc ..
            FSEntryOld newEntry;
            try {
                newEntry.name        = name;
                newEntry.path        = path;
                newEntry.isDirectory = true;
                // newEntry.size = entry.file_size();
                // newEntry.modifyTime = entry.last_write_time();
            } catch (const std::filesystem::filesystem_error& err) {
                qDebug() << "[DirectoryManager]" << err.what();
                continue;
            }
            dirEntryVec.emplace_back(newEntry);
        } else if (match.hasMatch()) {
            FSEntryOld newEntry;
            try {
                newEntry.name        = name;
                newEntry.path        = path;
                newEntry.isDirectory = false;
                newEntry.size        = entry.file_size();
                newEntry.modifyTime  = entry.last_write_time();
            } catch (const std::filesystem::filesystem_error& err) {
                qDebug() << "[DirectoryManager]" << err.what();
                continue;
            }
            fileEntryVec.emplace_back(newEntry);
        }
    }
}

void loadEntryList__new(
    std::vector<FSEntryNew>& fileEntryVec,
    std::vector<FSEntryNew>& dirEntryVec,
    QString                  directoryPath) {
    QDir root(directoryPath);
    root.setFilter(QDir::Dirs | QDir::Files | QDir::NoDot);

    QRegularExpressionMatch match{};
    for (const auto& entry : root.entryInfoList()) {
        if (!entry.isDir() && !match.hasMatch()) { continue; }
        FSEntryNew newEntry{};
        newEntry.name        = entry.fileName();
        newEntry.path        = entry.absoluteFilePath();
        newEntry.isDirectory = entry.isDir();
        if (newEntry.isDirectory) {
            dirEntryVec.emplace_back(newEntry);
        } else {
            newEntry.size       = entry.size();
            newEntry.modifyTime = entry.lastModified();
            fileEntryVec.emplace_back(newEntry);
        }
    }
}

static void OLD_addEntriesFromDirectory(benchmark::State& state) {
    for (auto _ : state) {
        std::vector<FSEntryOld> fileEntryVec;
        std::vector<FSEntryOld> dirEntryVec;
        loadEntryList__old(
            fileEntryVec, dirEntryVec, QStringLiteral("D:\\foobar.dir"));
    }
}

static void NEW_addEntriesFromDirectory(benchmark::State& state) {
    for (auto _ : state) {
        std::vector<FSEntryNew> fileEntryVec;
        std::vector<FSEntryNew> dirEntryVec;
        loadEntryList__new(
            fileEntryVec, dirEntryVec, QStringLiteral("D:\\foobar.dir"));
    }
}

static void OLD_sortByName(benchmark::State& state) {
    QCollator collator;
    for (auto _ : state) {
        std::vector<FSEntryOld> fileEntryVec;
        std::vector<FSEntryOld> dirEntryVec;
        loadEntryList__old(
            fileEntryVec, dirEntryVec, QStringLiteral("D:\\foobar.dir"));
        std::sort(
            fileEntryVec.begin(),
            fileEntryVec.end(),
            [&collator](const auto& lhs, const auto& rhs) {
                return collator.compare(lhs.path, lhs.path) < 0;
            });
    }
}

static void NEW_sortByName(benchmark::State& state) {
    QCollator collator;
    for (auto _ : state) {
        std::vector<FSEntryNew> fileEntryVec;
        std::vector<FSEntryNew> dirEntryVec;
        loadEntryList__new(
            fileEntryVec, dirEntryVec, QStringLiteral("D:\\foobar.dir"));
        std::sort(
            fileEntryVec.begin(),
            fileEntryVec.end(),
            [&collator](const auto& lhs, const auto& rhs) {
                return collator.compare(lhs.path, lhs.path) < 0;
            });
    }
}

static void OLD_sortByDate(benchmark::State& state) {
    QCollator collator;
    for (auto _ : state) {
        std::vector<FSEntryOld> fileEntryVec;
        std::vector<FSEntryOld> dirEntryVec;
        loadEntryList__old(
            fileEntryVec, dirEntryVec, QStringLiteral("D:\\foobar.dir"));
        std::sort(
            fileEntryVec.begin(),
            fileEntryVec.end(),
            [&collator](const auto& lhs, const auto& rhs) {
                return lhs.modifyTime < rhs.modifyTime;
            });
    }
}

static void NEW_sortByDate(benchmark::State& state) {
    QCollator collator;
    for (auto _ : state) {
        std::vector<FSEntryNew> fileEntryVec;
        std::vector<FSEntryNew> dirEntryVec;
        loadEntryList__new(
            fileEntryVec, dirEntryVec, QStringLiteral("D:\\foobar.dir"));
        std::sort(
            fileEntryVec.begin(),
            fileEntryVec.end(),
            [&collator](const auto& lhs, const auto& rhs) {
                return lhs.modifyTime < rhs.modifyTime;
            });
    }
}

BENCHMARK(OLD_addEntriesFromDirectory);
BENCHMARK(NEW_addEntriesFromDirectory);
BENCHMARK(OLD_sortByName);
BENCHMARK(NEW_sortByName);
BENCHMARK(OLD_sortByDate);
BENCHMARK(NEW_sortByDate);

BENCHMARK_MAIN();

@easymodo easymodo changed the base branch from master to dev January 2, 2024 20:25
@easymodo easymodo merged commit 0420a51 into easymodo:dev Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants