Update (remove/add) several files at a time

[MikaMika]

In a project with thousands of files and just a few hundreds to update, it would be preferable to have mpqcli handle all the files in a list in one command while the mpq archive is open, rather than (open delete close, open add close) for each file. At that point, re-creating the archive is faster, but wasteful. The -w, --overwrite option suggested in PR #115 would help (-f, --force could be an alias), but wouldn't completely solve the issue for multiple writes.

# Create proj.mpq with a thousand random files
# time 0m0,167s
mkdir proj
for (( i = 0 ; i < 1000 ; i++ )); do 
    echo $RANDOM > proj/file$i.txt
done
mpqcli create proj

# Update a hundred files in proj.mpq
# time 0m1,078s
for (( i = 0 ; i < 100 ; i++ )); do 
    echo $RANDOM > proj/file$i.txt
    mpqcli remove file$i.txt proj.mpq
    mpqcli add proj/file$i.txt proj.mpq
done

Tools like tar cf target.tar file1 file2 ... have the archive first then any number of files after. Another way of doing it would be like mv file1 file2 ... target where the target is always the last argument.

Having a few hundred files would make the command very large too. This could be solved by either:

• using a new option -l, --list TEXTFILE where each line would be a file path
• enabling recurse in case mpqcli add points at a directory

I'll see if I can manage to create a patch, but I'm not very familiar with the code and I wanted to have the issue laid out first.

[thomaslaurenson]

Thanks for writing such a clear explanation or the problem, providing some of the metrics, and some good examples of how other tools handle this. I will have to think a little about the problem, and also do some performance testing to see what the best solution would be.

More than happy if you want to attempt a PR, but I could also put it on my list to fix. But I am a little slow in development at the moment due to work/life stuff, so might be a little wait time.

[thomaslaurenson]

Just a quick comment to keep you in the loop. I just merged in #115, and made a new release in #152 . The useful part of this release (0.9.8) is the inclusion of the overwrite argument for the add subcommand.

I will make a new PR and see what performance I get from a variety of different methods. And compare to overwrite. Have not yet thought about the potential syntax for multiple files and the add subcommand, but I will do some thinking and research. Will keep you posted

[thomaslaurenson]

Just dumping some notes and stats from testing. Made a sample data set from a WoW version 1.9.0 base patch. Extracted:

ll sample/wow-patch_original | wc -l
14509
du -sh sample/wow-patch_original
542M    sample/wow-patch_original

Wrote (well, generated) a Python script to modify the files randomly, which resulted in ~20% change in files, randomly appending bytes. So the files timestamp and filesize were different. Second data set:

ll sample/wow-patch_modified | wc -l
14509
du -sh sample/wow-patch_modified
552M    sample/wow-patch_modified

The some testing...

1. Create MPQ

time ./build/bin/mpqcli create sample/wow-patch_original
real    0m34.211s
user    0m32.463s
sys     0m1.645s

time ./build/bin/mpqcli create sample/wow-patch_original &> /dev/null
real    0m28.209s
user    0m26.898s
sys     0m1.252s

• Approximately 30 seconds to create MPQ archive
• Faster when redirecting all output, this is because we do not get streaming "Added File" output 15,000 times

2. Brute force remove and add everything

time (while IFS= read -r file; do ./build/bin/mpqcli remove "$file" sample/wow-patch_original.mpq && ./build/bin/mpqcli add "sample/wow-patch_modified/$file" sample/wow-patch_original.mpq; done < sample/wow-patch_modified_files.txt)
real    16m45.660s
user    15m49.762s
sys     0m55.549s

• Very slow, 16 minutes to remove every file and add it back

3. Add "modified" files to dataset using new overwrite argument

time (while IFS= read -r file; do ./build/bin/mpqcli add --overwrite "sample/wow-patch_modified/$file" sample/wow-patch_original.mpq; done < sample/wow-patch_modified_files.txt)
real    8m26.739s
user    8m0.395s
sys     0m26.179s

• Processing time halved, but still terrible performance
• So this is not a solution that is valid, recreating would be much better, but like you mentioned, still wasteful

The bottleneck in the per-file approaches is obvious: StormLib opens and flushes the archive's internal tables (hash table, block table, (listfile), etc.) on every close. In the test data set, this is done ~750 times, which results in the crazy processing times.

From here, I can see a couple valid options:

• Like you said - ingest a text file of paths to add. Simple and fast. Would require user to create a list, which seems painful if working with a large number of files and even more painful if switching archives/datasets
• Refactor add subcommand to allow a directory as input. This is a nice feature addition, but doesn't do much for the performance, especially if you ingest the same folder that was originally used when creating. A folder with subset (and same directory structure) could be a good option here - but this is also cumbersome for the end user
• Refactor add subcommand to allow a directory as input AND include new logic for updating. For example, like rsync and the update feature, only add in files if they have a newer timestamp or different file size.

