From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Date: Fri, 13 Aug 2021 13:05:47 +0100 Message-ID: References: <3d3f894f-a6fa-53ae-5d50-c3aa9bffc73e@daniel-mendler.de> <56ab18b1-4348-9b2c-85bb-af9b76cd429a@daniel-mendler.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25907"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 47711@debbugs.gnu.org, Stefan Monnier , 48841@debbugs.gnu.org, Dmitry Gutov To: Daniel Mendler Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Aug 13 14:07:23 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 1mEVy5-0006Se-Sv for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 13 Aug 2021 14:07:21 +0200 Original-Received: from localhost ([::1]:36858 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEVy4-0002av-02 for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 13 Aug 2021 08:07:20 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:33574) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mEVxm-0002aR-RA for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 08:07:06 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57368) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEVxm-0005ca-7F for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 08:07:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mEVxl-0007g5-Q9 for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 08:07:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 13 Aug 2021 12:07:01 +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.162885637429437 (code B ref 47711); Fri, 13 Aug 2021 12:07:01 +0000 Original-Received: (at 47711) by debbugs.gnu.org; 13 Aug 2021 12:06:14 +0000 Original-Received: from localhost ([127.0.0.1]:40678 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEVww-0007ec-H4 for submit@debbugs.gnu.org; Fri, 13 Aug 2021 08:06:14 -0400 Original-Received: from mail-pl1-f176.google.com ([209.85.214.176]:36863) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEVwr-0007dy-3b; Fri, 13 Aug 2021 08:06:09 -0400 Original-Received: by mail-pl1-f176.google.com with SMTP id f3so11648504plg.3; Fri, 13 Aug 2021 05:06:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=njSiQZeqjhLp38f+q1STdsHcPKVcgTrEHylx+LpFmXQ=; b=mRybdEqQZZnXQ+wU8nuO02C7OgdE70ro+46P0pATdrPTNswOqyV8UJ8PUS9VtW0SPf VzMVuQpMif7UNSR5qytOEKul+S5kzXJw5cRg0YDCVjkJpi7cBM9PcqAplrtBnUCQw3wo NLnASLQXzEy4e6KCvPgVgHnmGImAeETOsALWq5E3R4V1DL/XuzYwwdKoHXJzIm46fikI nEAdAoPQXgB0jaarEGkweITDlYPwhyv2TLda0XFpXP6+UutpTkTXhusniX90baGjkHNV BugikjEx/lXJhnWusr5PLPs6oddB10h+b4g+Hz+GKa0DpV+k5xP7pCNxlLPprpZiBO7a m0kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=njSiQZeqjhLp38f+q1STdsHcPKVcgTrEHylx+LpFmXQ=; b=W/imYrU92wciOyw2t/wJhW/DXkxz4KZI31KIKn0N8EWDDO4Qa/2DY0ZRyWfzxdd2XA vPd8KbhcfWqhhCgaTU4AqniQocifXMsCpcqILOUCFveLitTTKAJkah0bCEfHfNnzBcGZ lrPimWfTG84qiWMykUPaAM3yAINbVI7gVDmC+DZemRiVcb5OjMzH6Pj4rflBrycpYetu koVLblfSjjYwte4sCIY0/8Umw8o1vgkmSuZ1l8DWvz5B33opDq3Gz/F7EkgRJSfk3iqo D8alBpqI2rNfcx7LqGavxxxDsS/xbjJivMNgRqqjNAWv/0xujzpCy8r6YZ/MMKUYpc23 jCZQ== X-Gm-Message-State: AOAM533qhNr1IbKcVGH0kbevY5VqsnO9AGh+bm1INMIzJtDOdesWCtca 9Rjs+vA2DLBrTkkZhbJriucb9ffWcVR31B9ykwc= X-Google-Smtp-Source: ABdhPJyQMdCxprMZ56Ulp2KcbQOQamn6j8Gk84j90A/p0TH9H4ZILhFxlt32JRNgYDIOHmjKvSOaUG0hjQuyC993JyM= X-Received: by 2002:a17:90b:14b:: with SMTP id em11mr2340093pjb.125.1628856359053; Fri, 13 Aug 2021 05:05:59 -0700 (PDT) In-Reply-To: <56ab18b1-4348-9b2c-85bb-af9b76cd429a@daniel-mendler.de> 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:211739 Archived-At: On Fri, Aug 13, 2021 at 12:21 PM Daniel Mendler wr= ote: > No, this is not the case. There is no simple fix of the allocation issue > on the frontend side. I didn't claim that. At all. I claimed that the frontends that would be affected by the (small) backend patch are easy to adapt. I think you completely read past my idea. > The existing API `completion-all-completions` > necessarily has to allocate all the strings in order to attach > highlighting and scoring. The new API solves this in a clean way by > both deferring highlighting and scoring. I'm not sure you understand my alternative idea. As far as I understand (and have actually measured) the lines: ;; Don't modify the string itself. (setq str (copy-sequence str)) in minibuffer.el, in the function completion-pcm--hilit-commonality Are the cause of the problem that _I am talking about_ and that I have actually measured. Again you may be referring to a _different_ problem that I am unaware of. If one removes these lines, the process becomes much faster, but there is a problem with highlighting. My idea is indeed to defer highlighting by not setting the 'face property directly on that shared string, but some other property that is read later from the shared string by compliant frontents. If you have understood this idea, can you comment on it? (Preferably in terms of less adjectification regarding "cleanliness", but i= n terms of actual drawbacks/advantages?) The drawback that I can see in it is that frontends directly relying on 'face are broken by that patch. But according to Dmitry (and I tend to agree), it's quite easy to address those frontends. Most of them live in Emacs core or GNU Elpa. The advantage that I see is that those adaptations apart, it is a small localized and effective change. > I claim that my patch is easy to reason about and refactors the existing > code to address the exact problem we are having. Please take some time > in reviewing it. I am already taking some time. I need your assistance in explaining the problems first. I take into account your claims of cleanliness and elegance= , but in terms of their power of persuasion, they are much more limited than hard material evidence. > The main problem is that `completion-all-completions` allocates all the > strings every time the completions are filtered. This is the same > performance issue you encountered in fido-mode/icomplete-mode. OK. I encountered at least two different performance problems there, with quite different causes. So let's stick to the string-allocation problem. P= ost a code snippet that demonstrates the problem the way you see it/experience = it? Some benchmark code would be very welcome. You can probably grab my benchmarking code from that other bug. Then it becomes easy to study multiple solutions to that problem and choose the best one! > The second problem addressed by the new API > `completion-filter-completions` is that `completion-all-completions` is > limited in what it can return. For example it cannot return the end > position of the completion. And why is this a problem? Can you post an example of something you'd like to do, but can't? Regardless, it does seem indeed like a "second" pro= blem (as you state) so perhaps something that can be addressed separately. Is your particular solution to this second problem instrumental in solving the "main problem" > This is also solved by the new API. The new API is a clean extensible wa= y forward. I understand you've put time and effort into producing this work. We are all indebted and I promise to read it. But every API writer in history of programming has claimed those things and reality often shows otherwise. So it's not that your work can't be those things you claim, maybe it is, bu= t generally the larger and broader the work the harder it is to reason about. Jo=C3=A3o