Skip to content

Conversation

@kleuter
Copy link

@kleuter kleuter commented Sep 2, 2025

Description

  • Reuses existing progress callback to make the library more responsive when opening an archive with containing tens of thousands of entries—previously the library remained unresponsive until enumeration finished. This required some of existing classes (BitAbstractArchiveHandler, BitAbstractArchiveOpener, BitArchiveReader ...) constructors to accept ProgressCallback as the last OPTIONAL parameter
  • Reuses existing progress callback to solve freezes during extraction of very large files by allowing progress-driven cancelation. New ProgressOutStream class that that acts as a thin wrapper around any IOutStream, tracking how many bytes have been written and invoking a user-supplied ProgressCallback after each write.

How Has This Been Tested?

  • Builts fine with CMake
  • Opened large archives while observing progress updates and confirming the ability to cancel early.
  • Extracted multi‑gigabyte files and verified the application stayed responsive and could abort mid-extraction.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@rikyoz
Copy link
Owner

rikyoz commented Sep 5, 2025

Hi!

First of all, thank you for the PR!
And sorry for the late reply, I have only just found the time to test your changes.

  • Reuses existing progress callback to make the library more responsive when opening an archive with containing tens of thousands of entries—previously the library remained unresponsive until enumeration finished. This required some of existing classes (BitAbstractArchiveHandler, BitAbstractArchiveOpener, BitArchiveReader ...) constructors to accept ProgressCallback as the last OPTIONAL parameter

There are a few problems with the current implementation, some of which I just discovered.
As I said, my idea was to reuse the ProgressCallback, but I never got the chance to test it before:

  • First, 7-Zip seems to unreliably pass either files or bytes (or both) to IArchiveOpenCallback::SetCompleted. Which one it passes depends on the archive format.
    • For example, for the Zip format, 7-Zip passes files but not bytes.
    • This means that we cannot simply pass *bytes to ProgressCallback.
    • I fear that we cannot reuse ProgressCallback and have to introduce a new callback (OpenProgressCallback?) which passes the two pointers to the user. It would allow the user to decide how to handle the data it receives.
  • Secondly, a lonely progress callback without a corresponding total callback is not much useful, as you cannot compare the current progress with the total files/bytes to be read during the opening process.

I did a quick experiment by adding a total callback:

std::uint64_t total = 0;
BitArchiveReader reader{ lib, BIT7Z_STRING( R"(E:\Downloads\bit7z.zip)" ), BitFormat::Auto, {},
    [&total](const std::uint64_t* totFiles, const std::uint64_t* /*totBytes*/){
        total = *totFiles;
    },
    [&total](const std::uint64_t* progressFiles, const std::uint64_t* /*progresBytes*/){
        std::println("[Opening] {} of {} ({:.2f}%)", *progressFiles, total, (100.0 * *progressFiles) / total);
        return true;
    }
};
const auto itemsCount = reader.itemsCount();
std::println("[Finished] {} of {} ({:.2f}%)", itemsCount, total, (100.0 * itemsCount) / total);
Screenshot 2025-09-04 225230

Notice that in this case 7-Zip doesn't call the progress callback on completion, so I had to manually add the print after the constructor.

As a side note, there are some archive formats that do not communicate via IArchiveOpenCallback at all; one noteworthy example of such formats is 7z.
In these cases, there's not much we can do at bit7z level.

  • Reuses existing progress callback to solve freezes during extraction of very large files by allowing progress-driven cancelation. New ProgressOutStream class that that acts as a thin wrapper around any IOutStream, tracking how many bytes have been written and invoking a user-supplied ProgressCallback after each write.

I was actually thinking of reusing the progress callback only for opening, not for the write callback.

The problem with this part of your PR is that it breaks the current library behavior, and thus can break the behavior of existing programs that use it.

Take this example program that prints the extraction progress:

std::uint64_t total = 0;
BitArchiveReader reader{ lib, BIT7Z_STRING( R"(E:\Downloads\bit7z.7z)" ) };
reader.setTotalCallback([&total](const std::uint64_t tot) {
    total = tot;
})
reader.setProgressCallback([&total](const std::uint64_t progress){
    std::println("[Extracting] {} of {} ({:.2f}%)", progress, total, (100.0 * progress) / total);
    return true;
});
reader.extractTo(BIT7Z_STRING("./bit7z-extracted"));

The expected/current behavior is the following:

image

while the behavior with your changes is the following:

image

The ProgressOutStream idea is nice, though, as it reduces the overhead, at least in part.
Maybe we could introduce something like a CurrentFileProgressCallback.
But I need to evaluate whether it's ok to store such callback in BitAbstractArchiveHandler, as this class already stores too many callbacks.

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