I really like option 3, but I need to think a little before implementing. I also want to research and see if there are easily leveraged bash tools/workflow for doing this outside of mpqcli (but guessing there is not, or it would require multiple MPQ operations and loose the performance benefit). I did a quick implementation and test, and the performance is very good.

time ./build/bin/mpqcli add sample/wow-patch_modified/ sample/wow-patch_original.mpq --update --overwrite
real    0m3.656s
user    0m3.061s
sys     0m0.567s

• 3 seconds for the same dataset
• ~750 files added

Side note for myself. When modifying the MPQ archive heavily, the archive can develop a lot of dead space due to data movement. It might be sensible to add in a compact subcommand to call SFileCompactArchive - or have some logic in the application to handle this.

[thomaslaurenson]

Dumping the patch modification script for reference:

#!/usr/bin/env python3
"""
generate_bench_data.py

Prepares two benchmark dataset directories from a real extracted MPQ archive.

Usage:
    python generate_bench_data.py <extracted_archive_dir>

Given an input directory (e.g. WoW-1.9.0-zhCN-patch/), the script produces:
    <input>_original/         -- exact copy, used to create the initial .mpq archive
    <input>_modified/         -- copy with every 20th file padded with random bytes
    <input>_modified_files.txt -- relative paths of the modified files, one per line

The outputs are placed next to the input directory, sharing the same parent.
"""

import argparse
import os
import random
import shutil
import sys
from pathlib import Path


def collect_files(root: Path) -> list[Path]:
    """Return all files under *root* as relative Paths, sorted alphabetically."""
    files = []
    for path in root.rglob("*"):
        if path.is_file():
            files.append(path.relative_to(root))
    files.sort(key=lambda p: str(p).lower())
    return files


def copy_tree(src: Path, dst: Path, files: list[Path]) -> None:
    """Copy each file in *files* (relative to *src*) into *dst*, preserving layout."""
    dst.mkdir(parents=True, exist_ok=True)
    for rel in files:
        src_file = src / rel
        dst_file = dst / rel
        dst_file.parent.mkdir(parents=True, exist_ok=True)
        shutil.copy2(src_file, dst_file)


def corrupt_files(modified_root: Path, selected: list[Path]) -> None:
    """Append random bytes to each selected file inside *modified_root*."""
    for rel in selected:
        target = modified_root / rel
        byte_count = random.randint(10_240, 20_480)
        with open(target, "ab") as fh:
            fh.write(os.urandom(byte_count))


def main() -> None:
    parser = argparse.ArgumentParser(
        description="Generate bench_original and bench_modified datasets from an extracted MPQ archive."
    )
    parser.add_argument(
        "extracted_dir",
        help="Path to the already-extracted MPQ archive directory.",
    )
    args = parser.parse_args()

    src = Path(args.extracted_dir).resolve()
    if not src.is_dir():
        print(f"[!] Not a directory: {src}", file=sys.stderr)
        sys.exit(1)

    parent = src.parent
    base = src.name
    original_dir = parent / f"{base}_original"
    modified_dir = parent / f"{base}_modified"
    modified_files_txt = parent / f"{base}_modified_files.txt"

    # 1. Collect all files, sorted deterministically
    print(f"[*] Collecting files from: {src}")
    all_files = collect_files(src)
    total = len(all_files)
    print(f"[*] Total files: {total}")

    # 2. Copy to _original/
    print(f"[*] Copying to: {original_dir}")
    copy_tree(src, original_dir, all_files)

    # 3. Copy to _modified/
    print(f"[*] Copying to: {modified_dir}")
    copy_tree(src, modified_dir, all_files)

    # 4. Select every 20th file (index 0, 20, 40, ...)
    changed = [f for i, f in enumerate(all_files) if i % 20 == 0]
    print(f"[*] Modifying every 20th file ({len(changed)} files) in: {modified_dir}")
    corrupt_files(modified_dir, changed)

    # 5. Write modified_files.txt
    with open(modified_files_txt, "w", encoding="utf-8") as fh:
        for rel in changed:
            # Use forward slashes for portability; callers can adjust if needed
            fh.write(str(rel).replace("\\", "/") + "\n")
    print(f"[*] Modified file list written to: {modified_files_txt}")

    # 6. Summary
    print()
    print("[=] Summary")
    print(f"    Total files  : {total}")
    print(f"    Modified     : {len(changed)}")
    print(f"    Original dir : {original_dir}")
    print(f"    Modified dir : {modified_dir}")
    print(f"    List file    : {modified_files_txt}")
    print()
    print("[=] Modified files:")
    for rel in changed:
        print(f"    {rel}")


if __name__ == "__main__":
    main()

[sjoblomj]

If I understand you correctly, the issue is that if you add a directory with n files, this is done: CreateMpqArchive(), n * AddFile() followed by CloseMpqArchive(). But when adding n files, these three operations are performed over and over n times: OpenMpqArchive(), AddFile() followed by CloseMpqArchive().

