From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Sergey Vinokurov Newsgroups: gmane.emacs.bugs Subject: bug#53242: [PATCH] unify reads from local_var_alist Date: Sat, 15 Jan 2022 17:54:02 +0000 Message-ID: <6868768b-ed98-930f-0d50-74d961db4c0c@gmail.com> References: <50d65dbc-f3d6-86fb-6a7c-9200a6525ec2@gmail.com> <83zgny20z2.fsf@gnu.org> <74db84b1-5433-dfb8-8ee3-9d86d8fc9be7@gmail.com> <83ilum16qq.fsf@gnu.org> <1e4edf7d-7f9c-ef56-7870-6f0f3567a40d@gmail.com> <83ee591mkr.fsf@gnu.org> <15fa68fa-2bb2-eaf4-885b-abdff1e3f61f@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="28207"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Cc: 53242@debbugs.gnu.org To: Corwin Brust Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jan 15 18:55:44 2022 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 1n8nHD-00079K-Js for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 15 Jan 2022 18:55:43 +0100 Original-Received: from localhost ([::1]:57902 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n8nHB-0008Vv-0w for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 15 Jan 2022 12:55:42 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:47926) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n8nGY-0008VV-Rw for bug-gnu-emacs@gnu.org; Sat, 15 Jan 2022 12:55:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48782) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n8nGY-0006ne-HQ for bug-gnu-emacs@gnu.org; Sat, 15 Jan 2022 12:55:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1n8nGY-0002re-FH for bug-gnu-emacs@gnu.org; Sat, 15 Jan 2022 12:55:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Sergey Vinokurov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 15 Jan 2022 17:55:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 53242 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 53242-submit@debbugs.gnu.org id=B53242.164226925310946 (code B ref 53242); Sat, 15 Jan 2022 17:55:02 +0000 Original-Received: (at 53242) by debbugs.gnu.org; 15 Jan 2022 17:54:13 +0000 Original-Received: from localhost ([127.0.0.1]:41685 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1n8nFl-0002qU-17 for submit@debbugs.gnu.org; Sat, 15 Jan 2022 12:54:13 -0500 Original-Received: from mail-wm1-f44.google.com ([209.85.128.44]:54115) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1n8nFh-0002qD-T3 for 53242@debbugs.gnu.org; Sat, 15 Jan 2022 12:54:11 -0500 Original-Received: by mail-wm1-f44.google.com with SMTP id k5so3032051wmj.3 for <53242@debbugs.gnu.org>; Sat, 15 Jan 2022 09:54:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=VkrsI2GQkTLQy5S5YxfM0y7jaMFDTQq/xi8CwsuQROE=; b=P4fN8GHKfVqWdMykmb3W63fF8UPZ+e69jtmPT6sqJjXcvKu4ZNs9nZ8VeBvnfM80iP mtWhHM3N/tiJM/8xy+CK0pkklwkd/UDdP8r8ga1sp4sc+mQKNqkadimJCqT+4Ihd/wjb RJzROPjUkn6HTm6Cdod9qiXn46L4vNKB01uCCjiJ1xRQscVSXxSe9h+9jZye2AjFWVhJ llEVve565nNMlWQkBPfBSmKYpPHknrYtM1SIfNjjnGA2UkW8lBqY+1D15+wCukQrwZ1C 3L6rWlA3eftPL1EVFxPzLeWwETX7S1QLNoyh38WtgLa+xAqZyrhbQC1DLaheMXFUnlR/ +T4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=VkrsI2GQkTLQy5S5YxfM0y7jaMFDTQq/xi8CwsuQROE=; b=CBBHrKa6gwbjy3hPfb/xdPOF/ZI666UUZNfCPwKv3T5Cc++k/iZwxFk6OQBj3INrAP aWYIwWNwVEuma1KDpUhdM1PJcGB6dmkMlz7QNHECZL+0Lks3jdUonXqpy5FuFitpFEfl 7R8TapwKZwvKgG+qaLpUSyh5locXyAkRS/VcGUoakDawykH8GKjfdzYDtXtBszZvIXdH AM8PVlSSPqXl9jxO89JtDgM43Uk/vAKsUcHogwuNZaxnZf6V1TCDGvU+XHh8S0VhTaS+ WYGm5sQIUQ+K8cfOyXhuz8i5sh6352Ra/YVVduaaIxEFzqLpBnIrpKlfn3gY3B1Bk+2R QcHA== X-Gm-Message-State: AOAM5331JviqKjc+wmrtUx7nRLPM77BHFj7KgEwW13I0uBbcLYF4tK6A sXGZocGZOA4cx2D55wGzC30= X-Google-Smtp-Source: ABdhPJwJQH4s5HWqu4vQdXnWN6on245rzaYVjd9tnO9643DBPHkc7/ClOGYjaytPQTdPoGfXSsV+EQ== X-Received: by 2002:a05:600c:4e4b:: with SMTP id e11mr13075390wmq.28.1642269243762; Sat, 15 Jan 2022 09:54:03 -0800 (PST) Original-Received: from ?IPV6:2a01:4b00:8697:de00:607c:1dff:fe2e:2452? ([2a01:4b00:8697:de00:607c:1dff:fe2e:2452]) by smtp.gmail.com with ESMTPSA id p18sm8227942wma.40.2022.01.15.09.54.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 15 Jan 2022 09:54:02 -0800 (PST) Content-Language: en-GB In-Reply-To: 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:224323 Archived-At: On 15/01/2022 16:02, Corwin Brust wrote: > I tried to follow this conversation but it wasn't clear to me what out > motive is for this change. The motive is that prior to change the alist with buffer-local variables was handled inconsistently. Sometimes with Fassoc, other times with Fassq and even assq_no_quit (the one that doesn't allow interrupts). Since the keys of alist are symbols (variable names), it doesn't make sense to use Fassoc which compares them with Fequal - an Fassq which does the comparison which simpler Feq would suffice. > I had understood we typically make (especially in the c sources) our > changes to achieve specific, tangible improvement.  Is that the case > here?  is the particularly oppressive 'tech debt'?  In the latter case, > does history reflect consideration wrt the original selections in each > of the various cases we hereby change? I don't know whether this is an oppressive tech debt, but from my perspective I have taken a look over handling of buffer-local variables during hacking some elisp code and saw the inconsistency. My patch is just an effort to reduce it and try to make Emacs a little bit better than it was before. I don't know what the improvement will be, probably in will be pretty small. My main consideration for selecting which function to use it to look at the types, notice that this in an associative list with symbols as keys and select the most appropriate function that would handle lookups in the list. > Also (and especially if we must 'clean for the sake of cleanliness'), > could we prefer the (seeming more conservative of UX) interruptable > varient in this case?  (Is that very costly? How costly and how have we > measured that?) Some parts before the change were already using uninterruptible variant. The Fassq does more work than assq_no_quit because it's not only interruptible but also checks for circular lists whereas assq_no_quit does not handle them correctly and would just loop forever. It is safe to use assq_no_quit for buffer-local variables because this in Emacs internal structure not visible to the user, Emacs fully maintains it and does not make it into a circular list. > Please consider the case of a package developer who may be abusing buffer-local vars during experiments. It seems this will cause much more ’oops, time to kill Emacs/grab a coffee'. I think it's unrealistic to introduce, even accidentally, enough buffer-local variables that lack of interruptibility in these particular functions will start to show. This is based on the following benchmark, which I encourage everyone to try out. It creates a list of length n and does one lookup into it. This corresponds to a buffer having n local variables and the lookup is the operation we're arguing about (Fassq vs assq_no_quit). The assq_no_quit is not exposed in elisp as it's not safe so the benchmark uses Fassq but assq_no_quit will be pretty close as it does roughly the same amount of work. (defun mk-list (n) (let ((res nil)) (dotimes (i n) (push (cons i nil) res)) res)) (byte-compile #'mk-list) (let* ((n 100000) (xs (mk-list n))) (benchmark-call (lambda () (assq 'foo xs)) 1)) It takes pretty large n to get the lookup take significant amount of time (please note that list creation time is not included in the calculation as it has nothing to do with Fassq vs assq_no_quit, so look at what benchmark-call returns and not on how long it all subjectively takes). On my machine I need 10 000 000 elements for lookup to take 110 ms, which is a noticeable amount of time (probably still bearable to work with). For Emacs to "freeze" over, say 10 second, the number of variables introduced has to be even higher. Is having millions (or hundreds of millions...) of buffer-local variables a reasonable scenario to consider? Please keep in mind that even if this occurs, interruptibility will not make lookups finish faster (even probably slower because of additional work that Fassq does compared to assq_no_quit). Yes, user would be able to C-g out of an operation if we use Fassq. But the operation will still be as much slow the next time it's performed. User will do the 'oops, time to kill Emacs/grab a coffee' sequence anyway in this case since any commands looking at local variables will be affected. AND this is the case with Emacs prior to my patch. Please do a benchmark and define 100 000 000 local variables and see whether Emacs works appropriately well in this case. I argue that if we're really concerned what would happen when someone defines a few hundred million local variables then we should not be asking a question whether reads of said variables are interruptible but should be asking a question whether linked list is the appropriate data structure to store local variables in the first place (spoiler alert: it's not).