The current state of the comment-cache branch

The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Emacs.

I have enhanced the comment-cache branch over its state in March 2016.
In particular, it now handles set-syntax-table and modify-syntax-entry.

This branch implements an alternative, superior, method of scanning
comments which supersedes that currently in syntax.c.  It was created to
solve (as opposed to working around) the problem that gave rise to bug
#22884 "25.0.92; C/l Mode editing takes waaaayy too long", raised by Paul
Eggert on 2016-03-02.  That problem is the treating of parens at column
zero, even inside comments, as defun starts.

comment-cache does (forward_comment -1) by reading a @dfn{literal cache}
implemented in text properties which records the string/comment state
throughout the buffer, and moving back to the beginning of the comment as
recorded in this cache.

The cache is trimmed on any buffer change.  It is also trimmed when
(i) a _relevant_ syntax-table text property is applied or removed (even
  when change hooks are disabled);
(ii) set-syntax-table is called, but only when the current and new syntax
  tables parse literals differently.
(iii) an element of the current syntax table which is relevant to literal
  parsing is changed (by modify-syntax-entry).  In this case the literal
  cache of all buffers currently using the syntax table are trimmed.

comment-cache does not impose any noticeable run-time overhead.  Using my
favourite time tool, `time-scroll', which scrolls through a buffer
displaying it at each step I get the following timings for xdisp.c:

                   First forward run     Backwards     Second forward run

master:                 34.100s            36.050s	   34.575s 

comment-cache:          30.110s            32.275s         34.990s

.  For reference, here is the source for time-scroll:

#########################################################################
(defmacro time-it (&rest forms)
  "Time the running of a sequence of forms using `float-time'.
