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: [RFC]: replace-region-contents Date: Fri, 08 Feb 2019 23:27:57 +0200 Message-ID: <83ef8iotr6.fsf@gnu.org> References: <871s4rqk7u.fsf@gnu.org> <87o97syvno.fsf@gnu.org> <878syubwv3.fsf@gnu.org> <87y36u9xqt.fsf@gnu.org> <83k1ietcox.fsf@gnu.org> <87k1iew3u6.fsf@gnu.org> <83va1wsy95.fsf@gnu.org> <87h8dgetp1.fsf@gnu.org> <83mun8styg.fsf@gnu.org> <87lg2saiz9.fsf@gnu.org> <871s4idzb2.fsf@gnu.org> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="105504"; mail-complaints-to="usenet@blaine.gmane.org" Cc: eggert@cs.ucla.edu, monnier@IRO.UMontreal.CA, emacs-devel@gnu.org To: Tassilo Horn Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Feb 08 22:28:57 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 1gsDhh-000RCh-8m for ged-emacs-devel@m.gmane.org; Fri, 08 Feb 2019 22:28:57 +0100 Original-Received: from localhost ([127.0.0.1]:35420 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gsDhg-00015h-7j for ged-emacs-devel@m.gmane.org; Fri, 08 Feb 2019 16:28:56 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:38289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gsDhV-00014j-4x for emacs-devel@gnu.org; Fri, 08 Feb 2019 16:28:46 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:36783) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gsDhL-0007Sj-Fm; Fri, 08 Feb 2019 16:28:35 -0500 Original-Received: from [176.228.60.248] (port=2864 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1gsDh4-0002gn-8a; Fri, 08 Feb 2019 16:28:19 -0500 In-reply-to: <871s4idzb2.fsf@gnu.org> (message from Tassilo Horn on Fri, 08 Feb 2019 17:23:29 +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:233143 Archived-At: > From: Tassilo Horn > Cc: Paul Eggert , monnier@IRO.UMontreal.CA, emacs-devel@gnu.org > Date: Fri, 08 Feb 2019 17:23:29 +0100 > > if nobody complains, I'd like to install the following patch. No complaints from me, just a few comments. > I've set too_expensive to 64 because then my benchmark was almost as > fast as the original version without `replace-buffer-contents', and I > couldn't find any difference in observable behavior in that value anyway > except lower values give much better performance but a too low value > like 10 will segfault. My main concern was that point was at the same > position before and after pretty-printing my JSON. I'd prefer to expose this to Lisp, not hardcode the value. We could use the hardcoded value in json.el, but I'd like to leave this to the application in the other cases. I'm not sure such a small value will always be the right tradeoff, so I think we shouldn't decide just yet. If you agree with that, let's expose this via a new optional argument to replace-buffer-contents, and let's treat the value of nil as a very large integer value, i.e. "no limit". > --- a/src/editfns.c > +++ b/src/editfns.c > @@ -1910,6 +1910,11 @@ determines whether case is significant or ignored. */) > > #undef ELEMENT > #undef EQUAL > +#define USE_HEURISTIC > + > +#ifdef USE_HEURISTIC > +#define DIFFSEQ_HEURISTIC > +#endif > > /* Counter used to rarely_quit in replace-buffer-contents. */ > static unsigned short rbc_quitcounter; > @@ -2017,8 +2022,11 @@ differences between the two buffers. */) > .insertions = SAFE_ALLOCA (ins_bytes), > .fdiag = buffer + size_b + 1, > .bdiag = buffer + diags + size_b + 1, > +#ifdef DIFFSEQ_HEURISTIC > + .heuristic = true, > +#endif Why do we need this triple-level conditional? If we think the heuristic is a good idea, let's just enable it unconditionally. If someone wants to modify Emacs on the C level, they can edit the source as easily as they can invoke the compiler with a -U switch. Finally, I think this needs NEWS entries, both regarding the new function and the additional argument to replace-buffer-contents. The latter is also documented in the ELisp manual, which will need to be updated. Thanks. P.S. I still hope Paul will chime in and comments on the diffseq bits we are modifying here.