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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sergiodegioia
Copy link

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

…etFilenames is changed to manage paths with a trailing semantically-neutral '/.', thus avoiding to check l:path=='/' at each iteration.
@xuhdev
Copy link
Member

xuhdev commented Aug 5, 2020

Could you explain a bit more?

@sergiodegioia
Copy link
Author

sergiodegioia commented Aug 5, 2020

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
Copy link
Member

cxw42 commented Aug 13, 2020

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
Copy link
Author

sergiodegioia commented Aug 13, 2020

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
Copy link

This is actually very related to this:

#238

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

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

Successfully merging this pull request may close these issues.

4 participants