Subject: Re: bug#40407: [PATCH] slow ENCODE_FILE and DECODE_FILE

From: Eli Zaretskii <[email-masked]>
Date: Sat, 04 Apr 2020 20:22:37 +0300

> From: Mattias Engdegård <[email-masked]>
> Date: Sat, 4 Apr 2020 18:41:39 +0200
> Cc: [email-masked]
> 
> > DECODE_FILE is called because the file name in question starts with a
> > "~"?  Otherwise, I don't think I understand why would expand-file-name
> > need to decode a file name.
> 
> Maybe it's because default-directory started with a tilde. It doesn't really matter; it's a common case, and the profiler tells us as much.

I think it's important that we understand what happens here to the
last detail, but okay.

> > IME, the cases where we can safely assume it's OK to return the same
> > string are actually very rare.  It is no accident that you saw so few
> > calls of these functions where we use that optional behavior.
> 
> This does not mean that the remaining 179 calls require a copy; they just use the default value of the parameter.

And IMO the default must stay that a copy is returned, except when the
caller says otherwise.

> +  if (EQ (dst_object, Qt))
> +    {
> +      /* Fast path for ASCII-only input and an ASCII-compatible coding:
> +         act as identity.  */
> +      Lisp_Object attrs = CODING_ID_ATTRS (coding.id);
> +      if (! NILP (CODING_ATTR_ASCII_COMPAT (attrs))
> +          && (STRING_MULTIBYTE (string)
> +              ? (chars == bytes) : string_ascii_p (string)))
> +	return nocopy ? Fcopy_sequence (string) : string;

I think in the use case where we return a copy, we should make sure
the return value is unibyte when encoding and multibyte when decoding.
Otherwise, I think this is OK (for the master branch, obviously).

Thanks.