Call like this: \"M-: (time-it (foo ...) (bar ...) ...)\"."
  `(let ((start (float-time)))
    ,@forms
    (- (float-time) start)))

(defun time-scroll (&optional arg)
  (interactive "P")
  (message "%s"
           (time-it
            (condition-case nil
                (while t
                  (if arg (scroll-down) (scroll-up))
                  (sit-for 0))
              (error nil)))))
#########################################################################

.  Between successive runs, you should move point to BOB, type a space
and then erase it, to clear the font-lock cache.  Scrolling forward is
(time-scroll).  Backward is (time-scroll t).

I would like the comment-cache code to be merged into master soon.

Comments?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Stefan Monnier]

> I would like the comment-cache code to be merged into master soon.

This message is probably redundant, but just to make it clear:

I'm opposed to this change as it is because I find its cost/benefit
tradeoff to be poor.  I don't think solving some rare pathological case
of (forward-comment -1) deserves this complexity.

I think we'd be better off with either:

- a simpler change, to reduce the cost part of the tradeoff (such as the
  one I suggested, using syntax-ppss; such a change might make extra
  demands on syntax-ppss which will require further "refinement" to
  syntax-ppss, but these refinements are needed in any case, so are not
  an argument against that change, but only an argument to delay the
  change).

- a more thorough change, to improve the benefit (i.e. make your
  "comment-cache" able to provide the same kind of information that we
  currently get via syntax-ppss, so it can supersede syntax-ppss).


        Stefan

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Stephan.

On Fri, Dec 23, 2016 at 08:32:42PM -0500, Stefan Monnier wrote:
> > I would like the comment-cache code to be merged into master soon.

> This message is probably redundant, but just to make it clear:

> I'm opposed to this change as it is because I find its cost/benefit
> tradeoff to be poor.  I don't think solving some rare pathological case
> of (forward-comment -1) deserves this complexity.

The prime benefit is that open parens in column 0 inside comments
entirely cease to be an issue.  They are far from rare pathological
cases.  They have turned up continually in bug reports for many years,
including in Paul's bug #22884, where the cause of a 10s delay in a
simple edit was such a paren in column 0.

> I think we'd be better off with either:

> - a simpler change, to reduce the cost part of the tradeoff (such as the
>   one I suggested, using syntax-ppss; such a change might make extra
>   demands on syntax-ppss which will require further "refinement" to
>   syntax-ppss, but these refinements are needed in any case, so are not
>   an argument against that change, but only an argument to delay the
>   change).

This would involve a substantial change to syntax-ppss.  The way it is
at the moment, a use of syntax-ppss involves scanning forward over, on
average, 10,000 characters.  This would dominate the time taken to move
backwards over a small comment, slowing it down by 2 orders of
magnitude.  The time taken by comment-cache to do the same movement is
low, and essentially independent of the size of the comment.

> - a more thorough change, to improve the benefit (i.e. make your
>   "comment-cache" able to provide the same kind of information that we
>   currently get via syntax-ppss, so it can supersede syntax-ppss).

I can't imagine how that could work.  comment-cache, as you know, keeps
the "literal state" of all positions in text properties.  Were the
entire parse state to be encoded thus, the number of conses involved
would increase dramatically, along with the overhead on the text
property system.

parse-partial-sexp currently has a facility to stop at the start or end
of any string or comment.  This is used in comment-cache to write the
text properties.  There is no corresponding facility to stop after _any_
change in parse state.  Maybe this could be written, but the
benefit/cost ratio might be low.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Stefan Monnier]

>                    First forward run     Backwards     Second forward run
> master:                 34.100s            36.050s	   34.575s
> comment-cache:          30.110s            32.275s       34.990s

How come comment-cache is slower on the second run than on the first?


        Stefan

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Stefan.

On Fri, Dec 23, 2016 at 08:33:56PM -0500, Stefan Monnier wrote:
> >                    First forward run     Backwards     Second forward run
> > master:                 34.100s            36.050s	   34.575s
> > comment-cache:          30.110s            32.275s       34.990s

> How come comment-cache is slower on the second run than on the first?

I don't know.  I've spent quite some time searching for an answer
without coming up with any theories.  But it's definitely something
systematic, it's not just random noise in an isolated timing.

The good news is that on third and subsequent runs, it doesn't get any
slower.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Stephen Leake]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>                    First forward run     Backwards     Second forward run
>> master:                 34.100s            36.050s	   34.575s
>> comment-cache:          30.110s            32.275s       34.990s
>
> How come comment-cache is slower on the second run than on the first?

Building the interval tree that stores text properties from scratch is
faster than modifying it.

The ada-mode parser, which creates lots of text properties, also shows
this effect.

-- 
-- Stephe

Re: The current state of the comment-cache branch

[Stefan Monnier]

>> How come comment-cache is slower on the second run than on the first?
> Building the interval tree that stores text properties from scratch is
> faster than modifying it.

Indeed, that could explain it, thanks,


        Stefan

Re: The current state of the comment-cache branch

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Building the interval tree that stores text properties from scratch is
  > faster than modifying it.

Then would it be better to discard that data after each operation?

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> Date: Fri, 23 Dec 2016 21:50:56 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
>                    First forward run     Backwards     Second forward run
> 
> master:                 34.100s            36.050s	   34.575s 
> 
> comment-cache:          30.110s            32.275s         34.990s
> [...]
> I would like the comment-cache code to be merged into master soon.

Given that the speed-up is around 11% - 13%, and only for the first
scan, what are the other advantages/benefits of this code that would
cause us to merge it?

Thanks.

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Eli.

On Sat, Dec 24, 2016 at 10:02:30AM +0200, Eli Zaretskii wrote:
> > Date: Fri, 23 Dec 2016 21:50:56 +0000
> > From: Alan Mackenzie <acm@muc.de>

> >                    First forward run     Backwards     Second forward run

> > master:                 34.100s            36.050s	   34.575s 

> > comment-cache:          30.110s            32.275s         34.990s
> > [...]
> > I would like the comment-cache code to be merged into master soon.

> Given that the speed-up is around 11% - 13%, and only for the first
> scan, what are the other advantages/benefits of this code that would
> cause us to merge it?

The prime advantage is that we would support open parens in column 0
inside comments.  Not through some awkward workaround, but through a
change in algorithm, by which these open parens simply aren't even an
issue any more.

The timings I gave were intented to refute in advance any suggestion
that the new code might slow Emacs down, the way setting
open-paren-in-column-0-is-defun-start to nil does.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> Date: Sat, 24 Dec 2016 08:30:55 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > >                    First forward run     Backwards     Second forward run
> 
> > > master:                 34.100s            36.050s	   34.575s 
> 
> > > comment-cache:          30.110s            32.275s         34.990s
> > > [...]
> > > I would like the comment-cache code to be merged into master soon.
> 
> > Given that the speed-up is around 11% - 13%, and only for the first
> > scan, what are the other advantages/benefits of this code that would
> > cause us to merge it?
> 
> The prime advantage is that we would support open parens in column 0
> inside comments.  Not through some awkward workaround, but through a
> change in algorithm, by which these open parens simply aren't even an
> issue any more.
> 
> The timings I gave were intented to refute in advance any suggestion
> that the new code might slow Emacs down, the way setting
> open-paren-in-column-0-is-defun-start to nil does.

Can you show similar timings with that variable set to nil?  And in
particular in the use case reported by Paul back when bug#22884 was
filed?

(I'm actually unsure I understand how binding that variable to nil
will slow down Emacs, as in the bug in question such a binding was
proposed as a _solution_ to slow-down.  What am I missing?)

Re: The current state of the comment-cache branch

[Alan Mackenzie]

On Sat, Dec 24, 2016 at 10:55:11AM +0200, Eli Zaretskii wrote:
> > Date: Sat, 24 Dec 2016 08:30:55 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > >                    First forward run     Backwards     Second forward run

> > > > master:                 34.100s            36.050s	   34.575s 

> > > > comment-cache:          30.110s            32.275s         34.990s
> > > > [...]
> > > > I would like the comment-cache code to be merged into master soon.

> > > Given that the speed-up is around 11% - 13%, and only for the first
> > > scan, what are the other advantages/benefits of this code that would
> > > cause us to merge it?

> > The prime advantage is that we would support open parens in column 0
> > inside comments.  Not through some awkward workaround, but through a
> > change in algorithm, by which these open parens simply aren't even an
> > issue any more.

> > The timings I gave were intented to refute in advance any suggestion
> > that the new code might slow Emacs down, the way setting
> > open-paren-in-column-0-is-defun-start to nil does.

> Can you show similar timings with that variable set to nil?  And in
> particular in the use case reported by Paul back when bug#22884 was
> filed?

master:                        101.939s            95.049s        103.546s
(with open-paren-... nil)

> (I'm actually unsure I understand how binding that variable to nil
> will slow down Emacs, as in the bug in question such a binding was
> proposed as a _solution_ to slow-down.  What am I missing?)

In bug #22884, the pathological ~10s to insert a single character was
caused by continual repeated scanning between the ostensible BOD (i.e.
the "(" in column 0) and point.  IIRC, it was around 45k characters.  In
this particular case, setting open-paren-....-start to nil would indeed
have increased the speed.

This slowdown in Paul's config.h no longer happens, because of a
workaround put into syntax.c, I can't remember exactly what or when.
However, just last week, there was yet one more bug reported to CC Mode
about indentation, whose cause was a paren in C0 inside a comment.  I
would like these bug reports to cease.  As you said back in March, Emacs
should be able to handle these, as they are valid C.

But in general, having open-paren-....-start at nil slows down Emacs
massively, as shown above.  In CC Mode, this variable was bound to nil
around critical syntactic functions for some time, but this caused
complaints (in particular, from Martin Rudalics, and I think, from
yourself, too) at the slowdown.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Elias Mårtenson]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On 24 Dec 2016 5:44 pm, "Alan Mackenzie" <acm@muc.de> wrote:


This slowdown in Paul's config.h no longer happens, because of a
workaround put into syntax.c, I can't remember exactly what or when.
However, just last week, there was yet one more bug reported to CC Mode
about indentation, whose cause was a paren in C0 inside a comment.  I
would like these bug reports to cease.  As you said back in March, Emacs
should be able to handle these, as they are valid C.


I'm sorry for raising a question which may have an obvious answer, but as I
I have been following this thread I have become somewhat concerned with
confused as to the underlying cause of this issue.

Could you or someone else explain why an open paren in column 0 can affect
the parsing of syntax in cc-mode at all? After all, such parens have no
special syntactic nor semantic meaning in C nor C++.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 1584 bytes --]

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Elias.

On Sat, Dec 24, 2016 at 07:11:30PM +0800, Elias Mårtenson wrote:
> On 24 Dec 2016 5:44 pm, "Alan Mackenzie" <acm@muc.de> wrote:


>> This slowdown in Paul's config.h no longer happens, because of a
>> workaround put into syntax.c, I can't remember exactly what or when.
>> However, just last week, there was yet one more bug reported to CC
>> Mode about indentation, whose cause was a paren in C0 inside a
>> comment.  I would like these bug reports to cease.  As you said back
>> in March, Emacs should be able to handle these, as they are valid C.


> I'm sorry for raising a question which may have an obvious answer, but as I
> I have been following this thread I have become somewhat concerned with
> confused as to the underlying cause of this issue.

> Could you or someone else explain why an open paren in column 0 can affect
> the parsing of syntax in cc-mode at all? After all, such parens have no
> special syntactic nor semantic meaning in C nor C++.

It is a convention established in Emacs around ?40 years ago, that such
an open paren signalled the start of a defun.  This sped Emacs up
enormously, since the alternative was searching back to the beginning of
the buffer to check whether the paren was at the top level.

Currently, the only piece of low-level code which uses this convention is
back_comment in .../src/syntax.c.  This scans backward over a comment to
its beginning.  Scanning backwards is difficult, because you can't really
tell whether a string quote is contained within a comment, or a comment
marker might be contained within a string, ....  back_comment is
heuristic, and sort of works OK.  But it uses the open defun convention
to speed itself up.

When this convention is disabled (by setting
open-paren-in-column-0-is-defun-start to nil), Emacs runs s-l-o-w-l-y,
particularly in CC Mode (hence my concern).

The comment-cache branch solves these problems by NOT scanning comments
backwards.  Instead, it scans them forwards, storing the state of the
scanning at a particular point in a text property at that point.

I know that's not a brilliant answer.  Maybe the section in the emacs
manual might explain it better.  It's on page "Left Margin Paren".

> Regards,
> Elias

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> Date: Sat, 24 Dec 2016 11:36:20 +0000
> Cc: emacs-devel <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
> From: Alan Mackenzie <acm@muc.de>
> 
> > Could you or someone else explain why an open paren in column 0 can affect
> > the parsing of syntax in cc-mode at all? After all, such parens have no
> > special syntactic nor semantic meaning in C nor C++.
> 
> It is a convention established in Emacs around ?40 years ago, that such
> an open paren signalled the start of a defun.  This sped Emacs up
> enormously, since the alternative was searching back to the beginning of
> the buffer to check whether the paren was at the top level.
> 
> Currently, the only piece of low-level code which uses this convention is
> back_comment in .../src/syntax.c.  This scans backward over a comment to
> its beginning.  Scanning backwards is difficult, because you can't really
> tell whether a string quote is contained within a comment, or a comment
> marker might be contained within a string, ....  back_comment is
> heuristic, and sort of works OK.  But it uses the open defun convention
> to speed itself up.

I think one missing piece of the puzzle, which is what confused Elias,
is that open-paren-in-column-0-is-defun-start is not only about '(',
it's about any character that has the "open parenthesis" syntax class
under the current syntax tables.  And in C mode, that includes '{' and
'[', with the former being the important one.  I think.

Re: The current state of the comment-cache branch

[Andreas Röhler]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]



On 24.12.2016 13:00, Eli Zaretskii wrote:
>>
> I think one missing piece of the puzzle, which is what confused Elias,
> is that open-paren-in-column-0-is-defun-start is not only about '(',
> it's about any character that has the "open parenthesis" syntax class
> under the current syntax tables.  And in C mode, that includes '{' and
> '[', with the former being the important one.  I think.
>

Hi Alan and all,

there is no necessity for these `open-paren-in-column-0-is-defun-start' 
, as a function def starts at the beginning of the list, if its 
beginning is followed by the keyword "defun".

