Opened 3 years ago

Last modified 3 years ago

#33380 assigned New feature

Change runserver's auto-reloader to prefer (likely) user files when scanning for changes.

Reported by: Keryn Knight Owned by: Hrushikesh Vaidya
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Tom Forbes Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently, because of the way iter_modules_and_files returns an LRU cached frozenset, it executes (as far as I can tell) a maximum of twice where everything downstream of that essentially executes on every tick, so sorting further down is potentially a net detriment.

I'd like to propose changing that method to return a tuple (i.e. similarly immutable) of ordered paths, by their likelihood of being edited. This would I think reduce the number of stat calls in the common scenario of a user's code changing, because those would be stat'd soonest and notify_file_changed would fire to trigger a reload.

What I'm suggesting is something like (off the top of my head) the following, give or take implementation details:

stdlib_location1 = dirname(os.__file__)
stdlib_location2 = dirname(dirname(logging.__file__))  # Based on my own comments about having done this before, this isn't necessarily the same path (e.g. pyenv shims)
django_location = os.path.dirname(os.path.dirname(django.__file__))  # and again this may be a site-packages dir which is different
if is_django_path(resolved_path):
    results.add((resolved_path, 10))
elif resolved_path.startswith(stdlib_location1, stdlib_location2, django_location):   # third party packages are potentially stored in these locations
    results.add((resolved_path, 5))
elif module in sys.builtin_module_names:
    results.add((resolved_path, 30))
elif module in sys.stdlib_module_names: # eventually, it's new in py3.10
    results.add((resolved_path, 20))
elif not isinstance(module.__spec__.loader, SourceFileLoaderOrWhatever):  # covers .so files, zip imports etc.
    results.add((resolved_path, 40))
else:
    results.add((resolved_path, 0))

and then ordering the results by the number rank, and returning a tuple of those paths, in that order (again off the top of my head):

results = tuple(result for result, rank in sorted(results, key=itemgetter(1)))

which would hopefully yield something like:

/path/to/myapp/views.py  # (rank 0, local file, likely to change)
/another/path/django-livereloadish/livereloadish/middleware.py # (rank 0, this is a symlink to my consolidated git repos, maybe changable)
/user/path/.direnv/python-3.9.5/src/enviable/enviable.py  # (rank 0, this'd be a pip editable install)
/user/path/.direnv/python-3.9.5/lib/python3.9/site-packages/weasyprint/svg/path.py  # (rank 5, within site packages, potential for edits)
/user/path/.direnv/python-3.9.5/lib/python3.9/site-packages/mistune/markdown.py # (rank 5, within site packages, potential for edits)
/user/path/.direnv/python-3.9.5/lib/python3.9/site-packages/django/db/models/utils.py # (rank 10, django path, may be edited for debugging or development, but less likely in the scheme of things)
# ... ad infinitum for modules/files which rank as less likely to be edited (dylibs, builtins, etc)

compared to what I currently might get, which looks something like this (printing filepath within tick()):

/path/.asdf/installs/python/3.9.5/lib/python3.9/ctypes/macholib/framework.py
/path/.direnv/python-3.9.5/lib/python3.9/site-packages/django/core/management/commands/runserver.py
/path/.asdf/installs/python/3.9.5/lib/python3.9/email/mime/base.py
/path/.asdf/installs/python/3.9.5/lib/python3.9/encodings/utf_16_le.py
# ... hundreds more stdlib/unlikely things
/another/path/django-livereloadish/livereloadish/patches.py
# ... a bunch more
/user/path/.direnv/python-3.9.5/src/enviable/enviable.py
# ... ad infinitum.

Change History (4)

comment:1 by Mariusz Felisiak, 3 years ago

Cc: Tom Forbes added

comment:2 by Carlton Gibson, 3 years ago

Triage Stage: UnreviewedAccepted

Hey Keryn — this sounds worth having a look at to me. 👍 Thanks!

comment:3 by Hrushikesh Vaidya, 3 years ago

Owner: changed from nobody to Hrushikesh Vaidya
Status: newassigned

comment:4 by Hrushikesh Vaidya, 3 years ago

Has patch: set
Patch needs improvement: set

An initial PR. It's not 100% correct (some other non-project files manage to filter through), but all of the user's project files are put at the start.

Note: See TracTickets for help on using tickets.
Back to Top