From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?B?Sm/Do28gVMOhdm9yYQ==?= Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations Date: Tue, 01 Jun 2021 19:47:14 +0100 Message-ID: <8735u18lsd.fsf@gmail.com> References: <87h7irss50.fsf@mail.linkov.net> <43d1599e-2ba9-2efb-45c3-76c67d29a69d@daniel-mendler.de> <87tumrgqrb.fsf@gmail.com> <87tumq92pe.fsf@mail.linkov.net> <87lf82g10g.fsf@gmail.com> <87y2c24lww.fsf@mail.linkov.net> <871r9t2lsy.fsf@mail.linkov.net> <22880197-6d05-c821-2c58-328ed3cfc02e@daniel-mendler.de> <87eedruui3.fsf@gmail.com> <8dd915fe-fe67-2a45-67ff-8aaa3e9b9aca@daniel-mendler.de> <878s3zuq47.fsf@gmail.com> <09f2a253-84ba-5cfd-552e-0b89109864a5@daniel-mendler.de> <875yyxaoxp.fsf@gmail.com> <871r9laj6a.fsf@gmail.com> <1b73a130-204c-76fb-2b60-02b814aee0f0@daniel-mendler.de> <87r1hl8xom.fsf@gmail.com> <878s3t8tzw.fsf@gmail.com> <3c68bd00-70ca-fa18-f9b8-cd03029f9294@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="22466"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Juri Linkov , "emacs-devel@gnu.org" , monnier@iro.umontreal.ca, Dmitry Gutov To: Daniel Mendler Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Jun 01 20:48:34 2021 Return-path: Envelope-to: ged-emacs-devel@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 1lo9RK-0005cH-Rd for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Jun 2021 20:48:34 +0200 Original-Received: from localhost ([::1]:35322 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lo9RJ-0000wJ-UN for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Jun 2021 14:48:33 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:52544) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lo9Q9-0007xK-2a for emacs-devel@gnu.org; Tue, 01 Jun 2021 14:47:23 -0400 Original-Received: from mail-wr1-x42f.google.com ([2a00:1450:4864:20::42f]:36520) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lo9Q6-0004Vp-FY for emacs-devel@gnu.org; Tue, 01 Jun 2021 14:47:20 -0400 Original-Received: by mail-wr1-x42f.google.com with SMTP id n4so15420080wrw.3 for ; Tue, 01 Jun 2021 11:47:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=0oQwGKaTklO122FmjH0jwKw7HsD/U4OI7W9XxmSKWkE=; b=X+J3hIC8oOuWKmcTr6RnQKL0iJm3OBYp3vwAxBM22KaejTu+UAQb7Riabv/xPpPOQv neOdhUOTW8MVQcDPm2/G7If8XEcxD7t6mUtwuUAzIbI+yVSAQ2g66DIeEI603FiQJhRe RF95aA5ZSx7DKBF1WjQqHFWp9wNIiY8/Ckc4GNIqjCP0pGYEaK0B05vtS6XIAMWu7fZ9 J6eVFzLorwfgCvRPKx3FKrKSOB7l2pbR1p2M43ltCzqMGTT/+tDI98I362IMDGFY/yW0 KHI7Bl7UsBYbuxBmULL2QhjnSOOdnyBuOtvn7eghrit18Oufd/8M+42zvkIB1lrJENyW 2D+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=0oQwGKaTklO122FmjH0jwKw7HsD/U4OI7W9XxmSKWkE=; b=q2ksmX/rGFOQs53BfS1bLTb0Sogmj2hMo7VeX2emo0eMObr1XuCz6T2ggALs1OFX0F NUamG0K6/kYrbvd7D8zQc7BzfeOAoPR9x0RVKgpK5muwo3upKmFTVxWe3XSY6tGfK1XE Rm9xEPqeQn2Wyks16jaVtH0p4e5/tAcgc4Ixmw5uPs/iOwsq0sRl6SxoZAssbF4wgbna HNpajWBh+y/EO+eUJzd1EF75Wwa0sip+kGYy5yrO+B69MiuVdlRbwKXtKurI4wN6A3rH pmX2lVwxWnsWn0mPT4CkW0uPHIzjbfjIKfq8Idxq47j0JDpZsehXSEkFT9bVAc+767Ye Nb0A== X-Gm-Message-State: AOAM530CKwXg0Pq3UhqsKbuhZJWxfIBW2OfgvPVf+/HQs0jBwBUBVYb3 Q4JRteIgij7zXFYjAtVvoG8= X-Google-Smtp-Source: ABdhPJxxvve3QYDpKQFoWNlnTqrS76avTrxutmtkofJh1tYgOdl56z382Z1wiwJWFAy+hf5RGfH/EA== X-Received: by 2002:adf:f7d1:: with SMTP id a17mr11457459wrq.84.1622573236553; Tue, 01 Jun 2021 11:47:16 -0700 (PDT) Original-Received: from krug (6.213.115.89.rev.vodafone.pt. [89.115.213.6]) by smtp.gmail.com with ESMTPSA id v8sm4623114wrc.29.2021.06.01.11.47.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Jun 2021 11:47:16 -0700 (PDT) In-Reply-To: <3c68bd00-70ca-fa18-f9b8-cd03029f9294@daniel-mendler.de> (Daniel Mendler's message of "Tue, 1 Jun 2021 18:00:56 +0200") Received-SPF: pass client-ip=2a00:1450:4864:20::42f; envelope-from=joaotavora@gmail.com; helo=mail-wr1-x42f.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:270242 Archived-At: Daniel Mendler writes: > On 6/1/21 5:49 PM, Jo=C3=A3o T=C3=A1vora wrote: >> But do you have any evidence to suggest that making a cons and >> allocating a function is detrimental to this?? I just tried this small >> patch to the function you suggested as a potential hog, since it >> iterates all the completions. >> >> So basically the same. Cons and lambda's are cheap. > > The allocations itself are cheap but they put unnecessary pressure on > the GC. The current version avoids this. This matters when scrolling > through long candidate lists. I've now benchmarked correctly (the previous benchmarks were completely bogus) ;; current (20.18251486 3 0.3386851209999975) (19.646946365 3 0.3477981569999997) (20.10486081 3 0.34829881900000004) ;; allocate extra cons and non-capturing function (21.353061817 3 0.3557993659999994) (21.124979874999998 3 0.361779398000003) (21.009165698 3 0.36675408300000356) ;; allocate extra cons and closure function (22.860096656 4 0.49817515299999826) (22.091684587000003 4 0.4981799040000041) (21.948139371 4 0.505049483999997) ;; allocate extra cons and string (24.102438947 4 0.5202420680000017) This was the test: (length joaot/test) ;; 44547 taken from the `read-char-by-name' table (let ((test (mapcar #'substring-no-properties joaot/test))) (garbage-collect) (benchmark-run 1 (minibuffer--group-by #'mule--ucs-names-group #'identity test) (setq test nil) (garbage-collect))) I really doubt this "matters when scrolling", as grouping would be called once per filtering, and scrolling is just getting at each candidate's transform. If anything, if you take the last option, it could _speed up_ scrolling (and the cost of some more initial latency). >>> The current implementation is simple enough and also avoids allocation >>> of the cons pair and the allocation of the lambda as in your proposal. >>> The efficiency is crucial here. >>=20 >> I believe you, but I haven't seen any evidence (yet) to back up the >> claim. But instead of me whistling premature optimization song, feel >> free to point me to some numbers or something. > > The current version is proven in my packages and performs well. I don't > see a point in replacing it to a more complicated and less efficient > version. I've told you the point. A simpler minibuffer.el code, possibly with less branching and logic, which might even be faster. >>> Furthermore I argue that the current implementation is simpler to >>> understand for the completion table authors than your counter >>> proposal, which introduces a deferred computation with the lambda. >>=20 >> It's like the affixation-function thing, I don't think it makes sense >> for client code to do these IFs, much as I dont' think it makes sense >> for them to do MAPCARs. > > No it is not. Look, this is just my opinion, you don't need to concur with it it, but there's no need to say "no it's not" to an opinion. > completion table authors. Furthermore I disagree with you that the > minibuffer.el code is messy. Surely, you jest, lol. minibuffer-completion-help is a mess. That's ok! Most of minibuffer.el is a mess. There's no good code, in general, so let's not get emotionally attached to ours. > The suggestion of yours to mutate the candidates by putting a property > instead of threading the group function through the call sites is much > worse. In what way? Really, all other things being equal, in what way is threading arguments through lots of functions, bloating the arglist, implementations and the docstrings of said functions, calling group-fun multiple times and in multiple places in minibuffer.el better than _not_ doing those things? The only argument is if there was a terrible performance cost, which was indeed the first and only real argument you advanced. But I've not seen evidence of that reality. > Your version is not an improvement. Please read through the old > discussion and consider my arguments. I also don't see a point in > rediscussing this as long as you don't make a proposal which is a clear > improvement. Look, I get it that you oppose this suggestion and are happy with your implementation. That's fine it's your right to be. But there's no point in so aggresively gainsaying a suggestion while not offering any material arguments. APIs in master aren't meant to cristalize, they are meant to be iterated. Jo=C3=A3o