`open-paren-in-column-0-is-defun-start' will never be reliable WRT 
nested definitions.

Here is a suggestion how to solve this for Emacs Lisp:

ar-backward-defun

from

https://github.com/andreas-roehler/thingatpt-utils-core/blob/master/ar-subr.el

It should be possible to write things accordingly for other languages.


[-- Attachment #2: Type: text/html, Size: 1629 bytes --]

Re: The current state of the comment-cache branch

[Elias Mårtenson]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On 24 Dec 2016 7:36 pm, "Alan Mackenzie" <acm@muc.de> wrote:


> Could you or someone else explain why an open paren in column 0 can affect
> the parsing of syntax in cc-mode at all? After all, such parens have no
> special syntactic nor semantic meaning in C nor C++.

It is a convention established in Emacs around ?40 years ago, that such
an open paren signalled the start of a defun.  This sped Emacs up
enormously, since the alternative was searching back to the beginning of
the buffer to check whether the paren was at the top level.


Thank you and Eli for clarifying this. It makes much more sense to me now.

I'm still wondering how useful this could be for C code though. There are
many coding conventions where the opening { of a function does not go in
the left-most column. As far as I understand, such coding conventions would
lead to slower parsing?

I'm specifically asking this since I have sometimes used such style and at
least I don't think I've felt any slowness.

[-- Attachment #2: Type: text/html, Size: 1697 bytes --]

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Elias.

On Sat, Dec 24, 2016 at 08:21:32PM +0800, Elias Mårtenson wrote:
> On 24 Dec 2016 7:36 pm, "Alan Mackenzie" <acm@muc.de> wrote:


>> > Could you or someone else explain why an open paren in column 0 can affect
>> > the parsing of syntax in cc-mode at all? After all, such parens have no
>> > special syntactic nor semantic meaning in C nor C++.

>> It is a convention established in Emacs around ?40 years ago, that such
>> an open paren signalled the start of a defun.  This sped Emacs up
>> enormously, since the alternative was searching back to the beginning of
>> the buffer to check whether the paren was at the top level.


> Thank you and Eli for clarifying this. It makes much more sense to me now.

> I'm still wondering how useful this could be for C code though. There are
> many coding conventions where the opening { of a function does not go in
> the left-most column. As far as I understand, such coding conventions would
> lead to slower parsing?

This convention is not useful for C.  It is purely an artifice to speed
up the analysis of comment boundaries when scanning backwards.  It is
actually a hindrance to CC Mode.

> I'm specifically asking this since I have sometimes used such style and at
> least I don't think I've felt any slowness.

For small and smallish buffers, you're not going to notice.  For larger
buffers, compare performance with the convention enabled (i.e. the
variable open-paren-in-column-0-is-defun-start set to t) and disabled
(that variable set to nil).  There is a noticeable difference in
performance towards the end of large buffers.

The idea of the comment-cache branch is to get rigorous analysis of
comments (like having o-p-i-c-0-i-d-s nil) together with the speed which
is currently provided by the convention (with o-p-i-c-0-i-d-s t).

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> Date: Tue, 27 Dec 2016 17:55:00 +0000
> Cc: emacs-devel <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
> From: Alan Mackenzie <acm@muc.de>
> 
> > I'm still wondering how useful this could be for C code though. There are
> > many coding conventions where the opening { of a function does not go in
> > the left-most column. As far as I understand, such coding conventions would
> > lead to slower parsing?
> 
> This convention is not useful for C.

Our coding standards clearly call for starting a function's body with
an opening brace in column zero.

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Eli.

On Wed, Dec 28, 2016 at 05:36:58PM +0200, Eli Zaretskii wrote:
> > Date: Tue, 27 Dec 2016 17:55:00 +0000
> > Cc: emacs-devel <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
> > From: Alan Mackenzie <acm@muc.de>

> > > I'm still wondering how useful this could be for C code though. There are
> > > many coding conventions where the opening { of a function does not go in
> > > the left-most column. As far as I understand, such coding conventions would
> > > lead to slower parsing?

> > This convention is not useful for C.

> Our coding standards clearly call for starting a function's body with
> an opening brace in column zero.

Yes.  But there are other conventions where, for example, a function's
opening brace is hanging at the right hand side, e.g. in AWK Mode.  In
C++ Mode, when functions are written inside a namespace, they might be
indented to reflect that, rather than put at column 0.

I think what I really meant was that the convention was not helpful for
CC Mode.  ;-)

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Nikolaus Rath]

On Dec 28 2016, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 27 Dec 2016 17:55:00 +0000
>> Cc: emacs-devel <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
>> From: Alan Mackenzie <acm@muc.de>
>> 
>> > I'm still wondering how useful this could be for C code though. There are
>> > many coding conventions where the opening { of a function does not go in
>> > the left-most column. As far as I understand, such coding conventions would
>> > lead to slower parsing?
>> 
>> This convention is not useful for C.
>
> Our coding standards clearly call for starting a function's body with
> an opening brace in column zero.

I would hope that Emacs is used not only to edit its own source code but
also other code, for which these standards don't apply.


Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> From: Nikolaus Rath <Nikolaus@rath.org>
> Date: Wed, 28 Dec 2016 08:45:15 -0800
> 
> >> > I'm still wondering how useful this could be for C code though. There are
> >> > many coding conventions where the opening { of a function does not go in
> >> > the left-most column. As far as I understand, such coding conventions would
> >> > lead to slower parsing?
> >> 
> >> This convention is not useful for C.
> >
> > Our coding standards clearly call for starting a function's body with
> > an opening brace in column zero.
> 
> I would hope that Emacs is used not only to edit its own source code but
> also other code, for which these standards don't apply.

It is, but how is that relevant to the issue at hand?  The issue at
hand is how does open-paren-in-column-0-is-defun-start affect C code
editing.  The intent of my reference to GNU standards was to point
out that having an opening brace in column zero is a very frequent
situation in the C sources of any GNU project, and therefore saying
this convention is not useful for C flies in the face of the reality
right under our feet.

Re: The current state of the comment-cache branch

[Nikolaus Rath]

On Dec 28 2016, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Nikolaus Rath <Nikolaus@rath.org>
>> Date: Wed, 28 Dec 2016 08:45:15 -0800
>> 
>> >> > I'm still wondering how useful this could be for C code though. There are
>> >> > many coding conventions where the opening { of a function does not go in
>> >> > the left-most column. As far as I understand, such coding conventions would
>> >> > lead to slower parsing?
>> >> 
>> >> This convention is not useful for C.
>> >
>> > Our coding standards clearly call for starting a function's body with
>> > an opening brace in column zero.
>> 
>> I would hope that Emacs is used not only to edit its own source code but
>> also other code, for which these standards don't apply.
>
> It is, but how is that relevant to the issue at hand?  The issue at
> hand is how does open-paren-in-column-0-is-defun-start affect C code
> editing.  The intent of my reference to GNU standards was to point
> out that having an opening brace in column zero is a very frequent
> situation in the C sources of any GNU project, and therefore saying
> this convention is not useful for C flies in the face of the reality
> right under our feet.

What I meant is that setting open-paren-in-column-0-is-defun-start to t
in general is not a good idea, because it does not work for all C
files. So for this option to be useful, you'd have to do set it
conditionally - which IMO restricts its usefulness a lot.

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> From: Nikolaus Rath <Nikolaus@rath.org>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Dec 2016 15:58:53 -0800
> 
> >> I would hope that Emacs is used not only to edit its own source code but
> >> also other code, for which these standards don't apply.
> >
> > It is, but how is that relevant to the issue at hand?  The issue at
> > hand is how does open-paren-in-column-0-is-defun-start affect C code
> > editing.  The intent of my reference to GNU standards was to point
> > out that having an opening brace in column zero is a very frequent
> > situation in the C sources of any GNU project, and therefore saying
> > this convention is not useful for C flies in the face of the reality
> > right under our feet.
> 
> What I meant is that setting open-paren-in-column-0-is-defun-start to t
> in general is not a good idea, because it does not work for all C
> files. So for this option to be useful, you'd have to do set it
> conditionally - which IMO restricts its usefulness a lot.

We have that option set to t for a long time, so the decision whether
to do it was already made, and any problems that could cause are
already with us.

Re: The current state of the comment-cache branch

[Nikolaus Rath]

On Dec 29 2016, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Nikolaus Rath <Nikolaus@rath.org>
>> Cc: emacs-devel@gnu.org
>> Date: Wed, 28 Dec 2016 15:58:53 -0800
>> 
>> >> I would hope that Emacs is used not only to edit its own source code but
>> >> also other code, for which these standards don't apply.
>> >
>> > It is, but how is that relevant to the issue at hand?  The issue at
>> > hand is how does open-paren-in-column-0-is-defun-start affect C code
>> > editing.  The intent of my reference to GNU standards was to point
>> > out that having an opening brace in column zero is a very frequent
>> > situation in the C sources of any GNU project, and therefore saying
>> > this convention is not useful for C flies in the face of the reality
>> > right under our feet.
>> 
>> What I meant is that setting open-paren-in-column-0-is-defun-start to t
>> in general is not a good idea, because it does not work for all C
>> files. So for this option to be useful, you'd have to do set it
>> conditionally - which IMO restricts its usefulness a lot.
>
> We have that option set to t for a long time, so the decision whether
> to do it was already made, and any problems that could cause are
> already with us.

Yes. Therefore, I think applying a patch that eliminates the need for it
completely would be a good thing.


Best,
-Nikolaus
-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> From: Nikolaus Rath <Nikolaus@rath.org>
> Cc: emacs-devel@gnu.org
> Date: Thu, 29 Dec 2016 08:56:25 -0800
> 
> > We have that option set to t for a long time, so the decision whether
> > to do it was already made, and any problems that could cause are
> > already with us.
> 
> Yes. Therefore, I think applying a patch that eliminates the need for it
> completely would be a good thing.

I agree.  But I'm not sure this is what Alan wants to achieve...

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Eli.

On Thu, Dec 29, 2016 at 07:46:04PM +0200, Eli Zaretskii wrote:
> > From: Nikolaus Rath <Nikolaus@rath.org>
> > Cc: emacs-devel@gnu.org
> > Date: Thu, 29 Dec 2016 08:56:25 -0800

> > > We have that option set to t for a long time, so the decision whether
> > > to do it was already made, and any problems that could cause are
> > > already with us.

> > Yes. Therefore, I think applying a patch that eliminates the need for it
> > completely would be a good thing.

> I agree.  But I'm not sure this is what Alan wants to achieve...

This (eliminating the need for open-paren-in-column-0-is-defun-start) is
one of things I would love to achieve.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Andreas Röhler]

On 29.12.2016 20:44, Alan Mackenzie wrote:
> Hello, Eli.
>
> On Thu, Dec 29, 2016 at 07:46:04PM +0200, Eli Zaretskii wrote:
>>> From: Nikolaus Rath <Nikolaus@rath.org>
>>> Cc: emacs-devel@gnu.org
>>> Date: Thu, 29 Dec 2016 08:56:25 -0800
>>>> We have that option set to t for a long time, so the decision whether
>>>> to do it was already made, and any problems that could cause are
>>>> already with us.
>>> Yes. Therefore, I think applying a patch that eliminates the need for it
>>> completely would be a good thing.
>> I agree.  But I'm not sure this is what Alan wants to achieve...
> This (eliminating the need for open-paren-in-column-0-is-defun-start) is
> one of things I would love to achieve.
>

A crucial thing IMHO. Should it not be possible here, consider it worth 
a fork. Alongside with some other impediments removed, for example 
allowing all chars being an abbrev.

Re: The current state of the comment-cache branch

[Stefan Monnier]

> Alongside with some other impediments removed, for example allowing
> all chars being an abbrev.

The restriction to word chars in abbrevs as removed a long time ago.


        Stefan

Re: The current state of the comment-cache branch

[Andreas Röhler]

On 03.01.2017 18:39, Stefan Monnier wrote:
>> Alongside with some other impediments removed, for example allowing
>> all chars being an abbrev.
> The restriction to word chars in abbrevs as removed a long time ago.
>
>
>          Stefan
>
>

Really? That should be known.

So https://debbugs.gnu.org/cgi/bugreport.cgi?bug=15485 is fixed?

Info still mentions words to expand, not just symbols.


29.2 Defining Abbrevs
=====================

‘C-x a g’
      Define an abbrev, using one or more words before point as its
      expansion (‘add-global-abbrev’).

Re: The current state of the comment-cache branch

[Stefan Monnier]

>>> Alongside with some other impediments removed, for example allowing
>>> all chars being an abbrev.
>> The restriction to word chars in abbrevs as removed a long time ago.
> Really? That should be known.

It is known.

> So https://debbugs.gnu.org/cgi/bugreport.cgi?bug=15485 is fixed?

No, but I'm not sure it's a bug.  It's the way `add-abbrev` works.
The abbrev-tables themselves don't have that restriction.

> 29.2 Defining Abbrevs

There's a big difference between what abbrev-tables can do, and the part
of that functionality you can use via things like C-x a g.


        Stefan

Re: The current state of the comment-cache branch

[Andreas Röhler]

On 20.01.2017 22:48, Stefan Monnier wrote:
>>>> Alongside with some other impediments removed, for example allowing
>>>> all chars being an abbrev.
>>> The restriction to word chars in abbrevs as removed a long time ago.
>> Really? That should be known.
> It is known.

It's known you delivered some code WRT that purpose.
It never worked here. Seems other faced some difficulties too:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=23891



>> So https://debbugs.gnu.org/cgi/bugreport.cgi?bug=15485 is fixed?
> No, but I'm not sure it's a bug.  It's the way `add-abbrev` works.
> The abbrev-tables themselves don't have that restriction.

So what does abbrev--check-chars inside define-mode-abbrev?:

         (error "Some abbrev characters (%s) are not word constituents %s"

A related issue later on:

   (define-abbrev local-abbrev-table (downcase abbrev) expansion))


If everything is downcased, expansion can't distinguish the original case.

Re: The current state of the comment-cache branch

[Stefan Monnier]

>>> So https://debbugs.gnu.org/cgi/bugreport.cgi?bug=15485 is fixed?
>> No, but I'm not sure it's a bug.  It's the way `add-abbrev` works.
>> The abbrev-tables themselves don't have that restriction.
> So what does abbrev--check-chars inside define-mode-abbrev?:

add-abbrev and define-mode-abbrev: même combat!
The abbrev-tables themselves don't have that restriction.

Clearly, you've looked at the definition of define-mode-abbrev,
so I hope you realized that this definition is extremely short: it's
a very thin layer above `define-abbrev`.  Which means it'd be rather
easy to define such a layer which doesn't have those limitations
(since define-abbrev doesn't have those limitations).
Patches welcome (with appropriate paperwork, of course).


        Stefan


PS: But note that it's not quite as simple as I make it sound: in order
to define a non-word abbrev, the abbrev-table has to specify
a different :regexp property.

Re: The current state of the comment-cache branch

[Stefan Monnier]

> I would hope that Emacs is used not only to edit its own source code but
> also other code, for which these standards don't apply.

The open-paren-in-column-0-is-defun-start was a useful heuristic
optimization, which worked well in Lisp-derivatives and reasonably well
in C derivatives as well.  It was a good tradeoff (the occasional
inconvenience was small and infrequent compared to the performance
gains).

Nowadays, the performance-v-correctness balance is different, and we
have enough resources to use another optimization, based on caching,
which does not suffer from those inconveniences.


        Stefan

Re: The current state of the comment-cache branch

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

One useful thing about open-paren-in-column-0-is-defun-start
is that you can choose to treat an open paren as a defun start
even though there is another level of parentheses outside it.

A heuristic that works "automatically" won't give the user a choice.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: The current state of the comment-cache branch

[Stefan Monnier]

> One useful thing about open-paren-in-column-0-is-defun-start
> is that you can choose to treat an open paren as a defun start
> even though there is another level of parentheses outside it.

I believe you're talking a bout the use of
open-paren-in-column-0-is-defun-start in beginning-of-defun, which is
completely independent from the use which we're discussing here.


        Stefan

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> Date: Sat, 24 Dec 2016 09:42:46 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: emacs-devel@gnu.org
> 
> > Can you show similar timings with that variable set to nil?  And in
> > particular in the use case reported by Paul back when bug#22884 was
> > filed?
> 
> master:                        101.939s            95.049s        103.546s
> (with open-paren-... nil)

Hmm... doesn't match my timings here.

With an optimized (-Og) build of Emacs 25.1.90, I get 42.844 with
open-paren-in-column-0-is-defun-start t and 62.234 nil.  With the
unoptimized build of the current master, I get 193.922 and 1117.469,
respectively.  The 193 sec result is marginally reasonable for an
unoptimized build, the 1117 sec result is IMO preposterous, and the
6-fold slowdown is IMO too much to be explained just by the variable.

Anyway, is your suggestion to adopt the code on that branch and set
open-paren-in-column-0-is-defun-start to nil?  If we are not going to
set it to nil, then the speed advantage is IMO too small to justify
the changes.

Also, I wonder why do we need all these changes in syntax.c, when the
problem is AFAIK only with C mode and its derivatives.  Are these
changes beneficial in any way to modes with non-C syntax?

Thanks.

P.S. What does "hwm" stand for in literal-cache-hwm?

Re: The current state of the comment-cache branch

[Paul Eggert]

Eli Zaretskii wrote:
> P.S. What does "hwm" stand for in literal-cache-hwm?

Presumably "high water mark", as in:

https://en.wikipedia.org/wiki/High_water_mark

Re: The current state of the comment-cache branch

[Stefan Monnier]

> Also, I wonder why do we need all these changes in syntax.c, when the
> problem is AFAIK only with C mode and its derivatives.  Are these
> changes beneficial in any way to modes with non-C syntax?

The core of the problem is calls to (forward-comment -1).

Both this comment-cache and my syntax-ppss patch attack this specific
operation only.  This operation is currently implemented by trying to
skip comments "locally" by parsing backward, but when that fails, we use
parse-partial-sexp from point-min to find the matching opening of
a comment closer.

This expensive forward pass is not a problem in practice unless your buffer
is huge, or unless it happens many times within a single command.
These can happen in any major mode, basically.
It's only a historical accident if it seems to affect CC-mode more.

open-paren-in-column-0-is-defun-start is the current hack to try and
reduce the occurrence of this problem by only calling parse-partial-sexp
from the closest open-paren-in-column-0, which works fairly well in
practice for typical Elisp code as well as for typical C code (tho not
for all coding styles).

[ AFAIK open-paren-in-column-0-is-defun-start was specifically designed
for this (forward-comment -1) issue, but it is also used in
beginning-of-defun, although the two uses are completely independent and
of a different nature: the use in forward-comment is supposed to be only
an optimization, whereas the use in beginning-of-defun is meant to
really change the result.  So beginning-of-defun is completely
irrelevant to and independent from this thread.  ]

Also in the latest case where a major slow down showed up in CC-mode,
the problem was reversed: CC-mode had some special code for when
open-paren-in-column-0-is-defun-start is non-nil (can't remember what
it was for), which was the cause of the slow down, IIRC.

Both Alan's comment-cache and my syntax-ppss patch aim to replace the
open-paren-in-column-0-is-defun-start hack so as to completely eliminate
this pathological (forward-comment -1) case.


        Stefan

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 25 Dec 2016 11:07:46 -0500
> 
> It's only a historical accident if it seems to affect CC-mode more.

Given that CC Mode is the only one affected, I wouldn't dismiss this
as an accident, not without a good explanation what differences
between CC and other modes triggers that.

Thanks for the other explanations.

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Stefan.

Thanks for such a thorough summary of the issue.

On Sun, Dec 25, 2016 at 11:07:46AM -0500, Stefan Monnier wrote:
> > Also, I wonder why do we need all these changes in syntax.c, when the
> > problem is AFAIK only with C mode and its derivatives.  Are these
> > changes beneficial in any way to modes with non-C syntax?

The actual cause of the problem is in syntax.c.

> The core of the problem is calls to (forward-comment -1).

More precisely, in CC Mode, what triggers the problem is usually
scan-lists with a negative `count' argument.  This calls back_comment,
which is where the problem happens.

