unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: Dmitry Gutov <dgutov@yandex.ru>
Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
Subject: Re: Bug #25608 and the comment-cache branch
Date: Tue, 7 Feb 2017 19:21:19 +0000	[thread overview]
Message-ID: <20170207192119.GA2490@acm> (raw)
In-Reply-To: <4f0fabf3-be9c-7492-379b-59dc93e72b4f@yandex.ru>

Hello, Dmitry.

On Tue, Feb 07, 2017 at 03:42:50 +0200, Dmitry Gutov wrote:
> Hey Alan,

> On 06.02.2017 21:24, Alan Mackenzie wrote:

> > The essence of comment-cache is scanning comments only in the forward
> > direction.  This is impractical without a good cache.  The syntax-ppss
> > cache is wholly inadequate here (and would be even if it worked in the
> > general case).

> How come the "alternative patch" works well, then?

Well, aside from the fact that it doesn't (IMAO), it is only consulted
relatively rarely, in certain cases of back_coment where the backward
scanning hits something it doesn't want to handle.  The AP is marginally
slower than comment-cache.  If such awkward comments were prominent in a
file, it would be noticeably slower.  In comment-cache, the cache is
used for every back_comment.

> The only bugs you've outlined so far are related to narrowing and
> syntax table change, but not any static complex syntactic situations,
> which is where I would expect scanning direction to have an impact.

Those bugs are enough, aren't they?  (forward-comment -1) etc., should
work correctly in any circumstances.  There might be the sort of bugs
you're looking for, but I suspect not.  The backward scanning code is
very complicated.

> > There's no sign of syntax-ppss being fixed.  Bug #22983 has been open
> > for almost a year, and despite repeated requests from me, there has been
> > no movement on it.

> You didn't show any enthusiasm about the initial proposed fix, which was 
> rather simple. Now we've had more discussions, and the bar for a 
> solution has been raised. I'm thinking about it again. Let's not give up.

I wasn't enthusiastic about your proposed fix because I found it ugly.

> > Anyways, there are other problems with the "alternative patch".  It
> > doesn't clear it's caches when syntax-table properties are applied to or
> > removed from a buffer.  It doesn't clear its caches when a "literal
> > relevant" change is made to the current syntax table, or a different
> > syntax-table is made current.

> Tracking changes inside a syntax table is possible (at the expense of 
> some performance, as usual), but kinda pointless, I think. Most issues 
> related to that, if they ever come up, could be answered with "don't do 
> that".

That sounds like you've decided you want to use syntax-ppss no matter
what, and the bugs this will cause will just be relabeled as features.
As I've said before, the aim should be for back_comment always to work.

> Tracking the used syntax table is also a problem which we need to solve 
> for syntax-ppss. A good design could handle it and narrowing together.

You should now be able to see why I dislike syntax-ppss so much.  As
well as being incompatible with narrowing (which should be sort of
fixable), there is an essential lack of cache invalidating (which would
only be fixable by a radically different design).  There is no sign that
much thought was given to cache invalidation in the design of
syntax-ppss.  This probably cannot be fixed, or if it can, will involve
lots of programming at the C level, and will slow Emacs down quite a
bit.

> > comment-cache handles these situations correctly - that's where its
> > perceived complexity scores.

> And it does that in a pretty inflexible way.

It works.  Other ways (apart from M-nil (master with
open-paren-in-column-0-is-defun-start set to nil)) don't.  The sort of
flexibility I recall you wanting is simply not possible in
comment-cache, though its role could be expanded for other uses which
need the literality of a position.

> > comment-cache has rewriten back_comment entirely, hence the
> > troublesome merge.  It's no more difficult for maintainers than the
> > current version of Emacs.

> But surely it is more complex, with cache handling logic.

It's differently complicated.  master's back_comment, which attempts to
scan comments backwards is more complicated than comment-cache's
back_comment (including its cacheing logic).

> > They shouldn't drift apart at all.  But drifting apart is no worse a
> > problem than a single cache being wrong.

> Yes, it is worse. You have more code to debug. And comment-cache adds 
> quite a bit of code.

How have you quantified "quite a bit"?

> > Arguing for complete abandonment is not constructive criticism.

> When an alternative approach is recommended, yes, it is.

There is nothing to indicate you've even looked at comment-cache.  All
the criticisms you've made have been from a distance, based on rumour
(even if the source of that rumour has been me).  These criticisms have
been entirely destructive.  I repeat, you want comment-cache to be
wholly abandoned, apparently because you like syntax-ppss so much.  The
alternative "recommended" approach has documented deficiencies, yet you
still advocate it.

> > I'm not saying the "alternative patch" couldn't be enhanced to do these
> > things properly, but it would then no longer be a 20-line patch.

> I think it would be. The enhancements you're referring to will most 
> likely be implemented on the Lisp level, and they are needed anyway.

They can't be implemented at the Lisp level.  The tools Emacs Lisp
provides for cache invalidation (basically,
before/after-change-functions) aren't up to the job.

> So the "speed up forward-comment" patch would still come out to 20 lines.

