Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
joeytwiddle opened this issue Apr 14, 2018 · 1 comment
Open

Comments

@joeytwiddle
Copy link

joeytwiddle commented Apr 14, 2018

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:

  • Move all setup functions into a separate file, which will only be run when needed
  • Do a quick check on BufReadPost: If django-plus is not yet loaded, and this file looks like a Django file, then load django-plus

Challenges:

  • 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.)
@tweekmonster
Copy link
Owner

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 <buffer> 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  <SNR>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 <buffer> 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  <SNR>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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants