From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#46326: 27.1.50; Excessive memory allocations with minibuffer-with-setup-hook Date: Fri, 23 Apr 2021 14:26:57 -0400 Message-ID: References: <62c490ed0d8d24d8b259ac1ba55ea79e@mendler.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="5365"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 46326@debbugs.gnu.org To: mail@daniel-mendler.de Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Apr 23 20:28:13 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 1la0XF-0000xx-8r for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 23 Apr 2021 20:28:13 +0200 Original-Received: from localhost ([::1]:42104 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1la0XD-0007hP-UR for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 23 Apr 2021 14:28:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:56566) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1la0X4-0007gy-QM for bug-gnu-emacs@gnu.org; Fri, 23 Apr 2021 14:28:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:55495) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1la0X4-0002zZ-JL for bug-gnu-emacs@gnu.org; Fri, 23 Apr 2021 14:28:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1la0X4-0004aH-Fh for bug-gnu-emacs@gnu.org; Fri, 23 Apr 2021 14:28:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 23 Apr 2021 18:28:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46326 X-GNU-PR-Package: emacs Original-Received: via spool by 46326-submit@debbugs.gnu.org id=B46326.161920243017562 (code B ref 46326); Fri, 23 Apr 2021 18:28:02 +0000 Original-Received: (at 46326) by debbugs.gnu.org; 23 Apr 2021 18:27:10 +0000 Original-Received: from localhost ([127.0.0.1]:38808 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1la0WD-0004ZC-IC for submit@debbugs.gnu.org; Fri, 23 Apr 2021 14:27:09 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:47019) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1la0WB-0004Yi-65 for 46326@debbugs.gnu.org; Fri, 23 Apr 2021 14:27:07 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 24F90100317; Fri, 23 Apr 2021 14:27:01 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 6DACA100019; Fri, 23 Apr 2021 14:26:59 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1619202419; bh=RY/wLqBZEIpFHbmNzYJwceioJj7bnU2RBLiaYigB0ug=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=dkgLWb5avgVDMBho/LraG2+JvWNc0wPqko2lt0H6vVRr3lG8BQ2kUe134W4dqO9DD JgRFaC3GrWpkk6Eepchag3fvM0RBeLfVWVUFM2BryFb9JvCG5AkbiQef49NOcoDOtp 1Vpi7MectXeS3CdNLuBYm0ah2fy82kPaCj7ZdacpPIZrAdqwV9yz05CRvWvKNCE3+/ RbR0bkK8aIK54Wfky9xO1g6TfP0EPSn7A/vVAxUtI4zKBLhQi1XYx/cCNS8R0757Yv oMmlZ2Vr2E+cCf6U10R5zQ49XT2hRIRMSVzrMdw7ur44BVf2fE7zsphuvf0B/VWRrm Osqw/TK0QdZIA== Original-Received: from alfajor (104-222-126-84.cpe.teksavvy.com [104.222.126.84]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 0BB42120247; Fri, 23 Apr 2021 14:26:59 -0400 (EDT) In-Reply-To: <62c490ed0d8d24d8b259ac1ba55ea79e@mendler.net> (mail@daniel-mendler.de's message of "Fri, 05 Feb 2021 13:51:41 +0100") 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:204755 Archived-At: > I have an issue on 27.1.50 with excessive memory allocations when using > minibuffer-with-setup-hook with large closures and :append. Indeed, we have a problem there. I think it's fairly hard to fix it for good without introducing incompatibilities, because `add-hook` has been defined to compare its functions with `equal` "for ever" and changing it to use `eq` or `function-equal` will inevitably break code out there in subtle ways. IOW I think the better fix is to change `minibuffer-with-setup-hook` to use an indirection via a symbol. As for reducing the impact of the underlying issue, I see we could reduce the amount of `equal` tests being performed, by using `eq` for the lookups in `hook--depth-alist`. So before we install the "real" solution, could you try the patch below and report how much it helps (if at all)? Stefan diff --git a/lisp/subr.el b/lisp/subr.el index c2be26a15f5..7b718a48a8d 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -1830,12 +1834,13 @@ add-hook (unless (member function hook-value) (when (stringp function) ;FIXME: Why? (setq function (purecopy function))) + ;; All those `equal' tests performed between functions can end up being + ;; costly since those functions may be large recursive and even cyclic + ;; structures, so we index `hook--depth-alist' with `eq'. (bug#46326) (when (or (get hook 'hook--depth-alist) (not (zerop depth))) ;; Note: The main purpose of the above `when' test is to avoid running ;; this `setf' before `gv' is loaded during bootstrap. - (setf (alist-get function (get hook 'hook--depth-alist) - 0 'remove #'equal) - depth)) + (push (cons function depth) (get hook 'hook--depth-alist))) (setq hook-value (if (< 0 depth) (append hook-value (list function)) @@ -1845,8 +1850,8 @@ add-hook (setq hook-value (sort (if (< 0 depth) hook-value (copy-sequence hook-value)) (lambda (f1 f2) - (< (alist-get f1 depth-alist 0 nil #'equal) - (alist-get f2 depth-alist 0 nil #'equal)))))))) + (< (alist-get f1 depth-alist 0 nil #'eq) + (alist-get f2 depth-alist 0 nil #'eq)))))))) ;; Set the actual variable (if local (progn @@ -1907,11 +1912,20 @@ remove-hook (not (and (consp (symbol-value hook)) (memq t (symbol-value hook))))) (setq local t)) - (let ((hook-value (if local (symbol-value hook) (default-value hook)))) + (let ((hook-value (if local (symbol-value hook) (default-value hook))) + (old-fun nil)) ;; Remove the function, for both the list and the non-list cases. (if (or (not (listp hook-value)) (eq (car hook-value) 'lambda)) - (if (equal hook-value function) (setq hook-value nil)) - (setq hook-value (delete function (copy-sequence hook-value)))) + (when (equal hook-value function) + (setq old-fun hook-value) + (setq hook-value nil)) + (when (setq old-fun (car (member function hook-value))) + (setq hook-value (remq old-fun hook-value)))) + (when old-fun + ;; Remove auxiliary depth info to avoid leaks. + (put hook 'hook--depth-alist + (delq (assq old-fun (get hook 'hook--depth-alist)) + (get hook 'hook--depth-alist)))) ;; If the function is on the global hook, we need to shadow it locally ;;(when (and local (member function (default-value hook)) ;; (not (member (cons 'not function) hook-value)))