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