From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] /srv/bzr/emacs/trunk r109890: Do not mark objects from deleted buffers, windows and frames. Date: Thu, 06 Sep 2012 08:52:40 -0400 Message-ID: References: <504848D0.4020908@yandex.ru> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1346935976 20548 80.91.229.3 (6 Sep 2012 12:52:56 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 6 Sep 2012 12:52:56 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Sep 06 14:52:58 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1T9bZq-00036t-5E for ged-emacs-devel@m.gmane.org; Thu, 06 Sep 2012 14:52:58 +0200 Original-Received: from localhost ([::1]:46169 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9bZn-0008OT-6l for ged-emacs-devel@m.gmane.org; Thu, 06 Sep 2012 08:52:55 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:38545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9bZf-0008OH-S1 for emacs-devel@gnu.org; Thu, 06 Sep 2012 08:52:53 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T9bZZ-00063E-ST for emacs-devel@gnu.org; Thu, 06 Sep 2012 08:52:47 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.182]:5823) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9bZZ-00062t-OJ for emacs-devel@gnu.org; Thu, 06 Sep 2012 08:52:41 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Av0EAG6Zu09FpYew/2dsb2JhbABEtBGBCIIVAQEEAScvIwULCw4mBwsUGA0kiBwFugmQRAOjM4FYgwU X-IronPort-AV: E=Sophos;i="4.75,637,1330923600"; d="scan'208";a="197692818" Original-Received: from 69-165-135-176.dsl.teksavvy.com (HELO pastel.home) ([69.165.135.176]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 06 Sep 2012 08:52:40 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 6D84C588E8; Thu, 6 Sep 2012 08:52:40 -0400 (EDT) In-Reply-To: <504848D0.4020908@yandex.ru> (Dmitry Antipov's message of "Thu, 06 Sep 2012 10:55:12 +0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.182 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:153105 Archived-At: > IMHO such an operations shouldn't make any assumptions about internal > fields of the deleted objects. The only possible exception is NILP > (obj->field) since this may be used to distinguish between live and > dead objects (as we have now for the buffers and windows). No, there can be perfectly valid reasons to access a deleted/killed object. >> E.g. you might still be able to get (window|frame)-parameters of >> a deleted (window|frame). > I'm pretty sure that this is invalid and should be fixed. I don't see anything that needs fixing here. On the contrary. It can be very useful to keep a reference to a frame/window, and when you need to display data, see if it's still live, and if not create a new one, initializing some of its parameters from the old deleted one. > The only important exception is saving/restoring window configurations. > Strictly speaking, if the window configuration is recorded in > saved_window_data, such a window is not deleted. Ideally, struct > window should have a bit indicating that it's configuration is > recorded so such a window can be distinguished from the really dead > windows; In 99% of the cases, it's easy to distinguish: one can be GC'd and the other can't. >> IOW, it adds lines of code, makes the invariants more complex (in ways >> which I'm not sure is currently ensured by the rest of the code) and the >> benefits aren't obvious at all. > Hm. For example, killed buffers may sit in all_buffers for a while, > and still have from tens to thousands reachable objects per buffer > (although I didn't check whether these objects are reachable only from > this dead buffer). So, that's the problem you're attacking. Fine, but please: 1- Check how much extra data is really kept live because of this. As you mention, it may be a lot less than it appears at first. 2- Check how much longer it's kept alive. 4- Check which fields of the buffer keep that data alive. 3- Check why the buffer itself is kept alive: maybe the better fix is to make it so the buffer can be GC'd. >> I don't think scanning those objects can take a noticeable amount of >> time, so the only potential issue is holding on to data that can never >> be used again, in which case I'd much prefer changing >> kill-buffer/delete-(window|frame) so they set the various fields to >> NULL/nil. Which is a much safer change. > I agree about the safety, but: > 1) this is slower; Yes, it could be slower, tho I expect there are only a few slots that would need such a treatment, so I don't think it would be noticeable. > 2) IMHO this is conceptually wrong and I strongly disagree. I think you have an incorrect understanding of what a "deleted/killed" object is. It really isn't dead at all. > 3) it still has it's own traps. Obviously, we don't want to NULL/nil all fields blindly. We want to do it parsimoniously, so that we only do it for fields which we know can safely be set this way, and ideally only for fields which do hold on to too much data (e.g. not for fields which only contain numbers anyway). Stefan