From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Keeping replace-buffer-contents runtime in bounds Date: Sun, 17 Feb 2019 17:50:45 +0200 Message-ID: <83imxil8h6.fsf@gnu.org> References: <87r2c7ze9n.fsf@gnu.org> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="127237"; mail-complaints-to="usenet@blaine.gmane.org" Cc: emacs-devel@gnu.org To: Tassilo Horn Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Feb 17 16:51:31 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gvOj3-000Wva-0S for ged-emacs-devel@m.gmane.org; Sun, 17 Feb 2019 16:51:29 +0100 Original-Received: from localhost ([127.0.0.1]:43130 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gvOj1-0007qK-Sd for ged-emacs-devel@m.gmane.org; Sun, 17 Feb 2019 10:51:27 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:33248) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gvOiJ-0007q3-F6 for emacs-devel@gnu.org; Sun, 17 Feb 2019 10:50:44 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:56175) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gvOiJ-0005UR-Ak for emacs-devel@gnu.org; Sun, 17 Feb 2019 10:50:43 -0500 Original-Received: from [176.228.60.248] (port=3985 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1gvOiI-0005RI-D2; Sun, 17 Feb 2019 10:50:42 -0500 In-reply-to: <87r2c7ze9n.fsf@gnu.org> (message from Tassilo Horn on Sat, 16 Feb 2019 21:09:40 +0100) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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" Xref: news.gmane.org gmane.emacs.devel:233427 Archived-At: > From: Tassilo Horn > Date: Sat, 16 Feb 2019 21:09:40 +0100 > > 1. There's some too_expensive field of the context struct passed to > compareseq which has quite some effect on the speed. I couldn't find > any negative sides in using a much lower value than we originally > used (1,000,000). However, maybe it's just my use-case. This is now > exposed as an optional argument of replace-buffer-contents. > > 2. The too_expensive field is not enough. If the buffer contents and > the contents of the replacement buffer become too large and there are > too many differences, it may still take ages. > > So now I added some code which allows compareseq to abort early if the > difference computation is too costly. Initially I've tried to use the > number of differences found so far plus a max value. However, after > using that some days I noticed that this is not a too good measure. > Sometimes there were gazillion of differences, yet the difference > computation was quick. But other times the number of differences was > lower but still it took ages (most probably because the json was > larger). > > In the end I settled for a maximum number of seconds one can define by > setting a new variable replace-buffer-contents-max-secs, so that you can > define what's still acceptable in the respective use-case. (Actually, > if you set that to 1.5 or so, it may still run for 2 or more seconds > because the EARLY_ABORT expression isn't tested at regular intervals or > rather it is, but the intervals don't take equally long.) > > If that number of seconds is over, compareseq returns early and > replace-buffer-contents falls back to plain delete and insert. The gotcha about aborting after more than the time-out value should be mentioned in the doc string. Thanks for working on this. My only other comment is that maybe we should allow passing the time-out value via the function's arguments, not via a global variable. It seems to me the time-out will be used in more use cases than MAX-COSTS, and in any case treating these two differently API-wise sounds strangely inconsistent. > This is my first C encounter in emacs, so please feel free to nit-pick. Nitpicking: > + DEFVAR_LISP ("replace-buffer-contents-max-secs", > + Vreplace_buffer_contents_max_secs, > + doc: /* If differencing the two buffers takes longer than this, > +`replace-buffer-contents' falls back to a plain delete and insert. */); The first sentence of a doc string should not be longer than 79 characters. (But if you agree with me, this variable will go away, so it's a moot point.)