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.

Reply via email to