Hi Dan. Super!
On Tuesday, 28 August 2018 17:28:32 UTC+2, d...@thread.com wrote: > > I've run it on our codebase with ~1100 migrations and ~380 apps. > Yes! This is what I was looking for. > There were no exceptions thrown - the script completed cleanly, although I > haven't actually migrated with it or checked that the migrations work. I > assume the script is enough to ensure consistent migrations? > Perfect. The changes just relate to how the graph of migrations is built. This feeds into forward_plan, backwards_plan etc. (The individual migrations remain the same; theres no real need to migrate.) That the script executes without error on a large project is more or less what we want to see here. (There was additional input on the PR confirming the same, so that's good.) There's a second version of the script that writes the generated plans to a file. If you were to run this with v2.1 or master and the PR branch, you should find the generated plans to be same (i.e. the files to be identical). I'll paste this script at the bottom. If you have time to run it, as a extra check, that would be cool. Around a 3.5% speedup. > OK. Perfect. It looks like the kind of cycles that would lead to worst case performance don't really come up. (Most migrations just have a single dependency on the previous one, and that's it.) As such the performance boost of this patch is not massive. (Still worth having, other things being equal.) We would expect slightly better improvement in real-life, since the script here recreates the loader (which regenerates the graph) each time through the loop, whereas `migrate` etc re-use the loader (which in the patch, vs master, only does the graph validation a single time). However, still, performance will not be the key issue. Currently, with the feedback, my thinking is that the patch is a good refactoring (that will likely lead to further improvements). So I'm keen. I just need to go over the code and tests again. Second version of script to follow. Thanks for the input. It's really helpful! Kind Regards, Carlton # Save as e.g. migration_graph.py # Set DJANGO_SETTINGS_MODULE # From project dir run: python migration_graph.py # compare output from v2.1 or higher and PR. # import sys import django django.setup() from django.db import connection from django.db.migrations.loader import MigrationLoader f = open('plans-{}'.format(django.__version__), 'w') print('Writing plans for v{}'.format(django.__version__)) loader = MigrationLoader(connection) backwards = loader.graph.root_nodes() forwards = loader.graph.leaf_nodes() print('Calculating backwards plans:') for root in backwards: loader = MigrationLoader(connection) sys.stdout.write('.') plan = loader.graph.backwards_plan(root) f.write(str(plan)) f.write('\n') f.write('\n' * 3) sys.stdout.write('\n') print('Calculating forwards plans:') for leaf in forwards: loader = MigrationLoader(connection) sys.stdout.write('.') plan = loader.graph.forwards_plan(leaf) f.write(str(plan)) f.write('\n') f.close() sys.stdout.write('\nRun Done\n\n') -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/24ad1be0-c2a4-4a24-9b6c-24a748a09dcb%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.