> Both this comment-cache and my syntax-ppss patch attack this specific
> operation only.  This operation is currently implemented by trying to
> skip comments "locally" by parsing backward, but when that fails, we use
> parse-partial-sexp from point-min to find the matching opening of
> a comment closer.

> This expensive forward pass is not a problem in practice unless your buffer
> is huge, or unless it happens many times within a single command.
> These can happen in any major mode, basically.
> It's only a historical accident if it seems to affect CC-mode more.

> open-paren-in-column-0-is-defun-start is the current hack to try and
> reduce the occurrence of this problem by only calling parse-partial-sexp
> from the closest open-paren-in-column-0, which works fairly well in
> practice for typical Elisp code as well as for typical C code (tho not
> for all coding styles).

> [ AFAIK open-paren-in-column-0-is-defun-start was specifically designed
> for this (forward-comment -1) issue, but it is also used in
> beginning-of-defun, although the two uses are completely independent and
> of a different nature: the use in forward-comment is supposed to be only
> an optimization, whereas the use in beginning-of-defun is meant to
> really change the result.  So beginning-of-defun is completely
> irrelevant to and independent from this thread.  ]

> Also in the latest case where a major slow down showed up in CC-mode,
> the problem was reversed: CC-mode had some special code for when
> open-paren-in-column-0-is-defun-start is non-nil (can't remember what
> it was for), which was the cause of the slow down, IIRC.

