From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.bugs Subject: bug#51699: 29.0.50; [PATCH] Improve performance of 'file-name-case-insensitive-p' for Tramp files Date: Wed, 10 Nov 2021 13:17:08 +0100 Message-ID: <87wnlgtdbv.fsf@gmx.de> References: <0b41b0c1-0887-8a21-e2e4-eeb35d5a9bd0@gmail.com> <87ee7pqg2e.fsf@gmx.de> <5834652a-5624-a463-fe56-5c8f0841787b@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="36888"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 51699@debbugs.gnu.org To: Jim Porter Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Nov 10 13:18:44 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mkmYt-0009Nl-Nu for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 10 Nov 2021 13:18:43 +0100 Original-Received: from localhost ([::1]:42730 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mkmYs-0005O3-MI for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 10 Nov 2021 07:18:42 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:38124) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mkmYE-0004ur-9M for bug-gnu-emacs@gnu.org; Wed, 10 Nov 2021 07:18:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53873) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mkmYD-0006Zp-UE for bug-gnu-emacs@gnu.org; Wed, 10 Nov 2021 07:18:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mkmYD-0008G9-Kp for bug-gnu-emacs@gnu.org; Wed, 10 Nov 2021 07:18:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Michael Albinus Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 10 Nov 2021 12:18:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 51699 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 51699-submit@debbugs.gnu.org id=B51699.163654663831684 (code B ref 51699); Wed, 10 Nov 2021 12:18:01 +0000 Original-Received: (at 51699) by debbugs.gnu.org; 10 Nov 2021 12:17:18 +0000 Original-Received: from localhost ([127.0.0.1]:37181 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mkmXW-0008Ey-4f for submit@debbugs.gnu.org; Wed, 10 Nov 2021 07:17:18 -0500 Original-Received: from mout.gmx.net ([212.227.17.20]:54607) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mkmXU-0008Em-Qv for 51699@debbugs.gnu.org; Wed, 10 Nov 2021 07:17:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1636546629; bh=vIW21CoB2ycGnJUw8FJ8mN5FqtXUyElxsEwDhS625m8=; h=X-UI-Sender-Class:From:To:Cc:Subject:References:Date:In-Reply-To; b=AeOct/IoW51+lfsoOWaex3iTmwnWGHvHgpMOUmaYqoHCz6XgO9ywC8rmjOJ5hOB+l svQ3Ru4T56uXwLOQC/Ekh6DInSaqWNPMEpPd0ps2p0QR+GLFkLeNv1YaZfaBYnv1os wtSu5JYDii3/Jr5jtitT2r0b7/s4vudPRbc9+WhA= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from gandalf.gmx.de ([213.220.148.170]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1Mplbx-1mNDVQ2jPB-00qBfO; Wed, 10 Nov 2021 13:17:09 +0100 In-Reply-To: <5834652a-5624-a463-fe56-5c8f0841787b@gmail.com> (Jim Porter's message of "Tue, 9 Nov 2021 13:22:11 -0800") X-Provags-ID: V03:K1:MFhxCdLLX+CeeSkeU6e0JU483C94T8P9kK5fpxrE6BxaeskY1fW T5vAyxv2ya9iwOVcR+lBrgIMEe0zw3fQYsbjvSUhTOs4I3R/ZQeTJhstUsY0D02aNLp0a6f i2Lqd+oT/dZA+OOO+aTxAGSgsQ5W0dlJT3DVYDrzVYN6Sxs8KBH/8y6oJlcxQRBjQBlRUhZ eE6auCgHJNoqUq1VDm3Ig== X-UI-Out-Filterresults: notjunk:1;V03:K0:lxY2i3X3N54=:X8RZ/t879fu1Swcpo5jHUT ykYBVwGI10xpps0yAtTwTznT+Hw3Mhmygr6qOmxm7PaKkJF+fYPslPMYBfZvkstWO6N3/fMfw ePAXfPWsQDWNRmHjnMwcVefexRHhFej9osh/0YG9tUfk3oZJ+Yk7Ch/H9lrwxFeih9FlZWglv G5ZGGK1e4cJfEyySlp6u1Tq9KURBYNs48sbRq2HeYCs4d3tktqjn9o8PIzRyBcZ+vXpLjKLSX NcKqK+IQhnE9ebITtySmh8LB9WDVoojrEifo7vHZ0aFV4D9WehEWUmQPDVh98I+kjj7QAMaDb TdVR4J4VfW6TSU9xe0LIkeRBRuAvwYyYQ+FceIdw5QNU8xaNSAo5rh/IQpWqnK3YiTASZyj8b L0DFodFFSmUjHHsTui1Dce/UoUM+2o0H2qSaO6mQIdCirNLpL/nx1+xttu63zRwdECxOZvuwX FZyByjYkypPltXlEAaWXyDHZAN2BfkVIusph1y5h6fy9jqdsp3pkGQ/tZmHlOMbaY8Uoh3YBJ JK5US9sU5JoKxgFVpS6v3XuM0Elzkb6DLYJV9HAJAxZQecvNX88ktbFTaZfHzgytYZRL/p5y4 WnD5JnP2xu4sMPwTG1k9nnMEPhHYrUqkghHLJNiGg88XSsYluHHF4OzZ/oTM8oRBfpJOS5xxn xITUH/qRGmRxx5icrQvLf5Lj2lHxp9k0mj+HGWcHLtTuxif8A9imK3/HYA/6QFMoRnZ7dFVrW 6gPiDRFRc0eS3SLtSlDFdOc/m9OnnRY8wODGmIfVPOzOg50RbZwtzCXWSO9f6bykuExKzbwc X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:219530 Archived-At: Jim Porter writes: Hi Jim, >>> 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. `tramp-find-foreign-file-name-handler' loops through `tramp-register-foreign-file-name-handler'. Only operations registered there need to support a VEC-OR-FILENAME argument. `tramp-archive-file-name-p' and `tramp-crypt-file-name-p' are not registered, so no change is needed. > 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. . Yes, and there's also . These packages use a lot of internal Tramp functions which are not documented publicly. So they have always the risk that something fails. We could (and should) inform the authors of both packages, that the signature for the `tramp-FOO-file-name-p' functions will change with Tramp 2.6. As I can see, there's no problem to adapt `magit-tramp-file-name-p' and `tramp-jumper-file-name-p'. And yes, an entry in etc/NEWS should be added as well. > 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. The most simple approach in `tramp-find-foreign-file-name-handler' is --8<---------------cut here---------------start------------->8--- (when (ignore-errors (funcall (car elt) vec)) --8<---------------cut here---------------end--------------->8--- A little bit more friendly for debugging: --8<---------------cut here---------------start------------->8--- ;; The signature of `tramp-FOO-file-name-p' has changed, it ;; expects a VEC here. (when (with-demoted-errors "Error: %S" (funcall (car elt) vec)) --8<---------------cut here---------------end--------------->8--- Best regards, Michael.