From f91ab1c1881ae5d4d1f946fd0c11baf3918f9a58 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Wed, 7 Oct 2026 19:56:48 -1000 Subject: [PATCH 1/2] Faster copies: cache slots per class; explicit Accidental, Pitch deepcopy _getSlotsRecursive caches its result per class as a frozenset (callers only loop over it), which speeds up copying and pickling every slotted object. Accidental.__deepcopy__ and Pitch.__deepcopy__ assign each attribute directly instead of looping with getattr/setattr; Accidental's copy no longer carries over _client. Pitch attributes set after __init__ are still deep-copied. Accidental 1115 ns -> 394 ns Pitch, sharp 1966 ns -> 803 ns Pitch, no acc. 877 ns -> 490 ns Duration 3089 ns -> 2777 ns Beams 8203 ns -> 6906 ns Note, sharp 12626 ns -> 10738 ns pickle Note 11332 ns -> 9838 ns AI-assisted (Claude) --- music21/common/objects.py | 14 +++++++++--- music21/pitch.py | 44 +++++++++++++++++++++++++++----------- music21/test/test_pitch.py | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/music21/common/objects.py b/music21/common/objects.py index f0fbe3c08..24a63083b 100644 --- a/music21/common/objects.py +++ b/music21/common/objects.py @@ -159,6 +159,8 @@ def __call__(self) -> int: # ------------------------------------------------------------------------------ +_slotsByClass: dict[type, frozenset[str]] = {} + class SlottedObjectMixin: r''' @@ -223,9 +225,9 @@ def __setstate__(self, state: dict[str, t.Any]) -> None: for slot, value in state.items(): setattr(self, slot, value) - def _getSlotsRecursive(self) -> set[str]: + def _getSlotsRecursive(self) -> frozenset[str]: ''' - Find all slots recursively. + Find all slots recursively. Cached per class. A private attribute so as not to change the contents of inheriting objects private interfaces: @@ -250,10 +252,16 @@ def _getSlotsRecursive(self) -> set[str]: ['_editorial', '_style', 'direction', 'funkiness', 'groovability', 'id', 'independentAngle', 'number', 'type'] ''' + try: + return _slotsByClass[type(self)] + except KeyError: + pass slots: set[str] = set() for cls in self.__class__.mro(): slots.update(getattr(cls, '__slots__', ())) - return slots + frozenSlots = frozenset(slots) + _slotsByClass[type(self)] = frozenSlots + return frozenSlots class EqualSlottedObjectMixin(SlottedObjectMixin): diff --git a/music21/pitch.py b/music21/pitch.py index a644e758c..e78b97ad4 100644 --- a/music21/pitch.py +++ b/music21/pitch.py @@ -1042,8 +1042,17 @@ def __hash__(self) -> int: def __deepcopy__(self, memo: dict[int, t.Any]) -> Accidental: if type(self) is Accidental: # pylint: disable=unidiomatic-typecheck new = Accidental.__new__(Accidental) - for s in self._getSlotsRecursive(): - setattr(new, s, getattr(self, s)) + new._alter = self._alter + new._displayStatus = self._displayStatus + new._displayType = self._displayType + new._modifier = self._modifier + new._name = self._name + new._client = None + new.displayLocation = self.displayLocation + new.displaySize = self.displaySize + new.displayStyle = self.displayStyle + new._style = None + new._editorial = None if self._style is not None: new._style = copy.deepcopy(self._style, memo) if self._editorial is not None: @@ -2068,17 +2077,26 @@ def __deepcopy__(self, memo: dict[int, t.Any]) -> Pitch: ''' if type(self) is Pitch: # pylint: disable=unidiomatic-typecheck new = Pitch.__new__(Pitch) - for k in self.__dict__: - v = getattr(self, k, None) - if k in ('_step', '_overridden_freq440', - '_octave', 'spellingIsInferred'): - setattr(new, k, v) - elif k == '_client': - setattr(new, k, None) - elif v is None: # common -- save time over deepcopy. - setattr(new, k, None) - else: - setattr(new, k, copy.deepcopy(v, memo)) + new._groups = None if self._groups is None else copy.deepcopy(self._groups, memo) + new._step = self._step + new._overridden_freq440 = self._overridden_freq440 + new._accidental = (None if self._accidental is None + else copy.deepcopy(self._accidental, memo)) + new._microtone = (None if self._microtone is None + else copy.deepcopy(self._microtone, memo)) + new._octave = self._octave + new.spellingIsInferred = self.spellingIsInferred + new.fundamental = (None if self.fundamental is None + else copy.deepcopy(self.fundamental, memo)) + new._client = None + + selfDict = self.__dict__ + newDict = new.__dict__ + if len(selfDict) != len(newDict): + # attributes not set in __init__ + for k, v in selfDict.items(): + if k not in newDict: + setattr(new, k, copy.deepcopy(v, memo)) return new else: # pragma: no cover return common.defaultDeepcopy(self, memo) diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index 97607e8ca..4eb16a1b0 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -34,6 +34,27 @@ def testCopyManually(self): self.assertIsNot(p1, p2) self.assertIsNot(p1.accidental, p2.accidental) + def testPitchDeepcopyAttributes(self): + ''' + Pitch.__deepcopy__ copies attributes one at a time, for speed. Check that it + copies every attribute, including any set after __init__. Objects are copied, + not shared, and _client is cleared. + ''' + p = Pitch('C#4', microtone=20, fundamental=Pitch('C2')) + p.groups.append('ficta') + owner = note.Note(p) + self.assertIs(p._client, owner) + p.frequencyShifts = [1, 0.5, -1, 2] # not set by __init__ + + p2 = copy.deepcopy(p) + self.assertEqual(set(vars(p2)), set(vars(p))) + self.assertIsNone(p2._client) + for attr in ('_accidental', '_microtone', '_groups', 'fundamental', 'frequencyShifts'): + self.assertEqual(getattr(p2, attr), getattr(p, attr), attr) + self.assertIsNot(getattr(p2, attr), getattr(p, attr), attr) + for attr in ('_step', '_octave', '_overridden_freq440', 'spellingIsInferred'): + self.assertEqual(getattr(p2, attr), getattr(p, attr), attr) + def testRepr(self): p = pitch.Pitch('B#3') self.assertEqual(repr(p), '') @@ -165,6 +186,24 @@ def testAccidentalDeepcopyDoesNotShareStyleOrEditorial(self): self.assertEqual(a.style.color, 'red') self.assertEqual(len(a.editorial.comments), 1) + def testAccidentalDeepcopyCopiesEverySlot(self): + ''' + Every slot but _client is copied; a slot added to Accidental + must be added to its __deepcopy__ too. + ''' + a = Accidental('sharp') + a.displayStatus = True + a.displayType = 'always' + a.displayStyle = 'parentheses' + a.displaySize = 'cue' + a.displayLocation = 'above' + a._client = Pitch('C#4') # Accidental has no public client + b = copy.deepcopy(a) + for slot in a._getSlotsRecursive() - {'_client'}: + with self.subTest(slot=slot): + self.assertEqual(getattr(b, slot), getattr(a, slot)) + self.assertIsNone(b._client) + def testUpdateAccidentalDisplaySimple(self): ''' Test updating accidental display. From a86db327d6d046c181091f599e87a56c3386332e Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 9 Oct 2026 16:49:35 -1000 Subject: [PATCH 2/2] Keep only the slots cache; restore master's copying and Accidental init Accidental.__deepcopy__ and Pitch.__deepcopy__ go back to master's loops, so a slot or attribute added to a class or its parents is still copied without anyone updating a list. The tests that checked those lists go too. Accidental.__init__ calls super().__init__() again instead of inlining StyleMixin.__init__ (#2070), which listed a parent's attributes in a subclass. That costs about 30 ns per Accidental (91 -> 121 ns). What is left is the per-class _getSlotsRecursive cache. Best of 7, master -> branch: copy Accidental 1128 -> 787 ns copy Pitch, sharp 1964 -> 1627 ns copy Duration 3153 -> 2797 ns copy Note, sharp 12793 -> 11982 ns copy Measure of 4 notes 101429 -> 97116 ns pickle Note 11731 -> 10043 ns AI-assisted (Claude) --- music21/pitch.py | 53 +++++++++++--------------------------- music21/test/test_pitch.py | 39 ---------------------------- 2 files changed, 15 insertions(+), 77 deletions(-) diff --git a/music21/pitch.py b/music21/pitch.py index 4a1245bf8..51656f12e 100644 --- a/music21/pitch.py +++ b/music21/pitch.py @@ -1039,13 +1039,8 @@ class Accidental(prebase.ProtoM21Object, style.StyleMixin): # INITIALIZER # - def __init__( # pylint: disable=super-init-not-called - self, - specifier: int|str|float = 'natural', - ) -> None: - # StyleMixin.__init__, inlined for speed; keep in sync - self._style = None - self._editorial = None + def __init__(self, specifier: int|str|float = 'natural') -> None: + super().__init__() # managed by properties self._displayType = 'normal' # normal, always, never, if-absolutely-necessary, @@ -1093,17 +1088,8 @@ def __hash__(self) -> int: def __deepcopy__(self, memo: dict[int, t.Any]) -> Accidental: if type(self) is Accidental: # pylint: disable=unidiomatic-typecheck new = Accidental.__new__(Accidental) - new._alter = self._alter - new._displayStatus = self._displayStatus - new._displayType = self._displayType - new._modifier = self._modifier - new._name = self._name - new._client = None - new.displayLocation = self.displayLocation - new.displaySize = self.displaySize - new.displayStyle = self.displayStyle - new._style = None - new._editorial = None + for s in self._getSlotsRecursive(): + setattr(new, s, getattr(self, s)) if self._style is not None: new._style = copy.deepcopy(self._style, memo) if self._editorial is not None: @@ -2076,26 +2062,17 @@ def __deepcopy__(self, memo: dict[int, t.Any]) -> Pitch: ''' if type(self) is Pitch: # pylint: disable=unidiomatic-typecheck new = Pitch.__new__(Pitch) - new._groups = None if self._groups is None else copy.deepcopy(self._groups, memo) - new._step = self._step - new._overridden_freq440 = self._overridden_freq440 - new._accidental = (None if self._accidental is None - else copy.deepcopy(self._accidental, memo)) - new._microtone = (None if self._microtone is None - else copy.deepcopy(self._microtone, memo)) - new._octave = self._octave - new.spellingIsInferred = self.spellingIsInferred - new.fundamental = (None if self.fundamental is None - else copy.deepcopy(self.fundamental, memo)) - new._client = None - - selfDict = self.__dict__ - newDict = new.__dict__ - if len(selfDict) != len(newDict): - # attributes not set in __init__ - for k, v in selfDict.items(): - if k not in newDict: - setattr(new, k, copy.deepcopy(v, memo)) + for k in self.__dict__: + v = getattr(self, k, None) + if k in ('_step', '_overridden_freq440', + '_octave', 'spellingIsInferred'): + setattr(new, k, v) + elif k == '_client': + setattr(new, k, None) + elif v is None: # common -- save time over deepcopy. + setattr(new, k, None) + else: + setattr(new, k, copy.deepcopy(v, memo)) return new else: # pragma: no cover return common.defaultDeepcopy(self, memo) diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index ad59e2112..35875404a 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -36,27 +36,6 @@ def testCopyManually(self): self.assertIsNot(p1, p2) self.assertIsNot(p1.accidental, p2.accidental) - def testPitchDeepcopyAttributes(self): - ''' - Pitch.__deepcopy__ copies attributes one at a time, for speed. Check that it - copies every attribute, including any set after __init__. Objects are copied, - not shared, and _client is cleared. - ''' - p = Pitch('C#4', microtone=20, fundamental=Pitch('C2')) - p.groups.append('ficta') - owner = note.Note(p) - self.assertIs(p._client, owner) - p.frequencyShifts = [1, 0.5, -1, 2] # not set by __init__ - - p2 = copy.deepcopy(p) - self.assertEqual(set(vars(p2)), set(vars(p))) - self.assertIsNone(p2._client) - for attr in ('_accidental', '_microtone', '_groups', 'fundamental', 'frequencyShifts'): - self.assertEqual(getattr(p2, attr), getattr(p, attr), attr) - self.assertIsNot(getattr(p2, attr), getattr(p, attr), attr) - for attr in ('_step', '_octave', '_overridden_freq440', 'spellingIsInferred'): - self.assertEqual(getattr(p2, attr), getattr(p, attr), attr) - def testRepr(self): p = pitch.Pitch('B#3') self.assertEqual(repr(p), '') @@ -188,24 +167,6 @@ def testAccidentalDeepcopyDoesNotShareStyleOrEditorial(self): self.assertEqual(a.style.color, 'red') self.assertEqual(len(a.editorial.comments), 1) - def testAccidentalDeepcopyCopiesEverySlot(self): - ''' - Every slot but _client is copied; a slot added to Accidental - must be added to its __deepcopy__ too. - ''' - a = Accidental('sharp') - a.displayStatus = True - a.displayType = 'always' - a.displayStyle = 'parentheses' - a.displaySize = 'cue' - a.displayLocation = 'above' - a._client = Pitch('C#4') # Accidental has no public client - b = copy.deepcopy(a) - for slot in a._getSlotsRecursive() - {'_client'}: - with self.subTest(slot=slot): - self.assertEqual(getattr(b, slot), getattr(a, slot)) - self.assertIsNone(b._client) - def testAccidentalLookupTable(self): ''' Every way of writing each standard accidental gives the same accidental