|
From: | Jim Porter |
Subject: | bug#51699: 29.0.50; [PATCH] Improve performance of 'file-name-case-insensitive-p' for Tramp files |
Date: | Tue, 9 Nov 2021 13:22:11 -0800 |
On 11/9/2021 11:34 AM, Michael Albinus wrote:
This is obviously fine, so I've pushed this to master. Thanks for the improvement!
Thanks for merging!
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?
There's `tramp-archive-file-name-p' and `tramp-crypt-file-name-p', which both want a string filename. I looked over the implementation for those and couldn't figure out an easy way to convert them to take a VEC. Maybe it's possible though. I also looked into passing both the string form and the vec form as separate arguments, but that turned out to be even more complex than this implementation.
In addition, I did it this way to prevent any breakage for third-party code that calls `tramp-register-foreign-file-name-handler'. If we change `tramp-find-foreign-file-name-handler' to pass a VEC all the time, then any code out there that expects a string will break. This is probably rare, but I've seen a few examples of people doing stuff like this over the years, e.g. <https://github.com/thinkiny/emacs.d/blob/master/lisp/tramp-jumper.el>.
I'm not quite sure what you mean by the "usual error handling" though. If there's a simpler way to do this, I'm happy to change the implementation. So long as we can minimize the number of times `tramp-dissect-file-name' is called, it should be possible to get similar performance improvements to the current version of my patch.
[Prev in Thread] | Current Thread | [Next in Thread] |