tweekmonster / django-plus.vim

:guitar: Improvements to the handling of Django related files in Vim
MIT License
180 stars 15 forks source link

Add autodetection (only load plugin when editing a Django project) #13

Open joeytwiddle opened 6 years ago

joeytwiddle commented 6 years ago

This project could not be included in vim-polyglot because it does too much on startup, even if the user is only editing a text file or a Javascript file.

(I am also reluctant to keep it installed, for the same reason. I use Vim for lots of different things, and only use Django occasionally.)

Suggestion:

Challenges:

tweekmonster commented 6 years ago

Thanks for your input and I appreciate you trying to increase the visibility of this plugin.

Before going to the trouble of optimizing something, I spend a little time determining whether or not the time spent on the optimizations will make an impactful and noticeable difference without sacrificing usefulness.

Setup

I built my dev box in 2015 and use a nightly build of Neovim. The CPU is an Intel Core i3-4170 CPU @ 3.70GHz, and it has 8GB of RAM. I copied all of the scripts to a mechanical HDD that spins at 7200 RPM. I feel that I wasn't fast and loose about leaning on the hardware for performance.

I'm not grooming my configs for the profile. The stats below are based on my full config. If you look through my configs (not up to date), you will notice that I'm not very mindful at all about the startup time (another plugin I made) or file loading time. I mainly rely on the notion that anything (outside of keystrokes and very frequent actions) taking less than 200ms is acceptable.

Profiling

I used the following to load files:

