Low performance when loading existing MKVs with `MKVFile`

[kokonguyen191]

Describe the solution you'd like

I was running a bulk cleanup operation that mostly consists of checking if MKVs are uniformly formatted (1 vid track, at most 2 audio track, all subtitles are english, etc.), so the workflow was quite straightforward: just iterate through everything, call MKVFile(path), check the tracks. But I noticed my HDD usage consistently stayed at a low level, indicating the process was spending time somewhere else when it should be IO bound instead.

So a bit of profiling I did, and I found out a lot of redundant shell requests with the same paths were sent to mkvmerge. For example, each track would call pymkv.MKVTrack.MKVTrack.track_id once, which in turn would make a call to mkvmerge then parse some JSON (I'm not sure if parsing JSON might be another point that could be improved since AFAIK, JSON has pretty crappy performance, especially when you don't need the entire object). All of these mkvmerge calls would run on the same mkvmerge_path and file_path when only the first one should have been enough and should be cached.

Here's a very crude comparison using my bandage patch:

Before
loaded 10 files in 9.9s
After
loaded 10 files in 1.4s

[GitBib]

I’ve been observing the library’s behavior for a while. The library isn’t very efficient with requests. I thought about how to improve efficiency, but decided to cover it with tests first to minimize the risk of breaking something major. Right now, for one file, “mkvmerge -J …” is called 35 times and “mkvmerge -V” is called 12 times.

[xMohnad]

It currently takes around 34.8310s to load 10 files.
We need to find a solution to optimize this.

[GitBib]

Hi, I will run tests, collect metrics, and look in more detail at who consumes the most time during work.

[xMohnad]

Hi GitBib,

I’ve been analyzing the performance issue with loading MKV files, and I’ve identified a couple of areas that are causing unnecessary overhead:

---

pymkv.MKVFile.__init__ – redundant support verification

Currently, this block is being executed every time:

if not verify_supported(file_path, mkvmerge_path=self.mkvmerge_path):
    msg = f"The file '{file_path}' is not a valid Matroska file or is not supported."
    raise ValueError(msg)

Problem: verify_supported is already effectively covered by other checks (verify_mkvmerge, checking_file_path, get_file_info).

Proposed solution: rely on get_file_info during initialization to determine support:

if not info_json["container"]["supported"]:
    msg = f"The file '{file_path}' is not a valid Matroska file or is not supported."
    raise ValueError(msg)

Simplified __init__ block:

if not verify_mkvmerge(mkvmerge_path=self.mkvmerge_path):
    msg = "mkvmerge is not at the specified path, add it there or change mkvmerge_path property"
    raise FileNotFoundError(msg)

if file_path is not None:
    file_path = checking_file_path(file_path)
    try:
        info_json = get_file_info(
            file_path,
            self.mkvmerge_path,
            check_path=False,
        )
        self._info_json = info_json
    except sp.CalledProcessError as e:
        error_output = e.output.decode()
        raise sp.CalledProcessError(
            e.returncode,
            e.cmd,
            output=error_output,
        ) from e

    if not info_json["container"]["supported"]:
        msg = f"The file '{file_path}' is not a valid Matroska file or is not supported."
        raise ValueError(msg)

---

pymkv.MKVTrack.file_path setter – unnecessary repeated support checks

Current implementation:

@file_path.setter
def file_path(self, file_path: str) -> None:
    fp = checking_file_path(file_path)
    if not verify_supported(fp, mkvmerge_path=self.mkvmerge_path):
        msg = f"The file '{file_path}' is not a valid Matroska file or is not supported."
        raise ValueError(msg)
    self._file_path = fp
    self.track_id = 0

Issues:

• Tracks in MKV files are already part of a valid Matroska container. There is no need to verify support for these tracks individually.
• Running verify_supported here unnecessarily triggers mkvmerge calls and adds overhead.

Suggested approach:

• Skip verify_supported entirely for tracks that already exist in the MKV file.
• Only perform verification for new files being added, and even then, mkvmerge -J is sufficient; no need for mkvmerge -V.

These changes should significantly reduce redundant mkvmerge calls and improve loading speed, especially for bulk operations.

---

I was planning to open a PR with these changes, but it may conflicts with PR #75.
Once that PR is merged (or rebased), I can adjust my branch accordingly and submit the PR.

[GitBib]

Thank you for taking the time to analyze the problem. I will look into these points and collect more data. Since I use a separate worker, I have never measured the execution time.

[GitBib]

Implemented performance optimizations in #81 to address this issue:

Cache verify_mkvmerge() - avoid redundant mkvmerge -V calls
Remove duplicate verify_supported() from MKVFile.init
Skip verification for tracks with existing_info
Add profiling support with pyinstrument