From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Date: Sun, 29 Oct 2023 01:24:50 +0300 Message-ID: References: <3d3f894f-a6fa-53ae-5d50-c3aa9bffc73e@daniel-mendler.de> <56ab18b1-4348-9b2c-85bb-af9b76cd429a@daniel-mendler.de> <328f87eb-6474-1442-e1ca-9ae8deb2a84a@yandex.ru> <83fsvcbio7.fsf@gnu.org> <9f432d18-e70f-54c1-0173-1899fb66d176@gutov.dev> <877cnafv39.fsf@gmail.com> <9447dde3-b8e7-2ec0-9a9c-72c4cf9d12a8@gutov.dev> <7d14bc13-4419-816c-5708-c42988c39c02@gutov.dev> <5d0a78cc-4fa0-ef04-3462-1826f17d7d56@gutov.dev> <877cn8asud.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37909"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Cc: Daniel Mendler , Eli Zaretskii , Stefan Monnier , 47711@debbugs.gnu.org To: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Oct 29 00:25:52 2023 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 1qwrkc-0009eO-Sz for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 29 Oct 2023 00:25:52 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qwrkM-0004Yw-6w; Sat, 28 Oct 2023 18:25:34 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qwrkI-0004Yg-RS for bug-gnu-emacs@gnu.org; Sat, 28 Oct 2023 18:25:30 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qwrkI-0004OR-JK for bug-gnu-emacs@gnu.org; Sat, 28 Oct 2023 18:25:30 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qwrko-0008SY-Fs for bug-gnu-emacs@gnu.org; Sat, 28 Oct 2023 18:26:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 28 Oct 2023 22:26:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47711 X-GNU-PR-Package: emacs Original-Received: via spool by 47711-submit@debbugs.gnu.org id=B47711.169853194032481 (code B ref 47711); Sat, 28 Oct 2023 22:26:02 +0000 Original-Received: (at 47711) by debbugs.gnu.org; 28 Oct 2023 22:25:40 +0000 Original-Received: from localhost ([127.0.0.1]:39770 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qwrkR-0008Rp-Pf for submit@debbugs.gnu.org; Sat, 28 Oct 2023 18:25:40 -0400 Original-Received: from out4-smtp.messagingengine.com ([66.111.4.28]:60985) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qwrkM-0008RR-C8 for 47711@debbugs.gnu.org; Sat, 28 Oct 2023 18:25:38 -0400 Original-Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 1E3C35C0129; Sat, 28 Oct 2023 18:24:56 -0400 (EDT) Original-Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Sat, 28 Oct 2023 18:24:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm2; t= 1698531896; x=1698618296; bh=nf4kMJot6EJZiVhkpKbPRgNc78nBCOY3SOC RJxE/bow=; b=KNcJ3kH9/zHCpLGs3xTDL1ylU03whUVhsJl9pVODnBqYrjsQELS QqZHsHn+ROTIFS5eCU3oSTGVj0VpoV9RpW1lK0o7C6e91KiqDVEPmrp/1SorTNbf Row/083GcvcsMztUWsj1L1gUU/SS4K17ps/f8IpzbSlqijIAhRM9fPTY3+wpCS6s YMJk5O3qSRsxG0JiT0sjNKS47u9hzh6e5AFY3dDafMWpsuYqICTUTFyJQSulbgEL ytGbH3BmIf5H8dm/liKipPstm49gyBXYF6kA2JkqJwMW22Dd4Nv+lY5a4Dk1gQlG CBSA8enJWJF06+M29iTe/dCd0ckqgQBs8QQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1698531896; x=1698618296; bh=nf4kMJot6EJZiVhkpKbPRgNc78nBCOY3SOC RJxE/bow=; b=gABs6itfTuowYpM06QW2Zvzi6r/wRqIvOKGTChfeQmcgokGBDMK b0ukvIt8EWbOrGU92IETCP4GI+s29gCfkcyto56LayNvWIIEoigIWjtV8zBhkuC6 aAvhWMXIYpLBQaAaiFd8nzv62xtIQv/WRhYImg+EM+ID7HB9sObMuLKbXQMxe2Wd V82mbFbVYO5Hct8OqQHd/FNY0biEXVV68C98tCJ1L2u5avAkWG3MtdZKQUvelWC8 Z6tl+rJ4kIJ3vVaMo3F+/TMtTIvsNRPbaa6Ztv2UQMQbP90Zmnc8Imo/Liqq6ukL LNIXD5y9E4DzXgJ1SgyjBQQs7n/0iJVu13Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrleejgdduudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefkffggfgfuvfevfhfhjggtgfesthekredttdefjeenucfhrhhomhepffhmihht rhihucfiuhhtohhvuceoughmihhtrhihsehguhhtohhvrdguvghvqeenucggtffrrghtth gvrhhnpefftedtvdevuedvffffvdethedvveektedvhfdtfffhfeeklefhleeuhfehvdel jeenucffohhmrghinhepughifhhfrdhhvghrvgdpghhithhhuhgsrdgtohhmnecuvehluh hsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepughmihhtrhihsehg uhhtohhvrdguvghv X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat, 28 Oct 2023 18:24:53 -0400 (EDT) Content-Language: en-US In-Reply-To: <877cn8asud.fsf@gmail.com> 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-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:273471 Archived-At: On 27/10/2023 20:16, João Távora wrote: > My attempts to fix the optimization to work correctly in all cases > complicated the code and then slowed down the bare completion-read case. > Not worth it IMO, therefore I have reverted it in the branch and in the > latest patch I attach (lazy-hilit-2023-v4.diff). Here are the latest > measurements for the "yoyo" test, now considering the C-h v scenario: > > ;; Daniel+Dmitry completin-read: 0.834s avg > ;; Daniel+Dmitry C-h v: 0.988s avg > > ;; lazy-hilit completing-read: 0.825s avg > ;; lazy-hilit C-h v: 0.946s avg > > Again, this shows my patch to be about 2-4% faster, though not really > relevant. > > Again, the optimization I removed, if it were done correctly (which it > wasn't) could maybe shave off another 8-9% off that. Thanks. My measurements are similar, except the difference switch the other way a little bit. It might depend on the particulars of the individual machine anyway. I also tried to compare the perf for 150K symbols, in case either the filtering or the sorting (which should have different complexities) would come to the front, but no such luck: lazy-hilit 300000 completing-read 0.6063497 C-h v 0.72435995 150000 completing-read 0.316961 C-h v 0.39933575 d+d 300000 completing-read 0.571481 C-h v 0.69817995 150000 completing-read 0.308940 C-h v 0.350437 All averages made using 'M-x calc-grab-region' followed by 'u M'. >> But there are differences. The first is that the highlighter function >> takes one string as an argument instead of a collection. I mentioned >> this before, this will be much handier to use in company-capf. > > Don't fully follow this. Can you perhaps show two versions (or two > snippets) of the company-capf code, the handy and the non-handy version? I just meant that your version will be easier to use in company--match-from-capf-face (because it works on individual completions). >> Third, it made a principled stance to avoid altering the original >> strings, even the non-visual text properties. This approach could be >> adopted piecewise from Daniel's patch, especially if the performance >> ends up the same or insignificantly different in practical usage. > > If we really wanted to, we could also adopt the non-propertizing > approach in my lazy-hilit patch, by calculating the score "just in > time", much like Daniel's patch does it. But it should be clear that > what we save in allocation in completion-pcm--hilit-commonality, we'll > pay for in the same amount in consing later. So no performance benefit. Not for performance, no. Although the way it separates the sorting into its own phase makes it easier to reason about that particular cost. And for 300000 symbols, scoring and sorting really take the most time, e.g. about 2/3rds. Which might help with optimizing it further down in the future, somehow, > And if we do that, don't forget there will be the ugly "unquoted" > complication to deal with. Then again, in my understanding that > complication is due to similar problem of mixing business and display > logic. That's assuming I understand this comment in minibuffer.el > correctly: > > ;; Here we choose to quote all elements returned, but a better option > ;; would be to return unquoted elements together with a function to > ;; requote them, so that *Completions* can show nicer unquoted values > ;; which only get quoted when needed by choose-completion. > > So I would look into solving that first, instead of allowing the > "unquoted" hacks to spread even further in minibuffer.el I don't really understand this quoting-requoting business, never dug into the feature or the code. But perhaps keeping the original string might even help avoid the "requoting" step? Though that would depend on which version of the string the scoring and highligher functions expect to work on. Speaking of the comment, it sounds like the said "requote function" would need to be passed up the call stack and used according to some protocol. The idea itself reminds me of the proposal described in https://github.com/company-mode/company-mode/discussions/1422 (it was also previously brought up on the lsp-mode tracker). It seems like either the completion tables would need a new method, or the capf tuples - a new function property, which all UIs using would need to start supporting in lock-step. At least that's the case if we use that to solve the requoting issue (the LSP clients' migration to use complete-function can be done more easily). So far you advocated toward avoiding breaking changes while implementing the present performance improvement. Daniel's argument that the quoting completion tables are already slow enough sounds very reasonable to me, that an extra text property round-trip won't be noticeable. And the current version of the patch is functional and passes all tests, so it's not clear which other places would the "hack" need to spead into. Unless it helps with reducing code, etc, as per my vague guess above. But if course it would be nice to avoid the wart, so if you have any better ideas, they are welcome.