:argadd **/*.py
:profile start profile.log
:profile func *
:profile file *
:silent argdo edit
:profile pause
:noautocmd qall!

I got the file count using:

find . -name '*.py' | wc -l

Here's how long it took to load the detection script (from an SSD):

SCRIPT  /home/tallen/dev/vim/django-plus.vim/autoload/djangoplus/detect.vim
Sourced 1 time
Total time:   0.000106
 Self time:   0.000106

Non-Django files

I profiled the loading of 1677 non-Django python files at once from a randomly chosen pyenv site-packages directory, which is 10 directories deep minimum.

Here's the profiled djangoplus#detect#filetype() function:

Click to see full function ``` FUNCTION djangoplus#detect#filetype() Called 6708 times Total time: 1.401331 Self time: 0.268118 ``` ``` count total (s) self (s) 6708 0.013209 if empty(a:filename) return endif 6708 0.097265 0.029477 let is_django = s:simple_django_project(a:filename) 6708 0.006506 if is_django == -1 " Since the current directory isn't a Django project, perform a more " exhaustive scan. 6708 1.108504 0.043079 let is_django = s:scan(a:filename, 's:is_django_project') || s:scan(a:filename, 's:is_django_app') 6708 0.003022 endif 6708 0.004402 if is_django let b:is_django = 1 let filedir = fnamemodify(a:filename, ':h') autocmd! CursorHold call djangoplus#clear_template_cache() if a:filename =~? '\.html\?$' setfiletype htmldjango elseif s:is_django_settings(a:filename) setfiletype python let b:is_django_settings = 1 elseif a:filename =~# '\.py$' setfiletype python else for pat in get(g:, 'django_filetypes', []) if a:filename =~# glob2regpat(pat) let bft = &l:filetype if !empty(bft) let bft .= '.django' else let bft = 'django' endif execute 'setfiletype' bft endif endfor for ft in split(&l:filetype, '\.') execute 'runtime! ftplugin/'.ft.'.vim' execute 'runtime! after/ftplugin/'.ft.'.vim' endfor endif endif ```

It was called 6708 times, and cumulatively ran for ~1.4 seconds (with the bulk of the time spent on scanning), to determine that 1677 files were not related to Django. In each case, it completely skips the block that sets the filetype.

The function that does the directory scanning:

Click to see full function ``` FUNCTION 272_scan() Called 13416 times Total time: 1.065425 Self time: 1.063053 ``` ``` count total (s) self (s) 13416 0.015423 if empty(a:filename) return 0 endif 13416 0.049765 let dirname = fnamemodify(a:filename, ':p:h') 13416 0.011665 let last_dir = '' 13416 0.011066 let depth = 0 13416 0.025888 let max_depth = get(g:, 'django_max_scan_depth', 10) 95840 0.241928 while depth < max_depth && dirname != last_dir && dirname !~ '^\/*$' 82424 0.070947 let last_dir = dirname 82424 0.101617 if has_key(s:seen, dirname) 82238 0.068419 if s:seen[dirname] return 1 else 82238 0.121058 let dirname = fnamemodify(dirname, ':h') 82238 0.052974 continue endif endif 186 0.003204 0.000833 if call(a:func, [dirname]) let s:seen[dirname] = 1 return 1 endif 186 0.000416 let s:seen[dirname] = 0 186 0.000227 let depth += 1 186 0.000365 let dirname = fnamemodify(dirname, ':h') 186 0.000172 endwhile 13416 0.007319 return 0 ```

Most of the time was spent in the loop, skipping directories that were already checked. It could cache the results and never run the loop again, but as you issued in your challenge, we'd want to be able to still handle a case where the user starts a new project, right?

If you agree that it's unrealistic for someone to open 1677 files of any type in Vim, you might agree that it's unreasonable to think that 1.4 seconds is "slow" in this case.

Django scripts

This profile is based on a Django project that had 520 python scripts

The djangoplus#detect#filetype() function:

Click to see full function ``` FUNCTION djangoplus#detect#filetype() Called 2080 times Total time: 7.885908 Self time: 0.142030 ``` ``` count total (s) self (s) 2080 0.003983 if empty(a:filename) return endif 2080 0.030286 0.009184 let is_django = s:simple_django_project(a:filename) 2080 0.001975 if is_django == -1 " Since the current directory isn't a Django project, perform a more " exhaustive scan. 2080 0.097139 0.009842 let is_django = s:scan(a:filename, 's:is_django_project') || s:scan(a:filename, 's:is_django_app') 2080 0.001009 endif 2080 0.001353 if is_django 2080 0.002167 let b:is_django = 1 2080 0.003975 let filedir = fnamemodify(a:filename, ':h') 2080 0.042302 autocmd! CursorHold call djangoplus#clear_template_cache() 2080 0.006124 if a:filename =~? '\.html\?$' setfiletype htmldjango elseif s:is_django_settings(a:filename) 28 0.098566 0.000062 setfiletype python 28 0.000042 let b:is_django_settings = 1 28 0.000023 elseif a:filename =~# '\.py$' 2052 7.503698 0.004455 setfiletype python 2052 0.000999 else for pat in get(g:, 'django_filetypes', []) if a:filename =~# glob2regpat(pat) let bft = &l:filetype if !empty(bft) let bft .= '.django' else let bft = 'django' endif execute 'setfiletype' bft endif endfor for ft in split(&l:filetype, '\.') execute 'runtime! ftplugin/'.ft.'.vim' execute 'runtime! after/ftplugin/'.ft.'.vim' endfor endif 2080 0.000984 endif ```

Oof, this one takes about 8 seconds in total. However, if you expand it, you'll see the time is spent on setfiletype python. So, the slow down is from other scripts. I won't post the scan function again since you can see in the function above, it takes a cumulative ~100ms. It was actually closer to 90ms since the time above also includes the time spent on assigning the function's return value to a variable.

Actually here's the scan function because I now feel like I'm boasting without proof:

Click to see full function ``` FUNCTION 273_scan() Called 2080 times Total time: 0.087297 Self time: 0.086512 ``` ``` count total (s) self (s) 2080 0.002325 if empty(a:filename) return 0 endif 2080 0.008440 let dirname = fnamemodify(a:filename, ':p:h') 2080 0.001940 let last_dir = '' 2080 0.001791 let depth = 0 2080 0.004356 let max_depth = get(g:, 'django_max_scan_depth', 10) 6040 0.021360 while depth < max_depth && dirname != last_dir && dirname !~ '^\/*$' 6040 0.005979 let last_dir = dirname 6040 0.008872 if has_key(s:seen, dirname) 5957 0.005576 if s:seen[dirname] 2079 0.001004 return 1 else 3878 0.006828 let dirname = fnamemodify(dirname, ':h') 3878 0.003602 continue endif endif 83 0.001144 0.000360 if call(a:func, [dirname]) 1 0.000001 let s:seen[dirname] = 1 1 0.000001 return 1 endif 82 0.000159 let s:seen[dirname] = 0 82 0.000099 let depth += 1 82 0.000244 let dirname = fnamemodify(dirname, ':h') 82 0.000081 endwhile return 0 ```

I tried to figure out what exactly was taking so long to load when setting the python filetype, but it looks like it's sprinkled throughout a few python plugins, including nvim's builtin python plugin. The slowest plugin was my first plugin that I'm a bit ashamed of performance-wise. But, it only took about 800ms in total and jedi-vim contributed a bit with about 300ms total. I can at least confirm that it wasn't other django-plus scripts:

SCRIPT  /home/tallen/dev/vim/django-plus.vim/after/ftplugin/python.vim
Sourced 1040 times
Total time:   0.325178
 Self time:   0.325045
SCRIPT  /home/tallen/dev/vim/django-plus.vim/after/syntax/python.vim
Sourced 1 time
Total time:   0.000053
 Self time:   0.000053

A cumulative 325ms to load 520 files doesn't look bad to me.

The same file

Since nvim didn't really like me trying to open 1677 files at once, I ran another simpler test by opening an empty python script and wiping its buffer 1000 times (from the same working directory).

let i = 0

while i < 1000
  let i = i + 1
  split empty.py
  quit
  bwipeout empty.py
endwhile

This was the impact of the detection function:

FUNCTION  djangoplus#detect#filetype()
Called 2000 times
Total time:   0.342212
 Self time:   0.079352

This looks like an anomaly to me because it seems that the function is called twice each time the file is loaded. In the earlier runs, it was called four times per file loaded! I suspect it might be another plugin that's misbehaving. This is something I would investigate, but I would say that it still ran quickly enough for opening the same file 1000 times.

One last time but with just one file:

FUNCTION  djangoplus#detect#filetype()
Called 2 times
Total time:   0.000347
 Self time:   0.000086

I know it's not quite a solid test, but that run time is pretty close to the zero second delay that I personally feel that I experience when loading a file. Whatever delay you are experiencing might be caused by another plugin.

Conclusion

I guess it's fast enough? The script for scanning non-Django files could be faster, but the question remains: will the time spent on the optimizations make an impactful and noticeable difference without sacrificing usefulness? Personally, I don't think so.

Going by the numbers I see, I definitely want to avoid holding any of my plugins to the standard vim-polygot wants to set. If a plugin only has ftplugin, syntax, etc scripts, the existing system already loads them on demand and never before. As far as I can see, vim-polygot is just a curated collection of plugins that disallows plugin/* and filetype.vim, then flattens their directory structures and discards their commit history and documentation.

It's absolutely true that you save time from not loading a bunch of little scripts, but the time saved is negligible when compared to almost anything you could be waiting for in a lifetime.


I will address some things you mentioned here:

even if the user is only editing a text file or a Javascript file

The comment that I included should indicate that this is known.

I am also reluctant to keep it installed, for the same reason

If you benchmarked this and actually experience slowdown because of this plugin, your problem is most likely outside of my influence.

I use Vim for lots of different things, and only use Django occasionally

Due to a change in my career, I haven't had to work on Django that much either. I never noticed a slowdown because of this plugin and I have a tendency to hang onto sessions with a lot of buffers.

Move all setup functions into a separate file, which will only be run when needed

As you saw above, this is how it already works.

Do a quick check on BufReadPost

If this was done, html files related to Django would have already loaded the html ftplugin and syntax scripts. Wouldn't it hurt the load time more If this script allows the file to be detected as html, but then change it to htmldjango after the fact? Also, with Django already being a pain to detect in Vim, wouldn't it be nice if b:is_django was available before your custom scripts loaded?

If the user is starting a fresh project, or has not yet opened a Django-like file, then they will see none of the django-plus features. (I suggest providing an option to always load django-plus on startup, for those people who use django every day.)

This is basically the case now. If you're saying you'd prefer that the filetype not be autodetected by default, I would be fine with a PR to add an option to cause the detection to immediately return, but not flat out disable the autocmd.