Bug #22884.  This isn't quite accurate.  The slowdown happened when C
Mode thought that the "(" in the comment was a BOD and in repeated
scanning to point (which is typically harmless) was scanning many times
over ~40k characters.

> Both Alan's comment-cache and my syntax-ppss patch aim to replace the
> open-paren-in-column-0-is-defun-start hack so as to completely eliminate
> this pathological (forward-comment -1) case.

Is your syntax-ppss patch at a state where it could be benchmarked and
tested?  If so, please say again where it is, or put it somewhere
public.  Dmitry is interested in it too.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Stefan Monnier]

> Is your syntax-ppss patch at a state where it could be benchmarked and
> tested?

I already posted it, mentioning that I've been using it ever since the
last discussion several months ago.


        Stefan

Re: The current state of the comment-cache branch

[Stefan Monnier]

>> Also in the latest case where a major slow down showed up in CC-mode,
>> the problem was reversed: CC-mode had some special code for when
>> open-paren-in-column-0-is-defun-start is non-nil (can't remember what
>> it was for), which was the cause of the slow down, IIRC.

> Bug #22884.  This isn't quite accurate.  The slowdown happened when C
> Mode thought that the "(" in the comment was a BOD and in repeated
> scanning to point (which is typically harmless) was scanning many times
> over ~40k characters.

