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.devel Subject: Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER Date: Mon, 30 Nov 2020 17:10:20 -0500 Message-ID: References: <20201119153814.17541-1-sbaugh@catern.com> <837dqdxtea.fsf@gnu.org> <87a6v9jqz8.fsf@catern.com> <83y2itwbzk.fsf@gnu.org> <87ft4shxg8.fsf@catern.com> <835z5mk84w.fsf@gnu.org> <87y2iihafg.fsf@catern.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="36036"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Eli Zaretskii , dgutov@yandex.ru, arnold@tdrhq.com, rms@gnu.org, emacs-devel@gnu.org To: Spencer Baugh Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Nov 30 23:12:19 2020 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 1kjrP8-0009E7-3I for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Nov 2020 23:12:18 +0100 Original-Received: from localhost ([::1]:33740 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kjrP7-0005Bo-5o for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Nov 2020 17:12:17 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35742) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kjrNS-0004Mr-NZ for emacs-devel@gnu.org; Mon, 30 Nov 2020 17:10:34 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:48238) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kjrNP-0005hk-Jp; Mon, 30 Nov 2020 17:10:33 -0500 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 94026809A7; Mon, 30 Nov 2020 17:10:29 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 9F70680479; Mon, 30 Nov 2020 17:10:27 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1606774227; bh=mSygYgt2Bk7PyzsqWC+8ypNN3F2u2Mx9PNfQCcFJHHQ=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=hr9dO4Bah/uOyL8ORR5yR7K0+FcyOO2KY8kQkB6UaNpVR4IEYlSekhtAhMJczdgSo 1vj1NPox5ZfHbN8PHKOwHpnVpc5kRvD6ScOhS776GJ+qjOEQNcWGI81LNn4XFWJ+F1 DFHDN6odfGInc0IP/xr3FHOXVywkYkkYk/VZiswICNYMDIbca4eyHqOUbEkqUe918R U57vDb3KnL/NfSrwvnkJyj5ElAKpuJQ1SQqTPYnX97Z7ahiAQpABuzaQPe980z7dTK xsWpCFVriFSPtJmXJUtN4gFRPu8OGf1Txr/yYWhqyRn3GQwkQ2gQKpVCODyxtRzDsu JY/byALyCKOYA== Original-Received: from alfajor (69-165-136-52.dsl.teksavvy.com [69.165.136.52]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 4EA47120207; Mon, 30 Nov 2020 17:10:27 -0500 (EST) In-Reply-To: <87y2iihafg.fsf@catern.com> (Spencer Baugh's message of "Mon, 30 Nov 2020 15:11:31 -0500") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, 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:260099 Archived-At: > Yes, it's probably a trade-off. My guess is that the extra overhead of > BVAR will not be significant, but benchmarking is the only way to know > for sure. (Indeed, it's possible things will speed up by removing the > metadata, which reduces the size of the working set and lets more things > stay in cache) Maybe one way to figure it out is to magnify the impact: add extra slowdown code to BVAR to make sure the slowdown is noticeable. This might point to the place where a performance impact may be measurable, or it might make it obvious that the impact is negligible. > Well, with a completely naive static branch-predictor which assumes the > Qunbound branch will never be taken, there will be no overhead for > fields which are never Qunbound, namely non-DEFVAR_PER_BUFFER fields and > permanent-buffer-locals. We could encourage that prediction by marking > the Qunbound branch with an "unlikely()" macro using GCC's > __builtin_expect. While this might work for some vars, I suspect that it could be detrimental for some. > It's probably prematurely low-level for me to talk about such things > right now though - benchmarking is the only way to know. 100% agreement. And modern branch predictors are quite sophisticated so I expect they'll do a much better job than any hint we may be able to provide. > A DEFVAR_PER_BUFFER variable is normally marked as > permanent-buffer-local by: Indeed, there are two concepts: one if the concept that corresponds to the `permanent-local` symbol property (that's implemented via buffer_permanent_local_flags) and the other is the concept of buffer-local variables which are always buffer-local and don't even have a default value (that's the one your patch modifies, tho it only modifies the implementation, not the semantics). > - in the old version, setting -1 in the corresponding field in > buffer_local_flags (which is a `struct buffer' distinct from > buffer_permanent_local_flags which is an array of chars) > - in the new version, setting the corresponding field in buffer_defaults > to Qunbound (indicating there's no default value) BTW, one potentially unintended side effect of your change is that `makunbound` on PER_BUFFER vars won't work quite as usual. I guess we could address that by using some other special value than Qunbound, tho I'm not sure how important this is. > All this is rather confusing and inconsistent; I don't think we should > add more of these "pseudo-permanent" variables. Agreed: I think we should get rid of this special category of "always local" variables. This is a concept that only applies to the following variables: bset_filename (&buffer_local_flags, make_fixnum (-1)); bset_directory (&buffer_local_flags, make_fixnum (-1)); bset_backed_up (&buffer_local_flags, make_fixnum (-1)); bset_save_length (&buffer_local_flags, make_fixnum (-1)); bset_auto_save_file_name (&buffer_local_flags, make_fixnum (-1)); bset_read_only (&buffer_local_flags, make_fixnum (-1)); bset_major_mode (&buffer_local_flags, make_fixnum (-1)); bset_mode_name (&buffer_local_flags, make_fixnum (-1)); bset_undo_list (&buffer_local_flags, make_fixnum (-1)); bset_mark_active (&buffer_local_flags, make_fixnum (-1)); bset_point_before_scroll (&buffer_local_flags, make_fixnum (-1)); bset_file_truename (&buffer_local_flags, make_fixnum (-1)); bset_invisibility_spec (&buffer_local_flags, make_fixnum (-1)); bset_file_format (&buffer_local_flags, make_fixnum (-1)); bset_auto_save_file_format (&buffer_local_flags, make_fixnum (-1)); bset_display_count (&buffer_local_flags, make_fixnum (-1)); bset_display_time (&buffer_local_flags, make_fixnum (-1)); bset_enable_multibyte_characters (&buffer_local_flags, make_fixnum (-1)); I don't see the benefit of forcing those vars to always be buffer-local. We could just as well make them "normal" and they'll still have a buffer-local value pretty much all the time anyway. Stefan