Repository: mido/mido

Issue:
merge_tracks is slow
`mido.merge_tracks` is slow. This is because we are doing a lot of data checks when we copy messages. Since the messages that are being flattened into a single track have already been checked, I think these additional checks are unnecessary (and inefficient). 

```python
import mido
import cProfile, pstats, io, time
from pstats import SortKey

mf = mido.MidiFile()

for k in range(5):
    t = mido.MidiTrack()
    for _ in range(10000):
        msg = mido.Message("note_on", note=72, time=1000 + 100 * k)
        msg = mido.Message("note_off", note=72, time=500 + 100 * k)
        t.append(msg)
    mf.tracks.append(t)

with cProfile.Profile() as pr:
    now = time.time()
    i = 0
    for _ in mido.merge_tracks(mf.tracks):
        i += 1

    print(i)
    print(time.time() - now)
    s = io.StringIO()
    sortby = SortKey.TIME
    ps = pstats.Stats(pr, stream=s).sort_stats(sortby)
    ps.print_stats()
    print(s.getvalue())
```
Profiled on the current implementation:
```
(mido) ➜  mido git:(faster-merge-tracks) ✗ python -O examples/benchmark.py
50001
0.9691178798675537
         7300036 function calls in 0.965 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
   200000    0.146    0.000    0.735    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:88(check_msgdict)
  1000000    0.125    0.000    0.572    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:84(check_value)
   800001    0.089    0.000    0.215    0.000 {built-in method _abc._abc_instancecheck}
   800002    0.083    0.000    0.354    0.000 {built-in method builtins.isinstance}
   600000    0.081    0.000    0.081    0.000 {built-in method _abc._abc_subclasscheck}
   100000    0.073    0.000    0.923    0.000 /Users/intern/workspace/forks/mido/mido/messages/messages.py:117(copy)
   800001    0.057    0.000    0.272    0.000 /Users/intern/anaconda3/envs/mido/lib/python3.9/abc.py:117(__instancecheck__)
   600000    0.044    0.000    0.125    0.000 /Users/intern/anaconda3/envs/mido/lib/python3.9/abc.py:121(__subclasscheck__)
   400000    0.040    0.000    0.224    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:53(check_data_byte)
   100000    0.040    0.000    0.478    0.000 /Users/intern/workspace/forks/mido/mido/messages/messages.py:110(__init__)
   100000    0.032    0.000    0.041    0.000 /Users/intern/workspace/forks/mido/mido/messages/specs.py:110(make_msgdict)
   300000    0.025    0.000    0.025    0.000 {method 'update' of 'dict' objects}
   200000    0.023    0.000    0.145    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:11(check_channel)
   200001    0.018    0.000    0.066    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:60(check_time)
    50001    0.014    0.000    0.479    0.000 /Users/intern/workspace/forks/mido/mido/midifiles/tracks.py:71(_to_reltime)
   200002    0.013    0.000    0.013    0.000 {built-in method builtins.vars}
   200000    0.012    0.000    0.012    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:6(check_type)
    50005    0.012    0.000    0.470    0.000 /Users/intern/workspace/forks/mido/mido/midifiles/tracks.py:63(_to_abstime)
   200001    0.010    0.000    0.010    0.000 {method 'items' of 'dict' objects}
   200000    0.008    0.000    0.008    0.000 {method 'get' of 'dict' objects}
    50002    0.007    0.000    0.486    0.000 /Users/intern/workspace/forks/mido/mido/midifiles/tracks.py:80(fix_end_of_track)
...
```
If we change the `copy` calls in `_to_abstime`, _to_reltime`, and `fix_end_of_track` to exclude the `overrides` (and thus bypass all checks), the speed up is around 5x.

```
(mido) ➜  mido git:(faster-merge-tracks) ✗ python -O examples/benchmark.py
50001
0.17537927627563477
         1500036 function calls in 0.171 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
   100000    0.040    0.000    0.063    0.000 /Users/intern/workspace/forks/mido/mido/messages/messages.py:117(copy)
   100000    0.023    0.000    0.072    0.000 /Users/intern/workspace/forks/mido/mido/messages/messages.py:191(_setattr)
   400002    0.018    0.000    0.018    0.000 {built-in method builtins.vars}
    50001    0.012    0.000    0.081    0.000 /Users/intern/workspace/forks/mido/mido/midifiles/tracks.py:75(_to_reltime)
   100002    0.011    0.000    0.024    0.000 {built-in method builtins.isinstance}
    50005    0.011    0.000    0.077    0.000 /Users/intern/workspace/forks/mido/mido/midifiles/tracks.py:65(_to_abstime)
   100000    0.010    0.000    0.041    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:84(check_value)
   100000    0.008    0.000    0.008    0.000 {method 'update' of 'dict' objects}
   100001    0.007    0.000    0.031    0.000 /Users/intern/workspace/forks/mido/mido/messages/checks.py:60(check_time)
   100001    0.007    0.000    0.012    0.000 /Users/intern/anaconda3/envs/mido/lib/python3.9/abc.py:117(__instancecheck__)
   100001    0.006    0.000    0.006    0.000 {built-in method _abc._abc_instancecheck}
   100000    0.006    0.000    0.006    0.000 {built-in method __new__ of type object at 0x1027ab388}
    50002    0.005    0.000    0.086    0.000 /Users/intern/workspace/forks/mido/mido/midifiles/tracks.py:86(fix_end_of_track)