The thing I don't understand in this explanation is that if
open-paren-in-column-0-is-defun-start is set to nil the problem did not
show up, yet setting it to nil means that the scanning was done over
*more* than 40K characters so it should have made the problem worse.


        Stefan

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Eli.

On Sat, Dec 24, 2016 at 02:27:26PM +0200, Eli Zaretskii wrote:
> > Date: Sat, 24 Dec 2016 09:42:46 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: emacs-devel@gnu.org

> > > Can you show similar timings with that variable set to nil?  And in
> > > particular in the use case reported by Paul back when bug#22884 was
> > > filed?

> > master:                        101.939s            95.049s        103.546s
> > (with open-paren-... nil)

> Hmm... doesn't match my timings here.

> With an optimized (-Og) build of Emacs 25.1.90, I get 42.844 with
> open-paren-in-column-0-is-defun-start t and 62.234 nil.  With the
> unoptimized build of the current master, I get 193.922 and 1117.469,
> respectively.  The 193 sec result is marginally reasonable for an
> unoptimized build, the 1117 sec result is IMO preposterous, and the
> 6-fold slowdown is IMO too much to be explained just by the variable.

If the buffer is large enough (were you using xdisp.c?) a large factor
slowdown from using o-p-i-c-0-i-d-s seems inevitable.