Well, if you get a decent bug fix involving, say a 700 line patch which
includes those 20 lines, I suppose you could still call it a 20 line
patch, somehow.

> > It would also likely be much slower.

> I wouldn't be so sure. A syntax table comparison, for instance, would be 
> pretty cheap compared to what syntax-ppss does already.

Full syntax-table comparisons are slow, even when written in C.  I tried
it back in December.  CC Mode regularly switches syntax-tables.  My
usual time-scroll function on xdisp.c ran at about half the speed when a
comparison was done at every set-syntax-table.  The results had to be
cached, after which it ran at normal speed again.

-- 
Alan Mackenzie (Nuremberg, Germany).



  reply	other threads:[~2017-02-07 19:21 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-02 20:24 Bug #25608 and the comment-cache branch Alan Mackenzie
2017-02-02 20:47 ` Eli Zaretskii
2017-02-02 21:51   ` Alan Mackenzie
2017-02-02 22:15     ` Dmitry Gutov
2017-02-03  7:41     ` Eli Zaretskii
2017-02-03 17:29       ` Alan Mackenzie
2017-02-03 22:08         ` Dmitry Gutov
2017-02-04 10:24           ` Alan Mackenzie
2017-02-06  2:09             ` Dmitry Gutov
2017-02-06 19:24               ` Alan Mackenzie
2017-02-07  1:42                 ` Dmitry Gutov
2017-02-07 19:21                   ` Alan Mackenzie [this message]
2017-02-14 15:28                     ` Dmitry Gutov
2017-02-14 16:38                       ` Stefan Monnier
2017-02-22  2:25                         ` Dmitry Gutov
2017-02-22  3:53                           ` Stefan Monnier
2017-02-23 14:23                             ` Dmitry Gutov
2017-02-23 14:48                               ` Stefan Monnier
2017-02-24  7:46                                 ` Tom Tromey
2017-02-14 21:14                       ` Alan Mackenzie
2017-02-16 14:10                         ` Stefan Monnier
2017-02-18 10:44                           ` Alan Mackenzie
2017-02-18 13:49                             ` Stefan Monnier
2017-02-12  2:53               ` John Wiegley
2017-02-12  8:20                 ` Elias Mårtenson
2017-02-12 10:47                 ` Alan Mackenzie
2017-02-12 11:14                 ` martin rudalics
2017-02-12 15:05                   ` Andreas Röhler
2017-02-12 15:39                   ` Eli Zaretskii
2017-02-05 22:00       ` Alan Mackenzie
2017-02-06  1:12         ` Stefan Monnier
2017-02-06 18:37           ` Alan Mackenzie
2017-02-08 17:20         ` Eli Zaretskii
2017-02-11 23:25           ` Alan Mackenzie
2017-02-12  0:55             ` Stefan Monnier
2017-02-12 12:05               ` Alan Mackenzie
2017-02-12 13:13                 ` Juanma Barranquero
2017-02-12 15:57                 ` Dmitry Gutov
2017-02-12 17:29                   ` Alan Mackenzie
2017-02-12 20:35                     ` Dmitry Gutov
2017-02-13  1:47                     ` zhanghj
2017-02-13  5:50                       ` Stefan Monnier
2017-02-13  6:45                         ` zhanghj
2017-02-13  7:24                           ` Stefan Monnier
2017-02-13  7:59                             ` zhanghj
2017-02-13  9:25                               ` Stefan Monnier
2017-02-13 16:14                           ` Drew Adams
2017-02-13  7:05                         ` zhanghj
2017-02-13  7:16                         ` zhanghj
2017-02-13 14:57                           ` Dmitry Gutov
2017-02-12 17:49                 ` Stefan Monnier
2017-02-13 18:09                   ` Alan Mackenzie
2017-02-13 19:34                     ` Eli Zaretskii
2017-02-13 21:21                     ` Stefan Monnier
2017-02-02 22:14 ` Dmitry Gutov
2017-02-03 16:44   ` Alan Mackenzie
2017-02-03 21:53     ` Dmitry Gutov
2017-02-04 11:02       ` Alan Mackenzie
2017-02-06  1:28         ` Dmitry Gutov
2017-02-06 19:37           ` Alan Mackenzie
2017-02-06  2:08         ` Stefan Monnier
2017-02-06 20:01           ` Alan Mackenzie
2017-02-06 22:33             ` Stefan Monnier
2017-02-07 21:24               ` Alan Mackenzie
2017-02-08 12:54                 ` Stefan Monnier
2017-02-07 15:29             ` Eli Zaretskii
2017-02-07 21:09               ` Alan Mackenzie
2017-02-08 17:28                 ` Eli Zaretskii
2017-02-02 23:57 ` Stefan Monnier
2017-02-03 16:19   ` Alan Mackenzie
2017-02-04  9:06     ` Andreas Röhler
2017-02-04 18:18     ` Stefan Monnier
2017-02-04 18:28       ` Alan Mackenzie
2017-02-03  7:49 ` Yuri Khan
2017-02-03 18:30   ` Andreas Röhler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170207192119.GA2490@acm \
    --to=acm@muc.de \
    --cc=dgutov@yandex.ru \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).