GNU bug report logs - #51699
29.0.50; [PATCH] Improve performance of 'file-name-case-insensitive-p' for Tramp files

Previous Next

Package: emacs;

Reported by: Jim Porter <jporterbugs <at> gmail.com>

Date: Tue, 9 Nov 2021 03:53:02 UTC

Severity: normal

Tags: patch

Found in version 29.0.50

Fixed in version 29.1

Done: Michael Albinus <michael.albinus <at> gmx.de>

Bug is archived. No further changes may be made.

Full log


Message #8 received at 51699 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 51699 <at> debbugs.gnu.org
Subject: Re: bug#51699: 29.0.50; [PATCH] Improve performance of
 'file-name-case-insensitive-p' for Tramp files
Date: Tue, 09 Nov 2021 20:34:01 +0100
Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

> 1) 'tramp-handle-file-name-case-insensitive-p' calling 'file-remote-p'
> and 'expand-file-name'
>
> Since 'file-remote-p' only needed to check whether a connection was
> already established, it could be replaced with this (thanks to Michael
> Albinus for the pointer):
>
>   (let ((non-essential t)) (tramp-connectable-p v))
>
> 'expand-file-name' also had room for a small optimization, since it
> previously called 'tramp-connectable-p' (which dissects the file if
> it's not already) and then 'with-parsed-tramp-file-name' (which
> dissects it again). I reversed the order so now there's one fewer
> dissection, and it's a bit faster.

This is obviously fine, so I've pushed this to master. Thanks for the
improvement!

> 2) Potential handlers in 'tramp-find-foreign-file-name-handler' each
> dissect the file name
>
> Most Tramp methods have a 'tramp-FOO-file-name-p', and most of *those*
> take a file name string and dissect it. This is a lot of duplicated
> effort, so I modified 'tramp-find-foreign-file-name-handler' to pass
> the dissected file name to any of the functions that support it (this
> is indicated by an 'accepts-vec' property on the function). This
> probably warrants some documentation (at least a NEWS entry), but I
> wanted to be sure the strategy made sense before I wrote any docs.

Yes, this makes sense, and it works in my environment (more regression
tests running). I don't understand why you need the 'accepts-vec'
property -- is there any operation left, which is passed to
`tramp-find-foreign-file-name-handler' and which doesn't accept a VEC,
after applying your patch? And if yes, couldn't we apply usual error
handling?

We shall not add this additonal complexity to Tramp.

Best regards, Michael.




This bug report was last modified 3 years and 248 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.