bug#40407: [PATCH] slow ENCODE_FILE and DECODE_FILE

[Mattias Engdegård]

3 apr. 2020 kl. 18.24 skrev Eli Zaretskii <eliz@gnu.org>:

> AFAIR, on macOS the situation is worse than elsewhere, because of the
> normalization thing.

Very likely. It's just what I had in my lap.

> Can you tell more about the conversion steps and the memory each one
> allocates?

Courtesy the memory profiler:

         - file-relative-name                                 141,551  15%
          - file-name-case-insensitive-p                      100,613  11%
           - ucs-normalize-hfs-nfd-pre-write-conversion       100,613  11%
            - ucs-normalize-HFS-NFD-region                    100,613  11%
               ucs-normalize-region                           100,613  11%
          - expand-file-name                                   40,828   4%
           - ucs-normalize-hfs-nfd-post-read-conversion        40,828   4%
            - ucs-normalize-HFS-NFC-region                     40,828   4%
               ucs-normalize-region                            40,828   4%

where file_name_case_insensitive_p calls ENCODE_FILE and expand_file_name calls 
DECODE_FILE. I'm not sure how much each part of ucs-normalize-region actually 
consumes, but I think we can agree that we don't want it called on any platform 
unless strictly necessary.

> I don't think every encoding is ASCII compatible, so I don't see how
> we can assume that in general.  But the check whether an encoding is
> ASCII-compatible takes a negligible amount of time, so why bother with
> such an assumption?

Quite, I just thought I'd ask in case there were some unwritten invariant that 
you knew about.

> I'm not sure I understand what you mean by extending the shortcut to
> decode_file_name.  Please elaborate.

Never mind, it was an under-thought idea. The existing bootstrap hack making 
encode_file_name identity for any unibyte string does not seem to need or allow 
any symmetry in decode_file_name.

> I don't think we can return the same string if NOCOPY is non-zero.
> The callers might not expect that, and you might inadvertently cause
> the original string be modified behind the caller's back.

You are no doubt correct, but doesn't it look like the sense of NOCOPY has been 
inverted here? It runs contrary to the intuitive meaning and to the doc string 
of {encode,decode}-coding-string. In fact:

(let* ((nocopy nil)
       (x "abc")
       (y (decode-coding-string x nil nocopy nil)))
  (eq x y))
=> t

Looks like we suddenly got more work on our hands. What a surprise.

Since string mutation is so rare, I doubt it has caused any real trouble. Now, 
do we fix it by inverting the sense of the argument, or by renaming it to COPY? 
I'm fairly neutral, but there are arguments in either way, both in terms of 
performance and correctness. And what about internal calls to 
code_convert_string?

There are 193 calls to {encode, decode}-coding-string in the Emacs tree, and 
only 14 of them pass a non-nil value to NOCOPY. I'd be inclined to keep the 
semantics but rename the argument to COPY, on the grounds that no-copy is a 
better default; then change those 14 calls to pass nil instead, since that 
obviously was the intent.