bug#70036: 30.0.50; Move file-truename to the C level

[Eli Zaretskii]

> Date: Wed, 27 Mar 2024 20:08:54 +0100
> From:  Theodor Thornhill via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> During the last couple of weeks I've been studying Eglots performance
> and have been noticing a couple of things that I find very
> interesting. It seems like `file-truename` is in the hot path due to the
> fact that every request to the lsp server has to create the source file
> location, and in every response we have to parse the location the
> relevant file. `file-truename` is used for this, and its performance
> isn't really up to snuff for what it provides, afaict.
> 
> Below I've supplied some benchmarks and profile reports along with the
> actual patch. Before we discuss the patch itself, I want to get some
> answers to the following:
> 
>  - Is there a reason that this function should be supplied at the lisp
>    level?

No, we could have it implemented in C.  It just never was needed,
until now, and the processing there is not trivial, to say the least.

>  - Does it have to be recursive?

No, it doesn't.

> Firstly, I'll show some benchmarks
> 
> ```
> ;; Emacs 29 branch
> 
> (benchmark-run 10000
>   (file-truename 
> "~/Work/some/long/path/to/parse/that/is/very/deep/deep/deep/super/duper/deep/deep.el"))
> ;; (1.810892642 1 0.051070616)
> 
> 
> ;; With new C implementation
> 
> (benchmark-run 10000
>   (file-truename 
> "~/Work/some/long/path/to/parse/that/is/very/deep/deep/deep/super/duper/deep/deep.el"))
> ;; (0.018811035 0 0.0)
> ```
> 
> As you can see, the C implementation, though naive for now is two orders
> of magnitude faster, and makes a noticeable difference when running an
> lsp server in emacs.

Yes, but comparing a partial implementation is not very useful, since
the complete one could be much more expensive.

> As for the patch - it now relies on wordexp to resolve the paths, and I
> believe there is no real feature parity with the old variant as for now,
> but I haven't seen any issues thus far. If this approach is accepted I
> will of course make sure we have feature parity, unless that isn't
> wanted.

We cannot rely on wordexp and we cannot rely on realpath: both are not
portable enough.

> +  CHECK_STRING (filename);
> +  char *c_filename = SSDATA (filename);
> +
> +  wordexp_t we;
> +  wordexp(c_filename, &we, 0);
> +
> +  char *truename = realpath(we.we_wordv[0], NULL);
> +  wordfree(&we);
> +
> +  if (!truename)
> +    return result;
> +
> +  result = build_string(truename);

You cannot pass Lisp strings to libc functions like that: you need to
do 2 things first:

  . call expand-file-name
  . encode the file name with ENCODE_FILE

This is needed because relative file names in Emacs are relative to
the current buffer's directory, not relative to the current directory
of the Emacs process, and because file names with non-ASCII characters
need to be encoded to match the encoding expected by file-related APIs
in libc.  Likewise, when you get a file name from a libc function, you
need to decode it with DECODE_FILE, before you create a Lisp string
from it

> +  free(truename);

IMO, this should be xfree, not free.  And for that to work, we need to
call realpath with 2nd argument non-NULL, but pointing to a buffer we
allocated with xmalloc, or maybe a stack-based buffer.  (But since we
cannot rely on realpath, this could be a moot point.)

Thanks.