Repository navigation
Faster copies of slotted objects, Accidental, and Pitch - #2072
Merged
Merged
Conversation
…copy _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)
# Conflicts: # music21/test/test_pitch.py
|
coverage: 93.372% (+0.001%) from 93.371% — copy-speedups into master |
mscuthbert
marked this pull request as ready for review
October 8, 2026 09:37
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found an easy place to do some substantial speedups of Music21Object/ProtoM21Object with slots -- since we can't just copy the slots of the current class but also need to copy everything in the slots of the ancestors (unless we call super().deepcopy and they handle it), we need to have a way of figuring out all the slot names of everything up the ancestor chain. So we've used
_getSlotsRecursive...but the thing is: the recursive slots never change for a class. So we should cache it. Big improvements in deepcopying speed:AccidentalPitch, no accidentalPitch, sharp or naturalFor m21 slotted objects,
_getSlotsRecursivenow caches its result once per class as a frozenset (callers only loop over it).The numbers above for Pitch and Accidental are incorrect. they save about the same 220ns that everything else does. Agent had tried to have
Accidental.__deepcopy__andPitch.__deepcopy__list every attribute directly and figure out the best way to deal with that. I'm not ready to give up the flexibility of subclassing these classes etc.AI-assisted (Claude)