Updater freezes screen when computing checksums

[lucasoares]

Priority

Low

Area

• Data
• Source
• Docker
• Other

What happened?

I'm using the updater and when executing it, the screen freezes until the checksum is generated in the resourcemanager.cpp. I see my disk usage increasing by the otclient process, and the client keeps totally freezed while it is generating it.

Ps.: usually this process is very fast, but since I'm running it in the sources folder, it is generating the checksum for all directories (including folders like build, VS, etc). But this should never freeze the client, right?

What OS are you seeing the problem on?

Windows

Code of Conduct

• I agree to follow this project's Code of Conduct

[kokekanon]

I reported it a long time ago, the performance error.

https://github.com/mehah/otclient/issues/578#issuecomment-1871494821

In v8, data.zip libzip is used,
otcr all the files are read, if you are testing in dev includes vcpkg_installed 3.85 gb.💀 (of course, it would not be used in production)

With my limited knowledge of C++ at that time, I thought of passing an array as an argument in the filesChecksums function with the files to update obtained from the update.php.
I suppose your suggestion of using .libzip is better.

[lucasoares]

I reported it a long time ago, the performance error.

https://github.com/mehah/otclient/issues/578#issuecomment-1871494821

In v8, data.zip libzip is used, otcr all the files are read, if you are testing in dev includes vcpkg_installed 3.85 gb.💀 (of course, it would not be used in production)

With my limited knowledge of C++ at that time, I thought of passing an array as an argument in the filesChecksums function with the files to update obtained from the update.php. I suppose your suggestion of using .libzip is better.

I changed my mind for now hahahaha. I started a implementation using zip file but I think this may be worse in some cases (like where only few files are changed), which maybe is the most common case.

Now I'm changing the updater to:

1. Do not search all files on otclient's folder /. It will only execute checksum on necessary folder/files
2. Execute the checksum check on c++ async

I'm using the following configuration for necessary folders: init.lua,data,modules,mods

The rest remains the same.

This improved the freezing time on the screen (didn't removed it all) and now it's under an acceptable time because it will complete faster.

Here's the code I changed on resourcemanager.cpp:

std::unordered_map<std::string, std::string> ResourceManager::filesChecksums(const std::vector<std::string>& paths)
{
    std::mutex mutex;
    std::unordered_map<std::string, std::string> ret;

    std::vector<std::future<void>> futures;
    futures.reserve(paths.size());

    try {
        for (const auto& path : paths) {
            futures.push_back(std::async(std::launch::async, [this, &ret, &mutex, &path, &futures]() {
                auto prefixedPath = path;
                if (path.front() != '/') {
                    prefixedPath = "/" + path;
                }

                if (!std::filesystem::is_directory(path)) {
                    {
                        std::lock_guard<std::mutex> lock(mutex);
                        ret[prefixedPath] = fileChecksum(path);
                    }

                    return;
                }

                auto directoryFiles = listDirectoryFiles(prefixedPath, true, false, true);

                std::vector<std::future<void>> fileFutures;
                fileFutures.reserve(directoryFiles.size() * 2);

                for (const auto& filePath : directoryFiles) {
                    fileFutures.push_back(std::async(std::launch::async, [this, &ret, &mutex, &filePath]() {
                        {
                            std::lock_guard<std::mutex> lock(mutex);
                            ret[filePath] = fileChecksum(filePath);
                        }
                    }));
                }

                for (auto& future : fileFutures) {
                    future.get();
                }
            }));
        }

        for (auto& future : futures) {
            future.get();
        }
    } catch (const std::exception& e) {
        g_logger.error(std::format("Error computing files checksum: {}: {}'",
            typeid(e).name(),
            e.what()
        ));

        return {};
    }

    return ret;
}

A better async implementation would be using callbacks like how the http is made, but that would require more changes.

It now receives the paths to search, which means the updater.lua must send the init.lua,data,modules,mods array.