editorconfig / editorconfig-vim

EditorConfig plugin for Vim
http://editorconfig.org
Other
3.13k stars 137 forks source link

FIXED: filereadable('//.editorconfig') took too long under mingw64. #152

Open sergiodegioia opened 4 years ago

sergiodegioia commented 4 years ago

GetFilenames is changed to manage paths with a trailing semantically-neutral '/.', thus avoiding to check l:path=='/' at each iteration.

xuhdev commented 4 years ago

Could you explain a bit more?

sergiodegioia commented 4 years ago

Since I installed editorconfig-vim, buffer loading started to be dramatically slow, making Vim unusable. I launched the vim profiler: this is the excerpt

FUNCTION  32_UseConfigFiles()
    Defined: ~/.vim/pack/local/start/editorconfig-vim/plugin/editorconfig.vim line 199
Called 1 time
Total time:   4.670006
 Self time:   4.520006

count  total (s)   self (s)
    1              0.000000     let l:buffer_name = expand('%:p')
                                " ignore buffers without a name
    1              0.000000     if empty(l:buffer_name)
                                    return
    1              0.000000     endif

                                " Check if any .editorconfig does exist
    1              0.000000     let l:conf_files = s:GetFilenames(expand('%:p:h'), '.editorconfig')
    1              0.000000     let l:conf_found = 0
    6              0.000000     for conf_file in conf_files
    6              4.510006         if filereadable(conf_file)
    1              0.000000             let l:conf_found = 1
    1              0.000000             break
    5              0.000000         endif
    6              0.000000     endfor

Then I spotted a problem with the execution of filereadable(conf_file), as you can see. Then I echoed conf_file in the loop whose values come from s:GetFilenames(expand('%:p:h'), '.editorconfig'). Among its values there is always '//.editorconfig' because s:GetFilenames walks the way up to the root starting from the current directory of the current buffer expand('%:p:h'), and appends '/.editorconfig'. Then I tried

:call filereadable('//.editorconfig')
and it took more than 4 seconds to return. I'm using vim under mingw64 and it seems to me that spurious slashes are badly handled at the start of a path under mingw64. Eliminating the double slash at the start of the path, buffers are loaded instantly instead of after 4.5 seconds. In order to eliminate the double slash, s:GetFilenames must be changed such that, when l:path=='/',

let l:path_list += [l:path . '/' . a:filename]

is substituted for by

let l:path_list += [l:path . a:filename]

Or conveniently, in order to avoid checking for l:path=='/' at each iteration,

let l:path_list += [l:path . '/' . a:filename]

must be substituted for by

let l:path_list += ['/.' . l:path . '/' . a:filename]

as I did. In this case if a new buffer is loaded for a file under /home/foo filereadable will be executed for these file paths:

/./home/foo/.editorconfig
/./home/.editorconfig
/.//.editorconfig

that are semantically equivalent to these file paths (which are returned by the original s:GetFilenames):

/home/foo/.editorconfig
/home/.editorconfig
//.editorconfig

with the advantages that no double slash appears at the start of a path and no additional check is added.

cxw42 commented 4 years ago

Good find! Makes sense to me. Have you been able to test on any other platforms? Does this code also work correctly in native Windows vim?

sergiodegioia commented 4 years ago

Thanks for your insight. It did not work under Windows, so I reverted back to the former idea of substituting

        if l:path == '/'
            let l:path_list += [ '/' . a:filename]
        else
            let l:path_list += [ l:path . '/' . a:filename]
        endif

for

let l:path_list += [l:path . '/' . a:filename]

Moreover I also checked for its proper functioning under Centos 7 and Cygwin. (I found that Cygwin suffered from the same problem as Mingw64: dramatically slow buffer loading. This commit will then be useful for Cygwin user as well).

divVerent commented 3 months ago

This is actually very related to this:

https://github.com/editorconfig/editorconfig-vim/issues/238

Note that here //.editorconfig was generated too. It seems the path name handling generally has some issues.