> Anyway, is your suggestion to adopt the code on that branch and set
> open-paren-in-column-0-is-defun-start to nil?  If we are not going to
> set it to nil, then the speed advantage is IMO too small to justify
> the changes.

No.  My suggestion is that open-paren-in-column-0-is-defun start ceases
to play any role in syntax.c.  Instead comments and strings will be
scanned only in the forward direction, the results of these scan
operations being cached in a text property.  `back_comment' in syntax.c
will be superseded by a new implementation which uses these text
properties, and various other parts of Emacs will be amended to ensure
these text properties stay consistent with the buffer and the current
syntax table.  All of this is implemented in the comment-cache branch,
and completely removes the cause of all of these paren bugs.

o-p-i-c-0-i-d-s would then have a role only in lisp.el in
`beginning-of-defun-raw'.

> Also, I wonder why do we need all these changes in syntax.c, when the
> problem is AFAIK only with C mode and its derivatives.  Are these
> changes beneficial in any way to modes with non-C syntax?

The changes are in syntax.c because this is where the fundamental cause
of the problem lies.  I don't believe it is possible to solve them in CC
Mode.  I don't know why the problem doesn't manifest itself in other
modes.  Perhaps other modes do less scanning of comments backwards (with
`scan-sexps' with a negative count).

> Thanks.

> P.S. What does "hwm" stand for in literal-cache-hwm?

As Paul suggested, "high water mark".  It is perhaps not a very good
name and could be changed.  The allusion is to the highest point on a
beach reached by the sea at high tide.  This is marked as a line on some
(English) maps under the abbreviation HWMOT, "high water mark, ordinary
tide".

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Eli Zaretskii]

> Date: Tue, 27 Dec 2016 16:40:18 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> Hello, Eli.
> 
> On Sat, Dec 24, 2016 at 02:27:26PM +0200, Eli Zaretskii wrote:
> > > Date: Sat, 24 Dec 2016 09:42:46 +0000
> > > From: Alan Mackenzie <acm@muc.de>
> > > Cc: emacs-devel@gnu.org
> 
> > > > Can you show similar timings with that variable set to nil?  And in
> > > > particular in the use case reported by Paul back when bug#22884 was
> > > > filed?
> 
> > > master:                        101.939s            95.049s        103.546s
> > > (with open-paren-... nil)
> 
> > Hmm... doesn't match my timings here.
> 
> > With an optimized (-Og) build of Emacs 25.1.90, I get 42.844 with
> > open-paren-in-column-0-is-defun-start t and 62.234 nil.  With the
> > unoptimized build of the current master, I get 193.922 and 1117.469,
> > respectively.  The 193 sec result is marginally reasonable for an
> > unoptimized build, the 1117 sec result is IMO preposterous, and the
> > 6-fold slowdown is IMO too much to be explained just by the variable.
> 
> If the buffer is large enough (were you using xdisp.c?) a large factor
> slowdown from using o-p-i-c-0-i-d-s seems inevitable.

I was using the same file xdisp.c and the same timing code as you.
That's why I don't understand how come our results are so different.
With such differences unexplained, how can we ever trust these
benchmarks?

> > Anyway, is your suggestion to adopt the code on that branch and set
> > open-paren-in-column-0-is-defun-start to nil?  If we are not going to
> > set it to nil, then the speed advantage is IMO too small to justify
> > the changes.
> 
> No.  My suggestion is that open-paren-in-column-0-is-defun start ceases
> to play any role in syntax.c.

Does that mean to effectively set it to nil?  Otherwise, I don't
understand how can we judge the current situation vs what you suggest.

> Instead comments and strings will be scanned only in the forward
> direction, the results of these scan operations being cached in a
> text property.

Btw, adding text properties slows down redisplay.  I don't think the
slow-down is significant, but it's worth having in mind anyway.

> > Also, I wonder why do we need all these changes in syntax.c, when the
> > problem is AFAIK only with C mode and its derivatives.  Are these
> > changes beneficial in any way to modes with non-C syntax?
> 
> The changes are in syntax.c because this is where the fundamental cause
> of the problem lies.  I don't believe it is possible to solve them in CC
> Mode.  I don't know why the problem doesn't manifest itself in other
> modes.  Perhaps other modes do less scanning of comments backwards (with
> `scan-sexps' with a negative count).

If we don't completely understand the problem, and cannot explain why
it happens only in CC mode, then how do we make sure we solve the
problem which needs to be solved?  Why would the amount of
back-scanning through comments depend on the major mode?  I don't
think that C programmers write more/longer comments than programmers
in any other programming language.

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Eli.

On Wed, Dec 28, 2016 at 05:35:47PM +0200, Eli Zaretskii wrote:
> > Date: Tue, 27 Dec 2016 16:40:18 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Hello, Eli.

> > On Sat, Dec 24, 2016 at 02:27:26PM +0200, Eli Zaretskii wrote:
> > > > Date: Sat, 24 Dec 2016 09:42:46 +0000
> > > > From: Alan Mackenzie <acm@muc.de>
> > > > Cc: emacs-devel@gnu.org

> > > > > Can you show similar timings with that variable set to nil?  And in
> > > > > particular in the use case reported by Paul back when bug#22884 was
> > > > > filed?

> > > > master:                        101.939s            95.049s        103.546s
> > > > (with open-paren-... nil)

> > > Hmm... doesn't match my timings here.

> > > With an optimized (-Og) build of Emacs 25.1.90, I get 42.844 with
> > > open-paren-in-column-0-is-defun-start t and 62.234 nil.  With the
> > > unoptimized build of the current master, I get 193.922 and 1117.469,
> > > respectively.  The 193 sec result is marginally reasonable for an
> > > unoptimized build, the 1117 sec result is IMO preposterous, and the
> > > 6-fold slowdown is IMO too much to be explained just by the variable.

Might your unoptimized build have included the extra checking code which
can be enabled?  My debugging build didn't.

> > If the buffer is large enough (were you using xdisp.c?) a large factor
> > slowdown from using o-p-i-c-0-i-d-s seems inevitable.

> I was using the same file xdisp.c and the same timing code as you.
> That's why I don't understand how come our results are so different.
> With such differences unexplained, how can we ever trust these
> benchmarks?

Well, they are qualitative, and somewhat quantitative.  They are enough
to say "quite a bit slower" with, even if they can't tell us "exactly
44.5% slower".

> > > Anyway, is your suggestion to adopt the code on that branch and set
> > > open-paren-in-column-0-is-defun-start to nil?  If we are not going to
> > > set it to nil, then the speed advantage is IMO too small to justify
> > > the changes.

> > No.  My suggestion is that open-paren-in-column-0-is-defun start ceases
> > to play any role in syntax.c.

> Does that mean to effectively set it to nil?  Otherwise, I don't
> understand how can we judge the current situation vs what you suggest.

The code in comment-cache does back_comment by a totally different
algorithm which simply doesn't involve
open-paren-in-column-0-is-defun-start.  It is as fast as open-... being
set to t, yet as accurate (or more so) than open-... being set to nil.

> > Instead comments and strings will be scanned only in the forward
> > direction, the results of these scan operations being cached in a
> > text property.

> Btw, adding text properties slows down redisplay.  I don't think the
> slow-down is significant, but it's worth having in mind anyway.

I don't think it's significant, either.  time-scroll redisplays the
entire buffer, and we don't see a slow-down there.

> > > Also, I wonder why do we need all these changes in syntax.c, when the
> > > problem is AFAIK only with C mode and its derivatives.  Are these
> > > changes beneficial in any way to modes with non-C syntax?

> > The changes are in syntax.c because this is where the fundamental cause
> > of the problem lies.  I don't believe it is possible to solve them in CC
> > Mode.  I don't know why the problem doesn't manifest itself in other
> > modes.  Perhaps other modes do less scanning of comments backwards (with
> > `scan-sexps' with a negative count).

Sorry, that scan-sexps should have been scan-lists.

> If we don't completely understand the problem, and cannot explain why
> it happens only in CC mode, then how do we make sure we solve the
> problem which needs to be solved?

I think I do understand the problem fully, and I think Stefan does, too.

> Why would the amount of back-scanning through comments depend on the
> major mode?  I don't think that C programmers write more/longer
> comments than programmers in any other programming language.

It might be that functions like c-parse-state (which does a lot of
scan-lists in the backward direction) are more prominent in CC Mode.
Could it possibly be that most other languages don't have such large
buffers as C, C++, ...?  It might be that wrongly recognising an open
paren as a BOD does less damage to syntax analysis in other languages.
Or it could be that apostrophes are used a lot in comments, and in CC
Mode modes, "'" has string syntax - an odd number of apostrophes in a
comment will then trigger the code which involves open-paren-in-....

Or, it could be a combination of all these things.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  >                    First forward run     Backwards     Second forward run

  > master:                 34.100s            36.050s	   34.575s 

  > comment-cache:          30.110s            32.275s         34.990s

Judging by those figures, it seems to be a rather small improvement.
Is it worth adding complexity to get a 13% speedup in one operation?

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Richard.

On Sat, Dec 24, 2016 at 01:54:27PM -0500, Richard Stallman wrote:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]

>   >                    First forward run     Backwards     Second forward run

>   > master:                 34.100s            36.050s	   34.575s 

>   > comment-cache:          30.110s            32.275s         34.990s

> Judging by those figures, it seems to be a rather small improvement.
> Is it worth adding complexity to get a 13% speedup in one operation?

Sorry for not being clear over the motivation, here.  Lots of bugs
happen in CC Mode because of the convention "open paren at column 0 is
defun start".  I would like these bugs to stop happening completely,
which was why I designed and coded up branch comment-cache.

I gave the timings to show that the comment-cache code isn't
significantly _slower_ than the existing code in syntax.c.

The bugs happen in (e.g.) C Mode when somebody writes, say, a "(" in
column zero inside a comment.  This happened in the Emacs C source in
March 2016, when a "(" in the GPL 3 licence statement found itself in
column zero.  This showed itself as a simple insert operation taking
inordinately long (~10s) to redisplay.  This was reported by Paul Eggert
as bug #22884.

Other manifestations of this "bug" have been reported since bug #22884.

Setting open-paren-in-column-0-is-defun-start to nil solves the problem,
but at the unacceptable cost of making CC Mode much slower (and it's not
the fastest mode in the first place).

> -- 
> Dr Richard Stallman

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Dmitry Gutov]

On 27.12.2016 18:11, Alan Mackenzie wrote:

> Setting open-paren-in-column-0-is-defun-start to nil solves the problem,
> but at the unacceptable cost of making CC Mode much slower (and it's not
> the fastest mode in the first place).

Could you show the benchmarks against that, then?

Please also include the alternative (syntax-ppss based) patch in the 
comparison.

Re: The current state of the comment-cache branch

[Alan Mackenzie]

Hello, Dmitry.

On Wed, Dec 28, 2016 at 03:40:00AM +0200, Dmitry Gutov wrote:
> On 27.12.2016 18:11, Alan Mackenzie wrote:

> > Setting open-paren-in-column-0-is-defun-start to nil solves the problem,
> > but at the unacceptable cost of making CC Mode much slower (and it's not
> > the fastest mode in the first place).

> Could you show the benchmarks against that, then?

I've already posted them in this thread, but just for convenience, here
again are the timings on my machine for `time-scroll' (source below) for 
.../src/xdisp.c:

                   First forward run     Backwards     Second forward run


master:                 34.100s            36.050s         34.575s

comment-cache:          30.110s            32.275s         34.990s

master:                101.939s            95.049s        103.546s
(with open-paren-... nil)


#########################################################################
(defmacro time-it (&rest forms)
  "Time the running of a sequence of forms using `float-time'.
Call like this: \"M-: (time-it (foo ...) (bar ...) ...)\"."
  `(let ((start (float-time)))
    ,@forms
    (- (float-time) start)))

(defun time-scroll (&optional arg)
  (interactive "P")
  (message "%s"
           (time-it
            (condition-case nil
                (while t
                  (if arg (scroll-down) (scroll-up))
                  (sit-for 0))
              (error nil)))))
#########################################################################



> Please also include the alternative (syntax-ppss based) patch in the 
> comparison.

Does such a patch exist, yet?  If so, could you point me to it, please,
then I'll put it through my benchmark.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: The current state of the comment-cache branch

[Dmitry Gutov]

On 28.12.2016 09:54, Alan Mackenzie wrote:

>> Please also include the alternative (syntax-ppss based) patch in the
>> comparison.
>
> Does such a patch exist, yet?  If so, could you point me to it, please,
> then I'll put it through my benchmark.

It's https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884#88, AFAIK.

Thanks.