From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh Newsgroups: gmane.emacs.devel Subject: Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER Date: Mon, 30 Nov 2020 15:11:31 -0500 Message-ID: <87y2iihafg.fsf@catern.com> 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> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19370"; mail-complaints-to="usenet@ciao.gmane.io" Cc: arnold@tdrhq.com, dgutov@yandex.ru, monnier@iro.umontreal.ca, rms@gnu.org, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Nov 30 21:12:48 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 1kjpXT-0004xm-Ch for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Nov 2020 21:12:47 +0100 Original-Received: from localhost ([::1]:56742 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kjpXS-0002Ld-Cg for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Nov 2020 15:12:46 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60180) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kjpWP-0001u7-0p for emacs-devel@gnu.org; Mon, 30 Nov 2020 15:11:41 -0500 Original-Received: from venus.catern.com ([68.183.49.163]:53282) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kjpWM-0007Mc-FU; Mon, 30 Nov 2020 15:11:40 -0500 Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=98.7.229.235; helo=localhost; envelope-from=sbaugh@catern.com; receiver= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=catern.com; s=mail; t=1606767091; bh=F3+vh3ZFKBTUScKt3Kyhea6jN6Lp0gmmd66y2l91K34=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=esqyBepf4TO1Vh5aNsEI9Ls8/FQuEWaau2ylPd3XqCAKCY5T+9ZzwMow88XtL27MA 8LL1xWL0FKS8pboIZaQMQm5cBKydfsofMS8mIyuZ/VTjqf+tmCwIyV6W99qsEBUkDU 00QemoYEz/yv1FqjN/mUVFZhRYGkH60cnw7FPCHs= Original-Received: from localhost (cpe-98-7-229-235.nyc.res.rr.com [98.7.229.235]) by venus.catern.com (Postfix) with ESMTPSA id B4FF92DE899; Mon, 30 Nov 2020 20:11:31 +0000 (UTC) In-Reply-To: <835z5mk84w.fsf@gnu.org> Received-SPF: pass client-ip=68.183.49.163; envelope-from=sbaugh@catern.com; helo=venus.catern.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, 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:260096 Archived-At: Eli Zaretskii writes: >> === New implementation === >> >> In the new implementation, the BVAR macro actually does some work. >> Simplifying a bit: >> >> #define BVAR(buf, field) EQ ((buf)->field, Qunbound) ? buffer_defaults.field : (buf)->field >> >> We now use Qunbound as a sentinel to tell us whether the buffer has a >> buffer-local binding for this field or not. If the field contains >> Qunbound, we should use the default value out of buffer_defaults; if >> not, there's a buffer-local binding, and we should use the per-buffer >> value. >> >> We no longer need to iterate over all buffers whenever we change the >> default value. So setting the default value is now fast. > > But OTOH, referencing buffer-local values through BVAR is now slower, > about twice slower, I guess? For example, the display code has 90 > uses of BVAR, and they will now be slower. I wonder what that means > for us: we are making let-binding of local variables much faster, but > "punish" the code that just accesses these variables in our inner > loops. 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) >> We could do a mass change to all the uses of BVAR, so that accesses to >> non-DEFVAR_PER_BUFFER fields and permanent-buffer-local >> DEFVAR_PER_BUFFER fields don't go through the conditional. But the >> additional check adds minimal overhead anyway; it will be easily >> branch-predicted by modern CPUs. > > I don't think I understand the last part: how can branch-prediction > help here, when which variables do have local bindings and which don't > is not known in advance and can change a lot during a session. What > am I missing here? 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. It's probably prematurely low-level for me to talk about such things right now though - benchmarking is the only way to know. >> Get rid of buffer_permanent_local_flags array >> >> This removes some usage of the buffer_local_flags indices in favor of a >> simpler special case for the two pseudo-permanent-local variables it >> applied to. > > Does this mean we will no longer be able to mark built-in variables as > permanent-buffer-local? If so, why would we want to lose this > capability? No, we can still mark them as permanent-buffer-local. The control flow here is a bit weird. buffer_permanent_local_flags is not how DEFVAR_PER_BUFFER variables are normally marked as permanent-buffer-local. A DEFVAR_PER_BUFFER variable is normally marked as permanent-buffer-local by: - 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) When such marking is done, all the normal behavior of permanent-buffer-local variables happens: - there's no default value, - local-variable-p always returns t, - kill-local-variable is a no-op, etc. buffer_permanent_local_flags is a separate, orthogonal mechanism. If a variable is marked in buffer_permanent_local_flags: - it still has a default value which can be set, - local-variable-p can return either nil or t, - kill-local-variable kills the local binding, etc. In summary, a variable marked in buffer_permanent_local_flags is in general indistinguishable from any other non-permanent buffer-local variable. It's not actually permanent-buffer-local. buffer_permanent_local_flags has only one effect, on the reset_buffer_local_variables function. reset_buffer_local_variables in fact treats non-permanent and permanent buffer-local-variables identically, and always resets all of them. Its behavior only diverges for variables marked with buffer_permanent_local_flags, which it only resets if the confusingly named PERMANENT_TOO argument is true. This is mentioned in the docstring of reset_buffer_local_variables: it does not treat permanent locals consistently. All this is rather confusing and inconsistent; I don't think we should add more of these "pseudo-permanent" variables.