```

The three isolated changes in `tracks.py` each look like
```diff
 class MidiTrack(list):
     @property
@@ -65,7 +67,9 @@ def _to_abstime(messages):
     now = 0
     for msg in messages:
         now += msg.time
-        yield msg.copy(time=now)
+        new_msg = msg.copy()
+        new_msg.time = now
+        yield new_msg
```
I think the project prefers to treat `Message` objects as immutable ... So if this is a problem, maybe it would be better for an `_unsafe_copy` method or `skip_checks` boolean argument for  `copy`. 

Discussion:
almostimplemented: > I'm not a big fan of skipping checks by default in the `tracks.py` methods.

I think you are right -- I will change the default to enforce checks and only pass the skip flag internally.

> Also, in `messages.py`, shouldn't we check if we get `overrides` regardless of the `skip_checks` status?

Oof, duh! What an awful mistake. Fixed.
almostimplemented: For reference, the `benchmark.py` script in #472 takes 0.913 seconds on `main` and 0.175 seconds on the PR branch.
rdoursenaud: @almostimplemented Could you please review the test error and fix the merging conflicts. You may want to rebase upon the current main as I have fixed a bunch of GitHub workflow test errors. Thanks!
rdoursenaud: @akx Yeah, I'm pondering moving to dedicated objects instead, like what CircuitPython's [adafruit_midi](https://docs.circuitpython.org/projects/midi/en/latest/api.html).

I'm gathering all insights for v2 in https://github.com/orgs/mido/discussions/540

Patch:
diff --git a/mido/messages/messages.py b/mido/messages/messages.py
index ff82681..9f760a4 100644
--- a/mido/messages/messages.py
+++ b/mido/messages/messages.py
@@ -111,19 +111,25 @@ class SysexData(tuple):
 
 
 class Message(BaseMessage):
-    def __init__(self, type, **args):
+    def __init__(self, type, skip_checks=False, **args):
         msgdict = make_msgdict(type, args)
         if type == 'sysex':
             msgdict['data'] = SysexData(msgdict['data'])
-        check_msgdict(msgdict)
+
+        if not skip_checks:
+            check_msgdict(msgdict)
+
         vars(self).update(msgdict)
 
-    def copy(self, **overrides):
+    def copy(self, skip_checks=False, **overrides):
         """Return a copy of the message.
 
         Attributes will be overridden by the passed keyword arguments.
         Only message specific attributes can be overridden. The message
         type can not be changed.
+
+        The skip_checks arg can be used to bypass validation of message
+        attributes and should be used cautiously.
         """
         if not overrides:
             # Bypass all checks.
@@ -139,8 +145,11 @@ class Message(BaseMessage):
 
         msgdict = vars(self).copy()
         msgdict.update(overrides)
-        check_msgdict(msgdict)
-        return self.__class__(**msgdict)
+
+        if not skip_checks:
+            check_msgdict(msgdict)
+
+        return self.__class__(skip_checks=skip_checks, **msgdict)
 
     @classmethod
     def from_bytes(cl, data, time=0):
diff --git a/mido/midifiles/midifiles.py b/mido/midifiles/midifiles.py
index 3ee2847..cd3be20 100644
--- a/mido/midifiles/midifiles.py
+++ b/mido/midifiles/midifiles.py
@@ -327,7 +327,7 @@ class MidiFile:
             raise TypeError("can't merge tracks in type 2 (asynchronous) file")
 
         if self._merged_track is None:
-            self._merged_track = merge_tracks(self.tracks)
+            self._merged_track = merge_tracks(self.tracks, skip_checks=True)
         return self._merged_track
 
     @merged_track.deleter
@@ -396,7 +396,7 @@ class MidiFile:
             else:
                 delta = 0
 
-            yield msg.copy(time=delta)
+            yield msg.copy(skip_checks=True, time=delta)
 
             if msg.type == 'set_tempo':
                 tempo = msg.tempo
diff --git a/mido/midifiles/tracks.py b/mido/midifiles/tracks.py
index 17caccc..871f9bd 100644
--- a/mido/midifiles/tracks.py
+++ b/mido/midifiles/tracks.py
@@ -64,24 +64,24 @@ class MidiTrack(list):
         return f'{self.__class__.__name__}({messages})'
 
 
-def _to_abstime(messages):
+def _to_abstime(messages, skip_checks=False):
     """Convert messages to absolute time."""
     now = 0
     for msg in messages:
         now += msg.time
-        yield msg.copy(time=now)
+        yield msg.copy(skip_checks=skip_checks, time=now)
 
 
-def _to_reltime(messages):
+def _to_reltime(messages, skip_checks=False):
     """Convert messages to relative time."""
     now = 0
     for msg in messages:
         delta = msg.time - now
-        yield msg.copy(time=delta)
+        yield msg.copy(skip_checks=skip_checks, time=delta)
         now = msg.time
 
 
-def fix_end_of_track(messages):
+def fix_end_of_track(messages, skip_checks=False):
     """Remove all end_of_track messages and add one at the end.
 
     This is used by merge_tracks() and MidiFile.save()."""
@@ -95,7 +95,7 @@ def fix_end_of_track(messages):
         else:
             if accum:
                 delta = accum + msg.time
-                yield msg.copy(time=delta)
+                yield msg.copy(skip_checks=skip_checks, time=delta)
                 accum = 0
             else:
                 yield msg
@@ -103,16 +103,25 @@ def fix_end_of_track(messages):
     yield MetaMessage('end_of_track', time=accum)
 
 
-def merge_tracks(tracks):
+def merge_tracks(tracks, skip_checks=False):
     """Returns a MidiTrack object with all messages from all tracks.
 
     The messages are returned in playback order with delta times
     as if they were all in one track.
+
+    Pass skip_checks=True to skip validation of messages before merging.
+    This should ONLY be used when the messages in tracks have already
+    been validated by mido.checks.
     """
     messages = []
     for track in tracks:
-        messages.extend(_to_abstime(track))
+        messages.extend(_to_abstime(track, skip_checks=skip_checks))
 
     messages.sort(key=lambda msg: msg.time)
 
-    return MidiTrack(fix_end_of_track(_to_reltime(messages)))
+    return MidiTrack(
+        fix_end_of_track(
+            _to_reltime(messages, skip_checks=skip_checks),
+            skip_checks=skip_checks,
+        )
+    )


Test patch:
diff --git a/tests/midifiles/test_tracks.py b/tests/midifiles/test_tracks.py
index b28e88b..f48f2ed 100644
--- a/tests/midifiles/test_tracks.py
+++ b/tests/midifiles/test_tracks.py
@@ -3,7 +3,9 @@
 # SPDX-License-Identifier: MIT
 
 import itertools
+import time
 
+import mido
 from mido.messages import Message
 from mido.midifiles.meta import MetaMessage
 from mido.midifiles.tracks import MidiTrack
@@ -35,3 +37,23 @@ def test_track_repr():
     track_eval = eval(repr(track))
     for m1, m2 in zip(track, track_eval):
         assert m1 == m2
+
+
+def test_merge_large_midifile():
+    mid = mido.MidiFile()
+    for k in range(5):
+        t = mido.MidiTrack()
+        for _ in range(10000):
+            t.append(mido.Message("note_on", note=72, time=1000 + 100 * k))
+            t.append(mido.Message("note_off", note=72, time=500 + 100 * k))
+        mid.tracks.append(t)
+
+    start = time.time()
+    merged = list(mido.merge_tracks(mid.tracks, skip_checks=True))
+    finish = time.time()
+
+    merged_duration_ticks = sum(msg.time for msg in merged)
+    max_track_duration_ticks = max(
+        sum(msg.time for msg in t) for t in mid.tracks)
+    assert merged_duration_ticks == max_track_duration_ticks
+    assert (finish - start) < 2.0