Did I understand that correct? Is it basically the opening and closing over and over that is the cause of the performance issue?

I've been briefly looking into extending the add subcommand so that it can take a list of files or directories as input, and add all of them (i.e. only one OpenMpqArchive() and CloseMpqArchive()). I have not finished the work and have not done any metrics of it, but if the approach sound useful, I'm happy to finish it and make it a PR. In that case, I think we need to swap the order of the arguments, so that the mpq file is given first followed by 1-n files to add (as opposed to now, when you give the file to add followed by the mpq file parameter). This would thus break the old commands that users have in their shell history, after they update. That'd be unfortunate, but I guess that's not the worst thing before v1.0.

[MikaMika]

Thanks for looking into this. It is indeed the repetition of opening and closing the archive for each added file that takes the most time.

The overwrite allows to cut the time needed in half because it's opening and closing half as many times, from 16 min to 8 min. Seeing this operation can go as low as 3 sec if only one open/close happens is impressive.

Personally, I am comparing which files are modified with a git diff, and creating a text file or a subset folder is easy for me, but might not be the case for every one. I didn't think something like rsync would be possible (file size, timestamp, sha1?), but that probably would work amazingly.

There is no need to break existing commands. Currently it only accept 2 argument, and any further argument is ignored (or errors). $1 is obviously the first, $2 is not just the second but also the last one. So if the last argument is always the archive, that would be retro-compatible while allowing new commands to insert more files before the last argument. Like mv file1 file2 file3 target

Sorry if I'm repeating myself or speaking gibberish, not feeling very confident on my communication skills here 😃 But you mostly got it. Awesome stuff!

[thomaslaurenson]

Just to confirm, based on everyones comments - I think we are all on the same page with the issue and the functionality.

So the two main options are:

1. Add subcommand ingests directory
2. Add subcommand ingest list of files

I personally like the directory solution because it is dynamic, and would probably be more useful for multiple users. The logic is quite simple, even with the "update" functionality similar to how rsync does it (check timestamp and file size). But adding hashing (e.g., SHA, or even CRC32) would be too slow I think and put us in the same scenario with too many operations. AFAIK, MPQ archive doesn't store any file hash so we would have to calculate on the fly (please correct me if I am wrong).

I have option 1 written up in a PR locally, but not had the time to clean it up. So I don't need any help with that. However, nothing is stopping us from adding option 2. It could be supplementary and the "higher accuracy and control" option, as it is a categorically correct list of files controlled by the user. @sjoblomj - Would be more than happy if this was added too, but no pressure.

[sjoblomj]

I'm a bit sceptical to only add in files if they have a newer timestamp or different file size. It sounds brittle to me. In StarCraft I for example, the arr\units.dat file is commonly changed in mods, as it contains things like unit hit points, properties of the unit, what graphics and weapons to use etc etc. The file always has the same size. And StarCraft I has no timestamps. So this approach wouldn't work at all for one of the most commonly changed files in StarCraft I.

If adding custom logic to detect if files are updated is a path you do want to go down, then maybe add a --add-only-if-changed flag to use that behaviour; and if the user didn't give the flag, then just add the files regardless (i.e. the current behaviour of mpqcli).

I also would find it cleaner and nicer to just specify what files to add in the parameters to mpqcli. Also seems more in unison with other cli tools like zip or tar. Can't say I'm a fan of putting the files to add in a text file and then adding that. :)

[thomaslaurenson]

Ok - based on feedback, this is the plan:

• Keep the proposed -u and --update arguments for the add subcommand. Document that it is fragile and how it operates. I like this, and it works in most cases.
• If the user wants more control... Update the positional arguments from file to files. Allows the user to ingest multiple files, or one using the add subcommand (similar to tar allows a list of files). Include support to read from stdin.
• This will break the usual structure of positional arguments, and target will have to go first. This requires documentation to let the end user know.
• While doing this, it makes sense to also add this same functionality to the remove subcommand
• This solution means we will not implement any support for a "list" (text file) of paths. But this could be a future option, because both tar and rsync have this functionality using the --files-from argument
• Going to wait for Refactor/linting #155 to be finished before work on this (that is a decent code base change)

CLI11 args example:

add_cmd->add_option("archive", archive_path)->required();
add_cmd->add_option("files", files)->expected(-1);

Then something like this:

add_cmd->callback([&]() {
    std::vector<std::string> resolved_files;
    
    for (const auto& f : files) {
        if (f == "-") {
            std::string line;
            while (std::getline(std::cin, line)) {
                if (!line.empty()) resolved_files.push_back(line);
            }
        } else {
            resolved_files.push_back(f);
        }
    }

    // open MPQ once, loop over resolved_files, close once
});

End user example:

mpqcli add archive.mpq file1.txt file2.txt
find . -name "*.blp" | mpqcli add archive.mpq -