id,summary,reporter,owner,description,type,status,component,version,severity,resolution,keywords,cc,stage,has_patch,needs_docs,needs_tests,needs_better_patch,easy,ui_ux 24591,Copy ModelState.fields and ModelState.managers instead of cloning.,Marten Kenbeek,Marten Kenbeek,"Fields in a`ModelState` are guaranteed to not be bound to a model, and are deconstructed and reconstructed in `ModelState.from_model()`. They are also properly deconstructed and reconstructed when rendering a model. Furthermore, the fields themselves must not change during the lifetime of a `ModelState` object, though you may freely add, remove and replace fields (1). They should be considered immutable within model states. I think the current implementation of `ModelState.clone()` is too conservative in this regard. Both the input (in `ModelState.from_model()`) and the output (in `ModelState.render()`) do a full de- and reconstruct. The only change in between is the addition and removal of fields from the list. `ModelState.clone()` can do a simple `list(self.fields)` and still maintain the correct behaviour. The same holds for `ModelState.managers`. This change saves about 75% in rendering time, and 66% in memory usage in my benchmarks. If you want to safeguard against accidental changes that can affect other model states, you can use `copy.deepcopy()`, though at a performance cost. Other options are to somehow shield against accidental changes by generating an immutable field class, or by saving the deconstructed field and constructing the fields on demand. As this is ''the'' most performance-critical part of migrations -- save the actual database manipulation -- I suggest we take a careful look into this. The current implementation with `deconstruct()` is far from the ideal solution. (1) https://github.com/django/django/blob/master/django/db/migrations/state.py#L294",Cleanup/optimization,closed,Migrations,dev,Normal,fixed,,,Ready for checkin,1,0,0,0,0,0