[vim/vim] Why `bufloaded()` call chdir() under the hood? (Issue #12694)

[Yggdroot]

Steps to reproduce

1. vim --clean
2. :set verbose=9
3. echo bufloaded('/tmp/test.vim')

output is :

chdir(/tmp)
fchdir() to previous dir
0

Expected behaviour

0

Version of Vim

All

Environment

all

Logs and stack traces

I think there may be overhead if chdir.

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/12694@github.com>

[K.Takata]

mch_FullName() in os_unix.c calls chdir(). Not sure why.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/12694/1645036588@github.com>

[Christian Brabandt]

is this actually a problem?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/12694/1646938926@github.com>

[Yegappan Lakshmanan]

Hi Christian,

On Sun, Jul 23, 2023 at 12:24 PM Christian Brabandt <vim-dev...@256bit.org> wrote:

is this actually a problem?

Yes. Many of the LSP plugins use the bufloaded() function to check whether a buffer is loaded or not.

If the buffer is not loaded, then they call the bufload() function to load the buffer.   This is done before 

refreshing signs, virtual text, highlights, etc. in a buffer.  So this function is called many times.  If this is

an expensive operation, then this will slow down the plugin.

For this reason, in the Vim9 LSP plugin, I used the bufload() function to unconditionally load the plugin

without using the bufloaded() function.  If the buffer is already loaded, then this is no-op.

Also, if the buffer number is used instead of the buffer name, then the directory is not changed.

So the LSP plugin uses only the buffer number for the call to bufload().

Regards,

Yegappan

[vim-dev ML]

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/12694/1646942602@github.com>

[Christian Brabandt]

Well, it seems to be for simplifying stuff like '/tmp/../tmp/../tmp/foobar.viminto/tmp/foobar.vim`

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/issues/12694/1646953439@github.com>

[Christian Brabandt]

I made a simple experiment. I used latest master (v.9.0.1677) huge built in console mode.

I applied two different patches:

1. removed the call to chdir() and dirname() from vim_FullName() completely:

diff --git a/src/os_unix.c b/src/os_unix.c
index 31f66b137..1f3769ffa 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -2691,6 +2691,7 @@ mch_FullName(
     fname = posix_fname;
 #endif

+#if 0
     // Expand it if forced or not an absolute path.
     // Do not do it for "/file", the result is always "/".
     if ((force || !mch_isFullName(fname))
@@ -2803,6 +2804,7 @@ mch_FullName(
            STRCAT(buf, "/");
 #endif
     }
+#endif

     // Catch file names which are too long.
     if (retval == FAIL || (int)(STRLEN(buf) + STRLEN(fname)) >= len)

2. replaced the dirname() and chdir() calls by using simplify_fname():

diff --git a/src/os_unix.c b/src/os_unix.c
index 31f66b137..028989ba5 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -2696,6 +2696,9 @@ mch_FullName(
     if ((force || !mch_isFullName(fname))
            && ((p = vim_strrchr(fname, '/')) == NULL || p != fname))
     {
+       simplify_filename(fname);
+    }
+#if 0
        /*
         * If the file name has a path, change to that directory for a moment,
         * and then get the directory (and get back to where we were).
@@ -2803,6 +2806,7 @@ mch_FullName(
            STRCAT(buf, "/");
 #endif
     }
+#endif

     // Catch file names which are too long.
     if (retval == FAIL || (int)(STRLEN(buf) + STRLEN(fname)) >= len)

I am not sure, that the simplify_filename() is equivalent to the dirname() and chdir() calls. I suppose it may differ, if there are symlinks involved however.

Then I used the following vimscript to test the performance:

vim9script

def Bufloaded(nr: number)
  for i in range(1, nr)
    bufloaded('/tmp/../tmp/../tmp/foobar_123456789')
    bufloaded('/tmp/foobar_123456789')
  endfor
enddef

var start = reltime()
Bufloaded(100000)
echomsg $"{start->reltime()->reltimestr()} seconds"

This is a bit of a pathological testcase, as it runs 100, 000 times with a non-loaded buffer name that is already expanded (and cannot be simplified) and 100, 000 times with a buffer name that can be simplified.

This is the result:

Version	default vim (v9.0.1677)	vim patched 1	vim patch 2
Time (Avg)	0.811s	0.2561 s	0.5837 s

So the difference between fully removing the dirname() and chdir() calls and the default Vim is 0.5549s, which means changing the directory and getting the directory name adds about 0.0000027745s per bufloaded() call.

This seems negligible to me.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/issues/12694/1647334728@github.com>

[Yggdroot]

Closed #12694 as completed.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/issue/12694/issue_event/9899829618@github.com>