editorconfig-vim icon indicating copy to clipboard operation
editorconfig-vim copied to clipboard

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

Open sergiodegioia opened this issue 5 years ago • 6 comments

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

sergiodegioia avatar Aug 05 '20 10:08 sergiodegioia

Could you explain a bit more?

xuhdev avatar Aug 05 '20 21:08 xuhdev

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.

sergiodegioia avatar Aug 05 '20 22:08 sergiodegioia

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?

cxw42 avatar Aug 13 '20 01:08 cxw42

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).

sergiodegioia avatar Aug 13 '20 10:08 sergiodegioia