Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

>     Apply `comment-depth' text properties when calling `back_comment'.

FWIW, I think if you want to speed up back_comment, a simpler approach
is to make it use syntax-ppss, which already implements a cache, and
will usually already have the cache filled for you.

IIRC this should be easy to do, conceptually something akin to

    diff --git a/src/syntax.c b/src/syntax.c
    index 9fc76a6..229e615 100644
    --- a/src/syntax.c
    +++ b/src/syntax.c
    @@ -903,6 +903,8 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte, ptrdiff_t stop,
     	 to the one in question; this records where we
     	 last passed a comment starter.  */
           /* If we did not already find the defun start, find it now.  */
    +      if (use_syntax_ppss)
    +        return (nth 8 (syntax-ppss <somewhere>));
           if (defun_start == 0)
     	{
     	  defun_start = find_defun_start (comment_end, comment_end_byte);

I did have a prototype working at some point, but I can't get my hands
on it right now.  IIRC the way it worked was basically by calling a new
syntax--back-comment function (in syntax.el) which did the (nth
8 (syntax-ppss)) dance.


	Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Stefan.

On Tue, Mar 08, 2016 at 09:19:46AM -0500, Stefan Monnier wrote:
> >     Apply `comment-depth' text properties when calling `back_comment'.

> FWIW, I think if you want to speed up back_comment, a simpler approach
> is to make it use syntax-ppss, which already implements a cache, and
> will usually already have the cache filled for you.

My changes might speed up back_comment, but that's not the prime reason
for them.  Rather, I want utterly to expunge all the nonsense about
parens in column 0.  That a high class editor such as Emacs should have
problems with such parens is ludicrous and unacceptable.

The amount of time I have lost investigating reported bugs, only to
realise that an open paren in column 0 was the cause, would exceed any
reasonable person's guess.  Bug #22884, reported by Paul last week, is
only the latest in a long list.

With my change, open_paren_in_column_0_is_defun_start simply vanishes
from syntax.c[*], as does `find_defun_start'.  Users can freely write
parens in their strings and comments.  Comments are scanned only in the
forward direction, getting rid of all the nastiness about mixed quote
characters, and the like.  It doesn't appear to cause undue delay
anywhere (on my machine, scanning the entirety of xdisp.c for literals
takes about 0.25 seconds).

[*] OK, it stays in as a declaration for Lisp programs, but isn't used
in syntax.c.

[ .... ]

> 	Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

>> >     Apply `comment-depth' text properties when calling `back_comment'.
>> FWIW, I think if you want to speed up back_comment, a simpler approach
>> is to make it use syntax-ppss, which already implements a cache, and
>> will usually already have the cache filled for you.
> My changes might speed up back_comment, but that's not the prime reason
> for them.  Rather, I want utterly to expunge all the nonsense about
> parens in column 0.

Not sure how the two differ.  The main/only real use for this "paren in
column 0" hack is for back_comment.  IOW my comment also applies to "if
you want to get rid of all the nonsense about parens in column 0".

> That a high class editor such as Emacs should have
> problems with such parens is ludicrous and unacceptable.

FWIW, I (setq open-paren-in-column-0-is-defun-start nil) in my ~/.emacs.

> With my change, open_paren_in_column_0_is_defun_start simply vanishes
> from syntax.c[*], as does `find_defun_start'.

Using syntax-ppss would give you the same benefit.

My point is that instead of creating a new kind of "syntax cache" for
this specific purpose, we'd be better off using the cache we already have.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello again, Stefan.

On Tue, Mar 08, 2016 at 01:42:28PM -0500, Stefan Monnier wrote:
> >> >     Apply `comment-depth' text properties when calling `back_comment'.
> >> FWIW, I think if you want to speed up back_comment, a simpler approach
> >> is to make it use syntax-ppss, which already implements a cache, and
> >> will usually already have the cache filled for you.
> > My changes might speed up back_comment, but that's not the prime reason
> > for them.  Rather, I want utterly to expunge all the nonsense about
> > parens in column 0.

> Not sure how the two differ.  The main/only real use for this "paren in
> column 0" hack is for back_comment.  IOW my comment also applies to "if
> you want to get rid of all the nonsense about parens in column 0".

syntax-ppss is a lisp level thing, rather loosely defined.  The text
property cache I have built is more rigorous; it is nearly impervious to
anything a non-malicious lisp program might do to it and works better.
For example it reacts to the setting/clearing of the particular
syntax-table text properties it's sensitive to, even when
inhibit-modification-hooks is non-nil (which it _always_ is when text
properties are changed).

> > That a high class editor such as Emacs should have
> > problems with such parens is ludicrous and unacceptable.

> FWIW, I (setq open-paren-in-column-0-is-defun-start nil) in my ~/.emacs.

People (e.g. Martin R.) have complained that this makes CC Mode too slow.
If I understand correctly, you work on a super fast machine, not an
ordinary PC.

> > With my change, open_paren_in_column_0_is_defun_start simply vanishes
> > from syntax.c[*], as does `find_defun_start'.

> Using syntax-ppss would give you the same benefit.

> My point is that instead of creating a new kind of "syntax cache" for
> this specific purpose, we'd be better off using the cache we already have.

Maybe, maybe not.  Special purpose things can have an austere simplicity
which their general purpose equivalents lack.  Think of a high quality
screwdriver, and compare it to using the screwdriver piece of a Swiss
Army knife.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/08/2016 10:07 PM, Alan Mackenzie wrote:

>> My point is that instead of creating a new kind of "syntax cache" for
>> this specific purpose, we'd be better off using the cache we already have.
>
> Maybe, maybe not.  Special purpose things can have an austere simplicity
> which their general purpose equivalents lack.  Think of a high quality
> screwdriver, and compare it to using the screwdriver piece of a Swiss
> Army knife.

Consider this: when we implement mixed-major-mode support in Emacs, it's 
probably going to be via an extension to syntax-ppss.

Will an independent comment-cache facility support that automatically, 
or will it need corresponding changes as well?

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Tue, Mar 08, 2016 at 11:22:26PM +0200, Dmitry Gutov wrote:
> On 03/08/2016 10:07 PM, Alan Mackenzie wrote:

> >> My point is that instead of creating a new kind of "syntax cache" for
> >> this specific purpose, we'd be better off using the cache we already have.

> > Maybe, maybe not.  Special purpose things can have an austere simplicity
> > which their general purpose equivalents lack.  Think of a high quality
> > screwdriver, and compare it to using the screwdriver piece of a Swiss
> > Army knife.

> Consider this: when we implement mixed-major-mode support in Emacs, it's 
> probably going to be via an extension to syntax-ppss.

If you have two different modes, one above the other, you will need two
different syntax tables, one above the other.

> Will an independent comment-cache facility support that automatically, 
> or will it need corresponding changes as well?

Once you've solved the symtax table problem, support will be automatic.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> syntax-ppss is a lisp level thing, rather loosely defined.

No, it's pretty precisely defined: it (should) returns the same value as
(parse-partial-sexp (point-min) pos).

> For example it reacts to the setting/clearing of the particular
> syntax-table text properties it's sensitive to, even when
> inhibit-modification-hooks is non-nil (which it _always_ is when text
> properties are changed).

`syntax-ppss' is used all over the place, so if there's a problem with it,
we should fix it.

>> > That a high class editor such as Emacs should have
>> > problems with such parens is ludicrous and unacceptable.
>> FWIW, I (setq open-paren-in-column-0-is-defun-start nil) in my ~/.emacs.
> People (e.g. Martin R.) have complained that this makes CC Mode too slow.

Hmm... I found a prototype patch (not the one I was thinking about, tho)
which uses syntax-ppss, see below.  It appears to give correct results
in my superficial testing, yet I can't notice any performance difference
whether I enable it nor by changing the value of
open-paren-in-column-0-is-defun-start, so I'm not sure I was testing correctly.

> If I understand correctly, you work on a super fast machine, not an
> ordinary PC.

Not really.  I did recently upgrade one of my desktops from a 1.6GHz
E350 to my first >3GHz CPU, but my typical work machine is a Thinkpad T61.

This said, recently I've been using sm-c-mode when editing C code, which
being much less sophisticated is also faster than CC mode.

> Maybe, maybe not.  Special purpose things can have an austere simplicity
> which their general purpose equivalents lack.  Think of a high quality
> screwdriver, and compare it to using the screwdriver piece of a Swiss
> Army knife.

While the patch below is just a proof-of-concept prototype, I can't
believe it could end up anywhere nearly as complex as your
current patch.
So "simplicity" is a question of point of view.


        Stefan


diff --git a/src/syntax.c b/src/syntax.c
index 249d0d5..d0dfa90 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -570,9 +570,6 @@ dec_bytepos (ptrdiff_t bytepos)
 /* Return a defun-start position before POS and not too far before.
    It should be the last one before POS, or nearly the last.
 
-   When open_paren_in_column_0_is_defun_start is nonzero,
-   only the beginning of the buffer is treated as a defun-start.
-
    We record the information about where the scan started
    and what its result was, so that another call in the same area
    can return the same value very quickly.
@@ -597,12 +594,25 @@ find_defun_start (ptrdiff_t pos, ptrdiff_t pos_byte)
       && MODIFF == find_start_modiff)
     return find_start_value;
 
-  if (!open_paren_in_column_0_is_defun_start)
+  if (NILP (Vopen_paren_in_column_0_is_defun_start))
     {
       find_start_value = BEGV;
       find_start_value_byte = BEGV_BYTE;
       goto found;
     }
+  else if (EQ (Vopen_paren_in_column_0_is_defun_start, Qsyntax_ppss))
+    {
+      EMACS_INT modiffs = CHARS_MODIFF;
+      Lisp_Object ppss = call1 (Qsyntax_ppss, make_number (pos));
+      if (modiffs != CHARS_MODIFF)
+	error ("syntax-ppss modified the buffer!");
+      TEMP_SET_PT_BOTH (opoint, opoint_byte);
+      Lisp_Object boc = Fnth (make_number (8), ppss);
+      if (NUMBERP (boc))
+        return XINT (boc);
+      else
+        return pos;
+    }
 
   /* Back up to start of line.  */
   scan_newline (pos, pos_byte, BEGV, BEGV_BYTE, -1, 1);
@@ -863,7 +873,7 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte, ptrdiff_t stop,
 
 	case Sopen:
 	  /* Assume a defun-start point is outside of strings.  */
-	  if (open_paren_in_column_0_is_defun_start
+	  if (EQ (Vopen_paren_in_column_0_is_defun_start, Qt)
 	      && (from == stop
 		  || (temp_byte = dec_bytepos (from_byte),
 		      FETCH_CHAR (temp_byte) == '\n')))
@@ -3647,6 +3657,7 @@ void
 syms_of_syntax (void)
 {
   DEFSYM (Qsyntax_table_p, "syntax-table-p");
+  DEFSYM (Qsyntax_ppss, "syntax-ppss");
 
   staticpro (&Vsyntax_code_object);
 
@@ -3687,10 +3698,10 @@ See the info node `(elisp)Syntax Properties' for a description of the
 	       doc: /* Non-nil means `scan-sexps' treats all multibyte characters as symbol.  */);
   multibyte_syntax_as_symbol = 0;
 
-  DEFVAR_BOOL ("open-paren-in-column-0-is-defun-start",
-	       open_paren_in_column_0_is_defun_start,
+  DEFVAR_LISP ("open-paren-in-column-0-is-defun-start",
+	       Vopen_paren_in_column_0_is_defun_start,
 	       doc: /* Non-nil means an open paren in column 0 denotes the start of a defun.  */);
-  open_paren_in_column_0_is_defun_start = 1;
+  Vopen_paren_in_column_0_is_defun_start = Qsyntax_ppss; /* Qt; */
 
 
   DEFVAR_LISP ("find-word-boundary-function-table",

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> While the patch below is just a proof-of-concept prototype, I can't

Indeed it was and while it seemed to somewhat work it was severely broken.
I've just been testing the patch below, and I'm a bit more confident
that it does what I want.
Testing its performance (by scrolling src/regex.c backward with
page-up) I get:
- 20s with comment-use-syntax-ppss=nil
- 17s with comment-use-syntax-ppss=t
and setting open-paren-in-column-0-is-defun-start has no effect
(i.e. 20s in either case).
[ Don't take those numbers too seriously, this is testing a build with
  full debug checking and such.  The only actual result is that there
  *is* a difference.  ]


        Stefan


diff --git a/src/syntax.c b/src/syntax.c
index 249d0d5..9c5a2a8 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -570,9 +570,6 @@ dec_bytepos (ptrdiff_t bytepos)
 /* Return a defun-start position before POS and not too far before.
    It should be the last one before POS, or nearly the last.
 
-   When open_paren_in_column_0_is_defun_start is nonzero,
-   only the beginning of the buffer is treated as a defun-start.
-
    We record the information about where the scan started
    and what its result was, so that another call in the same area
    can return the same value very quickly.
@@ -597,7 +594,27 @@ find_defun_start (ptrdiff_t pos, ptrdiff_t pos_byte)
       && MODIFF == find_start_modiff)
     return find_start_value;
 
-  if (!open_paren_in_column_0_is_defun_start)
+  if (!NILP (Vcomment_use_syntax_ppss))
+    {
+      EMACS_INT modiffs = CHARS_MODIFF;
+      Lisp_Object ppss = call1 (Qsyntax_ppss, make_number (pos));
+      if (modiffs != CHARS_MODIFF)
+	error ("syntax-ppss modified the buffer!");
+      TEMP_SET_PT_BOTH (opoint, opoint_byte);
+      Lisp_Object boc = Fnth (make_number (8), ppss);
+      if (NUMBERP (boc))
+        {
+          find_start_value = XINT (boc);
+          find_start_value_byte = CHAR_TO_BYTE (find_start_value);
+        }
+      else
+        {
+          find_start_value = pos;
+          find_start_value_byte = pos_byte;
+        }
+      goto found;
+    }
+  if (NILP (Vopen_paren_in_column_0_is_defun_start))
     {
       find_start_value = BEGV;
       find_start_value_byte = BEGV_BYTE;
@@ -863,7 +880,7 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte, ptrdiff_t stop,
 
 	case Sopen:
 	  /* Assume a defun-start point is outside of strings.  */
-	  if (open_paren_in_column_0_is_defun_start
+	  if (EQ (Vopen_paren_in_column_0_is_defun_start, Qt)
 	      && (from == stop
 		  || (temp_byte = dec_bytepos (from_byte),
 		      FETCH_CHAR (temp_byte) == '\n')))
@@ -3647,6 +3664,11 @@ void
 syms_of_syntax (void)
 {
   DEFSYM (Qsyntax_table_p, "syntax-table-p");
+  DEFSYM (Qsyntax_ppss, "syntax-ppss-for-syntax.c");
+  DEFVAR_LISP ("comment-use-syntax-ppss",
+	       Vcomment_use_syntax_ppss,
+ 	       doc: /* Non-nil means `forward-comment' can use `syntax-ppss' internally.  */);
+  Vcomment_use_syntax_ppss = Qnil;
 
   staticpro (&Vsyntax_code_object);
 
@@ -3687,10 +3709,10 @@ See the info node `(elisp)Syntax Properties' for a description of the
 	       doc: /* Non-nil means `scan-sexps' treats all multibyte characters as symbol.  */);
   multibyte_syntax_as_symbol = 0;
 
-  DEFVAR_BOOL ("open-paren-in-column-0-is-defun-start",
-	       open_paren_in_column_0_is_defun_start,
+  DEFVAR_LISP ("open-paren-in-column-0-is-defun-start",
+	       Vopen_paren_in_column_0_is_defun_start,
 	       doc: /* Non-nil means an open paren in column 0 denotes the start of a defun.  */);
-  open_paren_in_column_0_is_defun_start = 1;
+  Vopen_paren_in_column_0_is_defun_start = Qt;
 
 
   DEFVAR_LISP ("find-word-boundary-function-table",

Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello again, Stefan.

On Tue, Mar 08, 2016 at 07:17:17PM -0500, Stefan Monnier wrote:
> > syntax-ppss is a lisp level thing, rather loosely defined.

> No, it's pretty precisely defined: it (should) returns the same value as
> (parse-partial-sexp (point-min) pos).

I note the lack of confidence expressed by the word "should".  Also we
should surely want what the result of a scan starting at position 1 (not
(point-min)) would be.

> > For example it reacts to the setting/clearing of the particular
> > syntax-table text properties it's sensitive to, even when
> > inhibit-modification-hooks is non-nil (which it _always_ is when text
> > properties are changed).

> `syntax-ppss' is used all over the place, so if there's a problem with it,
> we should fix it.

There are several problems with syntax-ppss.  It is not a rigorously
correct function (which is what we would expect from this low level of
the Emacs functionality.)  Looking at the version in the master branch:

Minor problems:
(i) After inserting a `with-syntax-table' near the beginning of
  syntax-ppss, the indentation wasn't redone.
(ii) The cache isn't flushed when a syntax-table text property gets set
  or cleared (see my patch for how to do this).
(iii) The specification of the function states that the returned state
  is equivalent to a scan from (point-min) rather than 1.  This parse
  state isn't very useful, and probably forces callers to widen before
  calling, or at least ensure that (point-min) isn't in a literal.

Major problems:
(i) The cache doesn't appear to be flushed on narrowing or widening the
  buffer.
(ii) No account is taken of possibly starting a parse-partial-sexp from
  after the first character of a two character comment starter or ender.
  OK, this is really a problem with the parser state not recording this
  situation (which we could correct), but failing that, syntax-ppss
  should check for this.  We did talk about this (in connection with a
  CC Mode bug) a few years ago.

Also the function is complicated and difficult to understand.  It seems
to be fairly heavily optimised.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> There are several problems with syntax-ppss.  It is not a rigorously
[...]

Those problems are nitpicks about the current state of the implementation.
Again: it's used all over the place, and your back-comment hack won't
replace syntax-ppss any time soon, so if you have a problem with the
implementation, fix it (or report it as a bug, at the very least) and
everyone else will benefit.

> Also the function is complicated and difficult to understand.

So your solution is to add another parallel cache with its own quirks,
complexity and "difficulty to understand" but which doesn't replace it,
so we end up with both quirks and complexities at the same time?

That's called "NIH syndrome".


        Stefan

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Wed, Mar 09, 2016 at 08:11:33AM -0500, Stefan Monnier wrote:
> > There are several problems with syntax-ppss.  It is not a rigorously
> [...]

> Those problems are nitpicks about the current state of the implementation.

No, these problems are to do with the philosophy of syntax-ppss.  To use
it demands active maintenance of the cache from the program using it.
(forward-comment -1) should just work, regardless of the buffer
restriction, text properties, what have you.

forward-comment should not erase syntax-table text properties.  This is,
however, what syntax-ppss does.  Think about it, syntax-propertize calls
syntax-propertize-extend-region-functions, which itself is likely to
want to do (forward-comment -1).  Whoops!  forward-comment is a
primitive - it shouldn't be dependent on the correct functioning of high
level Lisp code.

> Again: it's used all over the place, and your back-comment hack won't
> replace syntax-ppss any time soon, ....

Who ever said it was meant to?  It lacks many of the capabilities of
syntax-ppss.  It is intended solely to handle comments and strings,
particularly in back_comment.

> ...., so if you have a problem with the implementation, fix it (or
> report it as a bug, at the very least) and everyone else will benefit.

Perhaps you could fix the problems I've identified here and in my last
email.  Especially the one about starting a scan inside a two character
comment marker.  And the one about it calling out to high level Lisp,
leading to the risk of infinite recursive.

You're the maintainer who's familiar with the function, not me.

> > Also the function is complicated and difficult to understand.

> So your solution is to add another parallel cache with its own quirks,
> complexity and "difficulty to understand" but which doesn't replace it,
> so we end up with both quirks and complexities at the same time?

What quirks?  What complexity?  It is so simple that it could be written
in C.  You keep casting aspersions on the new code, but haven't
criticised a single line of it.  Have you even looked at it?

If you had, you'd realise that, being custom designed for back_comment,
it's a much better fit for the requirements than syntax-ppss.

> That's called "NIH syndrome".

syntax-ppss, in its current state, is unsuitable for use in
back_comment.  My new text property cache just works, unobstrusively, in
the background.  It solves the problem (open paren in column zero) it
was intended to solve, and does so elegantly and efficiently.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > My changes might speed up back_comment, but that's not the prime reason
  > for them.  Rather, I want utterly to expunge all the nonsense about
  > parens in column 0.

You might think of this rule as a kludgy rough approximation finding
the defuns in the file.  But it is also a useful way of controlling
what count as defuns.  For instance,

(eval-when-compile
(defun foo ...)

(defun bar...)
)

treats foo and bar as defuns, whereas

(eval-when-compile
  (defun foo ...)

  (defun bar...)
)

treats the whole thing as one defun.

I always want the former.

Without the open-paren-in-column-0 feature, the whole
eval-when-compile form would have to count as one defun, and the user
would have no choice about that.

Thus, before you talk about "fixing" this, would you please
state what behavior you want to implement?

If it makes the right thing happen automatically in some cases
and doesn't make the wrong thing start to happen in other cases,
it could be an improvement.  But let's see it and verify that.


-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/09/2016 06:37 PM, Richard Stallman wrote:

> (eval-when-compile
> (defun foo ...)
>
> (defun bar...)
> )
>
> treats foo and bar as defuns, whereas
>
> (eval-when-compile
>   (defun foo ...)
>
>   (defun bar...)
> )
>
> treats the whole thing as one defun.
>
> I always want the former.

I usually want the former too. _Even if_ the code is formatted like in 
the latter example.

> Without the open-paren-in-column-0 feature, the whole
> eval-when-compile form would have to count as one defun, and the user
> would have no choice about that.

Not if we make beginning-of-defun-raw smarter in emacs-lisp-mode. 
Probably by creating a dedicated beginning-of-defun-function for it.

How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 876 bytes --]

On 03/09/2016 05:49 AM, Alan Mackenzie wrote:
> (ii) No account is taken of possibly starting a parse-partial-sexp from
>   after the first character of a two character comment starter or ender.
>   OK, this is really a problem with the parser state not recording this
>   situation (which we could correct), but failing that, syntax-ppss
>   should check for this.  We did talk about this (in connection with a
>   CC Mode bug) a few years ago.

I keep running into this, actually. There are plenty of examples out there of people checking whether the face at point is font-lock-comment-face or font-lock-string-face or font-lock-doc-face to know whether the current point is in a comment or string. Using (nth 8 (syntax-ppss)) instead mostly works, except that it's nil in comment openers (e.g. in ;; in ELisp) and closers.

Am I missing something?

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Kaushal Modi]

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

> I keep running into this, actually. There are plenty of examples out
there of
> people checking whether the face at point is font-lock-comment-face or
> font-lock-string-face or font-lock-doc-face to know whether the current
point is
> in a comment or string. Using (nth 8 (syntax-ppss)) instead mostly works,
except
> that it's nil in comment openers (e.g. in ;; in ELisp) and closers.

(nth 8 (syntax-ppss)) works fine for me in something like ";; some comment
in an emacs-lisp-mode buffer"; it returns the starting point of the comment.

But I use something like below in my emacs config in one of the functions:

(or (nth 3 (syntax-ppss)) ; string
              (nth 4 (syntax-ppss))) ; comment


PS: I am on the latest emacs-25 build as of today.

--
Kaushal Modi

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

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Richard.

On Wed, Mar 09, 2016 at 11:37:52AM -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. ]]]

>   > My changes might speed up back_comment, but that's not the prime reason
>   > for them.  Rather, I want utterly to expunge all the nonsense about
>   > parens in column 0.

> You might think of this rule as a kludgy rough approximation finding
> the defuns in the file.  But it is also a useful way of controlling
> what count as defuns.  For instance,

> (eval-when-compile
> (defun foo ...)

> (defun bar...)
> )

> treats foo and bar as defuns, whereas

> (eval-when-compile
>   (defun foo ...)

>   (defun bar...)
> )

> treats the whole thing as one defun.

Thanks for the tip!

> I always want the former.

I suppose I do, too.

> Without the open-paren-in-column-0 feature, the whole
> eval-when-compile form would have to count as one defun, and the user
> would have no choice about that.

> Thus, before you talk about "fixing" this, would you please
> state what behavior you want to implement?

That moving backwards over comments and strings will do the Right Thing,
even when those literals contain parentheses in column 0.

Currently, problems happen frequently when somebody inadvertantly puts a
paren in C0, usually in a comment, and this wreaks havoc with
indentation and fontification, etc., because (e.g.) CC Mode handles part
of the comment as though it were code.

The latest problem, found and reported by Paul Eggert as bug #22884, was
in a config.h, and a comment near the start, containing the license
statement, started a line with "(at your option)".  The consequence of
this was editing a comment on Line 1661 took ~10 seconds for the edit to
echo.

Just for information, I've already implemented the behaviour, putting it
into a new git branch called "comment-cache".  It is a matter for
testing and experience whether this change will be merged into the
master branch.  The edit that previously took 10s is now instantaneous.

However, open-paren-in-column-0-is-defun-start still exists in this
version, just that the low level syntactic routines no longer need to
use it.  Trying out C-M-a on your eval-when-compile, it still jumps to
the (defun foo ...).  :-)  However, on setting open-paren-...-start to
nil, the C-M-a moved to the (eval-when-compile).

So, yes, it would seem that open-paren-...-start is still useful, and so
should survive.

> If it makes the right thing happen automatically in some cases
> and doesn't make the wrong thing start to happen in other cases,
> it could be an improvement.  But let's see it and verify that.

Well, to me, it's clearly an improvement (I maintain CC Mode).  But I
only committed the new code yesterday, and so far there's been no
feedback from anybody else who's tried it.

So, when I said "expunge all the nonsense about parens in column zero",
let's take it I meant "from the low level syntactic routines".  :-)

> -- 
> 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.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 344 bytes --]

On 03/09/2016 11:37 AM, Richard Stallman wrote:
> Without the open-paren-in-column-0 feature, the whole
> eval-when-compile form would have to count as one defun, and the user
> would have no choice about that.

But indenting already places defuns that are wrapped in an eval-when-compile in column two. Is there a way to disable that?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 562 bytes --]

On 03/09/2016 12:24 PM, Kaushal Modi wrote:
> (nth 8 (syntax-ppss)) works fine for me in something like ";; some
> comment in an emacs-lisp-mode buffer"; it returns the starting point
> of the comment.

Does it really? For me it returns nil when the point is as shown below:

  <point>;; some comment in an emacs-lisp-mode buffer

In C mode, it returns nil in the following two positions:

  /<point>/ asd
  <point>// asd

As well as these two:

  <point>/* asd */
  /<point>* asd */

In all these cases, the font trick works.

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

>> ...., so if you have a problem with the implementation, fix it (or
>> report it as a bug, at the very least) and everyone else will benefit.
> Perhaps you could fix the problems I've identified here and in my last
> email.

Why would I bother, since I don't even know of any real use case where
those problems are triggered?


        Stefan

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Kaushal Modi]

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

You are correct; it does not work if the point is before the first comment
character.  That makes sense if you visualize syntax-ppss to look at the
chars ONLY up to the point. I never faced this case in my application.

May be have a combination of looking-at comment-start or double quote (")
and syntax-ppss?

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

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 248 bytes --]

On 03/09/2016 02:16 PM, Stefan Monnier wrote:
> Why would I bother, since I don't even know of any real use case where
> those problems are triggered?

Stefan, do you have a suggestion about the "is the point in a comment or string?" issue?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 785 bytes --]

On 03/09/2016 02:19 PM, Kaushal Modi wrote:
> You are correct; it does not work if the point is before the first
> comment character.  That makes sense if you visualize syntax-ppss to
> look at the chars ONLY up to the point. I never faced this case in my
> application.
> 
> May be have a combination of looking-at comment-start or double quote
> (") and syntax-ppss?

Thanks for the suggestion!

Even then, I don't think this works in CC-mode, or in any language that has two-character comment markers: it works in Lisp because the comment marker is a single semicolon. But in OCaml, for example, (* is a comment marker and syntax-ppss doesn't see (<point>* as a comment. This matches your explanation, but I don't think the solution applies to that case.

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Wed, Mar 09, 2016 at 02:16:05PM -0500, Stefan Monnier wrote:
> >> ...., so if you have a problem with the implementation, fix it (or
> >> report it as a bug, at the very least) and everyone else will benefit.
> > Perhaps you could fix the problems I've identified here and in my last
> > email.

> Why would I bother, since I don't even know of any real use case where
> those problems are triggered?

OK, fine.  So we go with my approach to back_comment, then?

By the way, have you had a look at my patch, yet, or even tried it out?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > ... and so far there's been no
 > feedback from anybody else who's tried it.

With emacs -Q and non-optimized builds I've been profiling the following
two primitives

(defun foo ()
   (interactive)
   (while (not (eobp))
     (c-end-of-defun)))

(defun bar ()
   (interactive)
   (while (not (bobp))
     (c-beginning-of-defun)))

to scan the entire buffer of current master's xdisp.c.  The results of
current master with or without your change are approximately the same
‘foo’ taking about 73 seconds, ‘bar’ about 8 minutes, in both cases the
comment-cache version was slightly slower.

However, ‘foo’ is about ten times slower than for a version of Emacs
24.2 which is about two times slower than for a version of Emacs 23.0.
‘bar’ is about 5 times slower than the Emacs 24.2 version which is 3.5
times slower than the one from Emacs 23.0.

So, for example, executing ‘bar’ for my Emacs 23.0 took 24.89 seconds
(0.068 for an average call) versus 464.375 seconds (1.272 average) with
the comment-cache version.  This means that performance has deteriorated
by a factor of 18 over the past years.  This also means that I cannot
use non-optimized builds for my daily work any more.

Maybe someone could try to test these two functions in a similar fashion
so we have at least some numbers to base judgements on.  Sometimes it
seems to me that I'm living in a different world ...

Thanks, martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> Date: Wed, 09 Mar 2016 20:58:23 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> However, ‘foo’ is about ten times slower than for a version of Emacs
> 24.2 which is about two times slower than for a version of Emacs 23.0.
> ‘bar’ is about 5 times slower than the Emacs 24.2 version which is 3.5
> times slower than the one from Emacs 23.0.
> 
> So, for example, executing ‘bar’ for my Emacs 23.0 took 24.89 seconds
> (0.068 for an average call) versus 464.375 seconds (1.272 average) with
> the comment-cache version.  This means that performance has deteriorated
> by a factor of 18 over the past years.  This also means that I cannot
> use non-optimized builds for my daily work any more.

I concur that the current C mode is much slower than it was several
releases ago.  My personal subjective impression is that it gets
slower with every new release, and I suspect that the main reason is
our desire to support more and more C quirks and subtleties.  If this
is true, I'd be glad to go back to a simpler C mode, which sometimes
mis-fontified or even (gasp!) mis-indented an occasional rare
construct, but to get back the speed we enjoyed in the past.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> I concur that the current C mode is much slower than it was several releases
> ago. My personal subjective impression is that it gets slower with every new
> release, and I suspect that the main reason is our desire to support more
> and more C quirks and subtleties. If this is true, I'd be glad to go back to
> a simpler C mode, which sometimes mis-fontified or even (gasp!) mis-indented
> an occasional rare construct, but to get back the speed we enjoyed in the
> past.

I entirely agree, Eli.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

>> >> ...., so if you have a problem with the implementation, fix it (or
>> >> report it as a bug, at the very least) and everyone else will benefit.
>> > Perhaps you could fix the problems I've identified here and in my last
>> > email.
>> Why would I bother, since I don't even know of any real use case where
>> those problems are triggered?
> OK, fine.  So we go with my approach to back_comment, then?

Why?

> By the way, have you had a look at my patch, yet,

Yes, and I didn't like it, hence this thread.

> or even tried it out?

No, I trust you that it works.  I just don't like the way it works.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Andreas Röhler]

On 08.03.2016 19:30, Alan Mackenzie wrote:
> Hello, Stefan.
>
> On Tue, Mar 08, 2016 at 09:19:46AM -0500, Stefan Monnier wrote:
>>>      Apply `comment-depth' text properties when calling `back_comment'.
>> FWIW, I think if you want to speed up back_comment, a simpler approach
>> is to make it use syntax-ppss, which already implements a cache, and
>> will usually already have the cache filled for you.
> My changes might speed up back_comment, but that's not the prime reason
> for them.  Rather, I want utterly to expunge all the nonsense about
> parens in column 0.  That a high class editor such as Emacs should have
> problems with such parens is ludicrous and unacceptable.

Indeed. Thanks making this up again.

Also dropped use of syntax-ppss as not reliable. A cache simply doesn't 
make sense in this circumstance, as any previous insert might have 
changed the state.

See inside stuff like

        ;; Use OLD-PPSS if possible and close enough.

or

    syntax-ppss-toplevel-pos

Syntax must precede such notions like toplevel, can't depend on them.



Andreas

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Wed, Mar 09, 2016 at 04:40:45PM -0500, Stefan Monnier wrote:
> >> >> ...., so if you have a problem with the implementation, fix it (or
> >> >> report it as a bug, at the very least) and everyone else will benefit.
> >> > Perhaps you could fix the problems I've identified here and in my last
> >> > email.
> >> Why would I bother, since I don't even know of any real use case where
> >> those problems are triggered?
> > OK, fine.  So we go with my approach to back_comment, then?

> Why?

Well, given that syntax-ppss is not suitable for fixing back_comment,
the alternatives are not fixing it, substantially amending syntax-ppss,
using my new code, or fixing it some other way.  I really think it
should be fixed.  You don't want syntax-ppss to be changed.  Do you have
an idea for an "other way"?

> > By the way, have you had a look at my patch, yet,

> Yes, and I didn't like it, hence this thread.

Is there anything you don't like about it besides the partial
duplication of the syntax-ppss functionality?  If so, what?

> > or even tried it out?

> No, I trust you that it works.  I just don't like the way it works.

Thanks!

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

>> > There are several problems with syntax-ppss.  It is not a rigorously
>> Those problems are nitpicks about the current state of the implementation.
> No, these problems are to do with the philosophy of syntax-ppss.

Sorry, but "didn't reindent after adding with-syntax-table" is a very
far cry from a "philosophical" problem.

> To use it demands active maintenance of the cache from the program
> using it.

No, these are mere current implementation choices.

> (forward-comment -1) should just work, regardless of the buffer
> restriction, text properties, what have you.

By your own admission, your comment cache also has to make some
assumptions about what the user does (e.g. changing syntax-table, or
`category' properties, or modifying buffer-text within
inhibit-modification-hooks).

Those assumptions aren't exactly the same as the ones of syntax-ppss
simply because the two use different code, but none of the differences
are fundamental in any sense and it would be very easy to change your
code or syntax-ppss's to align one with the other.

> forward-comment should not erase syntax-table text properties.  This is,
> however, what syntax-ppss does.  Think about it, syntax-propertize calls
> syntax-propertize-extend-region-functions, which itself is likely to
> want to do (forward-comment -1).

Maybe it's "likely" in your book, but I haven't bumped into this is my
tests yet.

> Whoops!  forward-comment is a primitive - it shouldn't be dependent on
> the correct functioning of high level Lisp code.

forward-comment is not more primitive than syntax-ppss.

The fact that one is written in C and the other in Elisp is irrelevant
to their classification as "high-level vs primitive".

> back_comment.  My new text property cache just works, unobstrusively, in

Your "just works" has just the same kinds of strings attached as
syntax-ppss's.


        Stefan

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Stefan Monnier]

>> (nth 8 (syntax-ppss)) works fine for me in something like ";; some
>> comment in an emacs-lisp-mode buffer"; it returns the starting point
>> of the comment.
> Does it really? For me it returns nil when the point is as shown below:
>   <point>;; some comment in an emacs-lisp-mode buffer

That's correct, because point is *not* inside the comment.

> In all these cases, the font trick works.

Here it's because `get-text-property' returns the property of the
character after point, and indeed this is ";" which is in the comment.

IOW they're both correct because they don't ask the same thing.

> In C mode, it returns nil in the following two positions:
>
>   /<point>/ asd
>   <point>// asd
>
> As well as these two:
>
>   <point>/* asd */
>   /<point>* asd */

Indeed, this is a current problem with parse-partial-sexp (and hence
syntax-ppss) and is directly related to what Alan was referring to.

Given the way parse-partial-sexp works (i.e. it can only look at the
text that precedes), even if we fix it, parse-partial-sexp wouldn't be
able to tell you that <point> is inside a comment.  But it could tell
you that it's in the middle of something that could turn out to be
a comment.


        Stefan

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> Well, given that syntax-ppss is not suitable for fixing back_comment,

On the contrary I think it's perfectly suitable.

>> > By the way, have you had a look at my patch, yet,
>> Yes, and I didn't like it, hence this thread.
> Is there anything you don't like about it besides the partial
> duplication of the syntax-ppss functionality?  If so, what?

Obviously, I prefer a separate data structure over text-properties, but
that's just a mild preference.  The problem is in the duplication.


        Stefan

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1514 bytes --]

On 03/10/2016 09:28 AM, Stefan Monnier wrote:
>>> (nth 8 (syntax-ppss)) works fine for me in something like ";; some
>>> comment in an emacs-lisp-mode buffer"; it returns the starting point
>>> of the comment.
>> Does it really? For me it returns nil when the point is as shown below:
>>   <point>;; some comment in an emacs-lisp-mode buffer
> 
> That's correct, because point is *not* inside the comment.
> 
>> In all these cases, the font trick works.
> 
> Here it's because `get-text-property' returns the property of the
> character after point, and indeed this is ";" which is in the comment.
> 
> IOW they're both correct because they don't ask the same thing.
> 
>> In C mode, it returns nil in the following two positions:
>>
>>   /<point>/ asd
>>   <point>// asd
>>
>> As well as these two:
>>
>>   <point>/* asd */
>>   /<point>* asd */
> 
> Indeed, this is a current problem with parse-partial-sexp (and hence
> syntax-ppss) and is directly related to what Alan was referring to.
> 
> Given the way parse-partial-sexp works (i.e. it can only look at the
> text that precedes), even if we fix it, parse-partial-sexp wouldn't be
> able to tell you that <point> is inside a comment.  But it could tell
> you that it's in the middle of something that could turn out to be
> a comment.

Thanks for the clarification. What's the recommended way to check if the point is in a comment, then? I'm thinking for example of adjusting fill-nobreak-predicate, or similar things.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Stefan Monnier]

> Thanks for the clarification. What's the recommended way to check if the
> point is in a comment, then? I'm thinking for example of adjusting
> fill-nobreak-predicate, or similar things.

The recommended way is

   (nth 8 (syntax-ppss))

Whether "/<point>*" is inside the comment or not is very debatable, and
luckily it rarely matters.  So if/when it matters and syntax-ppss's
answer isn't the one we need, then we can discuss the better replacement
for that specific case, but the specifics will matter then.


        Stefan

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 10, 2016 at 09:52:49AM -0500, Stefan Monnier wrote:
> > Well, given that syntax-ppss is not suitable for fixing back_comment,

> On the contrary I think it's perfectly suitable.

You're wrong.

I pointed out several reasons for this in two posts yesterday around
lunch time (European time).  You've failed to respond in detail to the
most serious points, beyond saying you haven't encountered them in your
testing (so far).

> >> > By the way, have you had a look at my patch, yet,
> >> Yes, and I didn't like it, hence this thread.
> > Is there anything you don't like about it besides the partial
> > duplication of the syntax-ppss functionality?  If so, what?

> Obviously, I prefer a separate data structure over text-properties, but
> that's just a mild preference.  The problem is in the duplication.

If syntax-ppss were to be amended to be rigorously correct, and hence
usable from back_comment, I would be delighted.

But while we have the prospect of infinite recursion, and the prospect
of the cache being useless (because point-min is inside a comment or
string), the prospects for using syntax-ppss in back_comment don't look
bright.

Can you fix these (and other) faults of syntax_ppss?

Pending such a fix, I take it you wouldn't object to me merging the
comment-cache branch with master?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

>> > Well, given that syntax-ppss is not suitable for fixing back_comment,
>> On the contrary I think it's perfectly suitable.
> You're wrong.

I assume this is an opinion ;-)

> I pointed out several reasons for this in two posts yesterday around
> lunch time (European time).

None of these point to it being unsuitable.  At best they indicate that
it may require some tweaks/bugfixes.

> You've failed to respond in detail to the most serious points, beyond
> saying you haven't encountered them in your testing (so far).

What else do you expect me to say?  In most of those cases, it's
difficult to know what the right/best fix would be before encountering
a concrete problem case.
So I'm not going to "fix" those problems in advance.

> If syntax-ppss were to be amended to be rigorously correct, and hence
> usable from back_comment, I would be delighted.

There's no such thing as rigorously correct.  Your code is not
rigorously correct either, and for the exact same underlying reasons:
the best behavior either depends on the underlying intent or is assumed
to be too costly to be worth the trouble.

Your code faces the exact same problems as syntax-ppss, because they are
fairly fundamental.  You took extra measures to address some of those
problems, and I took extra measures to address some others, that's all,
so the weaknesses of our respective solutions aren't exactly the same.

> But while we have the prospect of infinite recursion,

Trivial to resolve, in my experience.

> and the prospect of the cache being useless (because point-min is
> inside a comment or string),

Trivial to resolve as well.

> the prospects for using syntax-ppss in back_comment don't look bright.

Who's talking about prospects.  I have sent a patch and I'm using it
right now.

> Can you fix these (and other) faults of syntax_ppss?

Go right ahead, if you want to fix them.  Personally I won't bother
until I get a concrete example exposing those problems, so I can decide
what's the best way to fix it.

> Pending such a fix, I take it you wouldn't object to me merging the
> comment-cache branch with master?

I definitely would.


        Stefan

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1126 bytes --]

On 03/10/2016 10:20 AM, Stefan Monnier wrote:
>> Thanks for the clarification. What's the recommended way to check if the
>> point is in a comment, then? I'm thinking for example of adjusting
>> fill-nobreak-predicate, or similar things.
> 
> The recommended way is
> 
>    (nth 8 (syntax-ppss))
> 
> Whether "/<point>*" is inside the comment or not is very debatable, and
> luckily it rarely matters.  So if/when it matters and syntax-ppss's
> answer isn't the one we need, then we can discuss the better replacement
> for that specific case, but the specifics will matter then.

Cool, thanks Stefan, As long as this behaviour is consistent with the rest of Emacs, that's fine by me.

Here's one case where it matters: in tuareg-mode (for OCaml), with the following code:

  (<point>* comment *)

The call to (prettify-symbols-default-compose-p (point) (1+ (point)) nil) returns t.
On the other hand, the same call with the point as follows returns nil:

  (*<point> comment *)

This makes it unreliable to prettify * to × in tuareg mode.
I'll report a bug for prettify-symbols-mode.

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 10, 2016 at 11:45:10AM -0500, Stefan Monnier wrote:

> > the prospects for using syntax-ppss in back_comment don't look bright.

> Who's talking about prospects.  I have sent a patch and I'm using it
> right now.

Would you please tell me where you've sent it.  Then I can try to break
it, thus assissting you in any necessary debugging.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> Would you please tell me where you've sent it.

In one of the messages of this long thread.
Here it is again.

> Then I can try to break it, thus assissting you in any
> necessary debugging.

Please do.  I expected a fair bit of breakage but haven't seen any yet,
so it's still in a very "primitive/naive" shape.


        Stefan


diff --git a/src/syntax.c b/src/syntax.c
index 249d0d5..9c5a2a8 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -570,9 +570,6 @@ dec_bytepos (ptrdiff_t bytepos)
 /* Return a defun-start position before POS and not too far before.
    It should be the last one before POS, or nearly the last.
 
-   When open_paren_in_column_0_is_defun_start is nonzero,
-   only the beginning of the buffer is treated as a defun-start.
-
    We record the information about where the scan started
    and what its result was, so that another call in the same area
    can return the same value very quickly.
@@ -597,7 +594,27 @@ find_defun_start (ptrdiff_t pos, ptrdiff_t pos_byte)
       && MODIFF == find_start_modiff)
     return find_start_value;
 
-  if (!open_paren_in_column_0_is_defun_start)
+  if (!NILP (Vcomment_use_syntax_ppss))
+    {
+      EMACS_INT modiffs = CHARS_MODIFF;
+      Lisp_Object ppss = call1 (Qsyntax_ppss, make_number (pos));
+      if (modiffs != CHARS_MODIFF)
+	error ("syntax-ppss modified the buffer!");
+      TEMP_SET_PT_BOTH (opoint, opoint_byte);
+      Lisp_Object boc = Fnth (make_number (8), ppss);
+      if (NUMBERP (boc))
+        {
+          find_start_value = XINT (boc);
+          find_start_value_byte = CHAR_TO_BYTE (find_start_value);
+        }
+      else
+        {
+          find_start_value = pos;
+          find_start_value_byte = pos_byte;
+        }
+      goto found;
+    }
+  if (NILP (Vopen_paren_in_column_0_is_defun_start))
     {
       find_start_value = BEGV;
       find_start_value_byte = BEGV_BYTE;
@@ -863,7 +880,7 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte, ptrdiff_t stop,
 
 	case Sopen:
 	  /* Assume a defun-start point is outside of strings.  */
-	  if (open_paren_in_column_0_is_defun_start
+	  if (EQ (Vopen_paren_in_column_0_is_defun_start, Qt)
 	      && (from == stop
 		  || (temp_byte = dec_bytepos (from_byte),
 		      FETCH_CHAR (temp_byte) == '\n')))
@@ -3647,6 +3664,11 @@ void
 syms_of_syntax (void)
 {
   DEFSYM (Qsyntax_table_p, "syntax-table-p");
+  DEFSYM (Qsyntax_ppss, "syntax-ppss");
+  DEFVAR_LISP ("comment-use-syntax-ppss",
+	       Vcomment_use_syntax_ppss,
+ 	       doc: /* Non-nil means `forward-comment' can use `syntax-ppss' internally.  */);
+  Vcomment_use_syntax_ppss = Qt;
 
   staticpro (&Vsyntax_code_object);
 
@@ -3687,10 +3709,10 @@ See the info node `(elisp)Syntax Properties' for a description of the
 	       doc: /* Non-nil means `scan-sexps' treats all multibyte characters as symbol.  */);
   multibyte_syntax_as_symbol = 0;
 
-  DEFVAR_BOOL ("open-paren-in-column-0-is-defun-start",
-	       open_paren_in_column_0_is_defun_start,
+  DEFVAR_LISP ("open-paren-in-column-0-is-defun-start",
+	       Vopen_paren_in_column_0_is_defun_start,
 	       doc: /* Non-nil means an open paren in column 0 denotes the start of a defun.  */);
-  open_paren_in_column_0_is_defun_start = 1;
+  Vopen_paren_in_column_0_is_defun_start = Qt;
 
 
   DEFVAR_LISP ("find-word-boundary-function-table",

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 10, 2016 at 12:34:13PM -0500, Stefan Monnier wrote:
> > Would you please tell me where you've sent it.

> In one of the messages of this long thread.
> Here it is again.

OK, thanks.

> > Then I can try to break it, thus assissting you in any
> > necessary debugging.

> Please do.  I expected a fair bit of breakage but haven't seen any yet,
> so it's still in a very "primitive/naive" shape.

Visit xdisp.c.  Run the following function.  It breaks:

(defun test-break-0 ()
  (interactive)
  (goto-char (point-min))
  (or (forward-comment 2)
      (error "(forward-comment 2) failed"))
  (or (forward-comment -2)
      (error "(forward-comment -2) failed"))
  (message "(forward-comment -2) succeeded"))

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > Not if we make beginning-of-defun-raw smarter in emacs-lisp-mode. 
  > Probably by creating a dedicated beginning-of-defun-function for it.

If you make a concrete proposal for the new behavior, we can think about it.

-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > > Thus, before you talk about "fixing" this, would you please
  > > state what behavior you want to implement?

  > That moving backwards over comments and strings will do the Right Thing,
  > even when those literals contain parentheses in column 0.

We are miscommunicating.  That's your desideratum.  Ok.  But what I'm
asking for is a complete clear proposal for the change in behavior in
Emacs.

For instance, what would C-M-a do?  What would C-M-e do?

-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > But indenting already places defuns that are wrapped in an
  > eval-when-compile in column two. Is there a way to disable that?

That will happen if you type TAB on the (defun lines -- so the
solution is, don't do that.



-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 761 bytes --]

On 03/10/2016 04:20 PM, 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. ]]]
> 
>   > But indenting already places defuns that are wrapped in an
>   > eval-when-compile in column two. Is there a way to disable that?
> 
> That will happen if you type TAB on the (defun lines -- so the
> solution is, don't do that.

That won't be enough: I use a package by Arthur that aggressively keeps everything indented (which is great), and many of the projects that I contribute to reject patches to Elisp code that do not follow the default indentation.

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello again, Richard.

On Thu, Mar 10, 2016 at 04:20:28PM -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. ]]]

>   > > Thus, before you talk about "fixing" this, would you please
>   > > state what behavior you want to implement?

>   > That moving backwards over comments and strings will do the Right Thing,
>   > even when those literals contain parentheses in column 0.

> We are miscommunicating.  That's your desideratum.  Ok.  But what I'm
> asking for is a complete clear proposal for the change in behavior in
> Emacs.

There should be no change in Emacs's visible behaviour, beyond
correction of some bugs.

> For instance, what would C-M-a do?  What would C-M-e do?

They would do the same as they currently do - except where an
inadvertant paren in C0 currently prevents the correct behaviour.

> -- 
> 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.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> Visit xdisp.c.  Run the following function.  It breaks:

> (defun test-break-0 ()
>   (interactive)
>   (goto-char (point-min))
>   (or (forward-comment 2)
>       (error "(forward-comment 2) failed"))
>   (or (forward-comment -2)
>       (error "(forward-comment -2) failed"))
>   (message "(forward-comment -2) succeeded"))

Ah, sorry, the patch indeed is incomplete in that it doesn't make Emacs
ignore open-paren-in-column-0-is-defun-start.

So you can fix the above problem with

    (setq open-paren-in-column-0-is-defun-start nil)

or replace my previous patch with this one below, which is almost
identical to the previous one, tho a bit cleaner and with a fix to make
comment-use-syntax-ppss override open-paren-in-column-0-is-defun-start,
as it should.


        Stefan


diff --git a/src/syntax.c b/src/syntax.c
index 249d0d5..f2268da 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -597,6 +597,26 @@ find_defun_start (ptrdiff_t pos, ptrdiff_t pos_byte)
       && MODIFF == find_start_modiff)
     return find_start_value;
 
+  if (!NILP (Vcomment_use_syntax_ppss))
+    {
+      EMACS_INT modiffs = CHARS_MODIFF;
+      Lisp_Object ppss = call1 (Qsyntax_ppss, make_number (pos));
+      if (modiffs != CHARS_MODIFF)
+	error ("syntax-ppss modified the buffer!");
+      TEMP_SET_PT_BOTH (opoint, opoint_byte);
+      Lisp_Object boc = Fnth (make_number (8), ppss);
+      if (NUMBERP (boc))
+        {
+          find_start_value = XINT (boc);
+          find_start_value_byte = CHAR_TO_BYTE (find_start_value);
+        }
+      else
+        {
+          find_start_value = pos;
+          find_start_value_byte = pos_byte;
+        }
+      goto found;
+    }
   if (!open_paren_in_column_0_is_defun_start)
     {
       find_start_value = BEGV;
@@ -864,6 +884,7 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte, ptrdiff_t stop,
 	case Sopen:
 	  /* Assume a defun-start point is outside of strings.  */
 	  if (open_paren_in_column_0_is_defun_start
+              && NILP (Vcomment_use_syntax_ppss))
 	      && (from == stop
 		  || (temp_byte = dec_bytepos (from_byte),
 		      FETCH_CHAR (temp_byte) == '\n')))
@@ -3647,6 +3668,11 @@ void
 syms_of_syntax (void)
 {
   DEFSYM (Qsyntax_table_p, "syntax-table-p");
+  DEFSYM (Qsyntax_ppss, "syntax-ppss-for-syntax.c");
+  DEFVAR_LISP ("comment-use-syntax-ppss",
+	       Vcomment_use_syntax_ppss,
+ 	       doc: /* Non-nil means `forward-comment' can use `syntax-ppss' internally.  */);
+  Vcomment_use_syntax_ppss = Qt;
 
   staticpro (&Vsyntax_code_object);
 

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[John Wiegley]

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

> Please do. I expected a fair bit of breakage but haven't seen any yet, so
> it's still in a very "primitive/naive" shape.

Stefan, are you saying that this is true of code you installed in emacs-25?
If it is still primitive/naive, should we be releasing it in 25.1?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/10/2016 11:20 PM, Richard Stallman wrote:

> If you make a concrete proposal for the new behavior, we can think about it.

There's nothing particularly hard about it--we have 
beginning-of-defun-function's implemented for much more complex 
functions than Elisp.

Here's a quick sketch:

(defun elisp-beginning-of-defun (&optional count)
   (setq count (or count 1))
   (dotimes (i count)
     (while (and (re-search-backward "(\\(?:cl-\\)?def" nil t)
                 (elisp--form-quoted-p (point))))))

It fails on forms such as (define-key ...), but it's easy to also check 
all positions in (nth 9 (syntax-ppss)), and only pick the outermost 
defun-looking form. We can also make the regexp stricter, to only accept 
[cl-]defun, defmacro, defvar and defconst.

Anyway, like Alan mentioned, this caching proposal is actually 
orthogonal to the use of open-paren-in-column-0-is-defun-start in 
beginning-of-defun in emacs-lisp-mode. We could still keep that 
application, if we so preferred.

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 495 bytes --]

On 03/10/2016 06:31 PM, John Wiegley wrote:
>>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
>> Please do. I expected a fair bit of breakage but haven't seen any yet, so
>> it's still in a very "primitive/naive" shape.
> 
> Stefan, are you saying that this is true of code you installed in emacs-25?
> If it is still primitive/naive, should we be releasing it in 25.1?

No, I think Stefan was talking about the patch that he attached to a previous email in this thread :)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

>> Please do. I expected a fair bit of breakage but haven't seen any yet, so
>> it's still in a very "primitive/naive" shape.
> Stefan, are you saying that this is true of code you installed in emacs-25?

No, it's in my set of local hacks and in an email message I sent
recently (as a patch against master).


        Stefan "actually, I take that back: yes, this is true of a fair
                bit of the code I installed in Emacs over the years.
                But it's probably too late to remove it ;-)"

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Andreas Röhler]

On 10.03.2016 14:01, Alan Mackenzie wrote:
> Hello, Stefan.
>
> On Wed, Mar 09, 2016 at 04:40:45PM -0500, Stefan Monnier wrote:
>>>>>> ...., so if you have a problem with the implementation, fix it (or
>>>>>> report it as a bug, at the very least) and everyone else will benefit.
>>>>> Perhaps you could fix the problems I've identified here and in my last
>>>>> email.
>>>> Why would I bother, since I don't even know of any real use case where
>>>> those problems are triggered?
>>> OK, fine.  So we go with my approach to back_comment, then?
>> Why?
> Well, given that syntax-ppss is not suitable for fixing back_comment,
> the alternatives are not fixing it, substantially amending syntax-ppss,
> using my new code, or fixing it some other way.  I really think it
> should be fixed.  You don't want syntax-ppss to be changed.  Do you have
> an idea for an "other way"?
>
>

There is neither a need nor a way to fix syntax-ppss : Determining 
syntax must check from beginning of buffer, as parse-partial-sexp does. 
parse-partial-sexp is in C, it's fast.

Best,

Andreas

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Andreas.

On Fri, Mar 11, 2016 at 08:27:17AM +0100, Andreas Röhler wrote:

> On 10.03.2016 14:01, Alan Mackenzie wrote:
> > Hello, Stefan.

> > On Wed, Mar 09, 2016 at 04:40:45PM -0500, Stefan Monnier wrote:
> >>>>>> ...., so if you have a problem with the implementation, fix it (or
> >>>>>> report it as a bug, at the very least) and everyone else will benefit.
> >>>>> Perhaps you could fix the problems I've identified here and in my last
> >>>>> email.
> >>>> Why would I bother, since I don't even know of any real use case where
> >>>> those problems are triggered?
> >>> OK, fine.  So we go with my approach to back_comment, then?
> >> Why?
> > Well, given that syntax-ppss is not suitable for fixing back_comment,
> > the alternatives are not fixing it, substantially amending syntax-ppss,
> > using my new code, or fixing it some other way.  I really think it
> > should be fixed.  You don't want syntax-ppss to be changed.  Do you have
> > an idea for an "other way"?

> There is neither a need nor a way to fix syntax-ppss : Determining 
> syntax must check from beginning of buffer, as parse-partial-sexp does. 
> parse-partial-sexp is in C, it's fast.

parse-partial-sexp starts scanning from where you ask it to, optionally
starting with a parse state returned by a previous invocation.  Although
it's fast, it's not all that fast when repeatedly run over large
portions of a large buffer.  This is what syntax-ppss was intended to
solve.

The position syntax-ppss notionally starts scanning from is
indeterminate.  Sometimes it's the beginning of buffer, sometimes it's
the beginning of the visible portion of the buffer, sometimes it's some
former beginning of visible portion.

Its documentation is confused, too.  The entry in the elisp manual says
"beginning of buffer".  The doc string string says "point-min".

I disagree about the need to fix syntax-ppss - medium level functions
should be rigorous and determinate.

> Best,

> Andreas

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello Dmitry.

On Fri, Mar 11, 2016 at 02:26:25AM +0200, Dmitry Gutov wrote:
> On 03/10/2016 11:20 PM, Richard Stallman wrote:

> > If you make a concrete proposal for the new behavior, we can think about it.

> There's nothing particularly hard about it--we have 
> beginning-of-defun-function's implemented for much more complex 
> functions than Elisp.

> Here's a quick sketch:

> (defun elisp-beginning-of-defun (&optional count)
>    (setq count (or count 1))
>    (dotimes (i count)
>      (while (and (re-search-backward "(\\(?:cl-\\)?def" nil t)
>                  (elisp--form-quoted-p (point))))))

> It fails on forms such as (define-key ...), but it's easy to also check 
> all positions in (nth 9 (syntax-ppss)), and only pick the outermost 
> defun-looking form. We can also make the regexp stricter, to only accept 
> [cl-]defun, defmacro, defvar and defconst.

Please don't think of using (nth 9 (snytax-ppss)) in such a high level
function.  It's strictly internal stuff used for continuing a parse in
the middle of something.  If you do really want to use it, we should
consider making that element fully defined at the user level (and
possibly stating any further elements are "internal").

I think a better way to do this would be to search for "containing
functions", such as `eval-when-compile' at the top level, and designate
everything inside them as "top level".

> Anyway, like Alan mentioned, this caching proposal is actually 
> orthogonal to the use of open-paren-in-column-0-is-defun-start in 
> beginning-of-defun in emacs-lisp-mode. We could still keep that 
> application, if we so preferred.

Indeed.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Dmitry Gutov]

On 03/11/2016 02:08 PM, Alan Mackenzie wrote:

> I disagree about the need to fix syntax-ppss - medium level functions
> should be rigorous and determinate.

This sentence makes no sense to me.

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> @@ -3647,6 +3668,11 @@ void
>  syms_of_syntax (void)
>  {
>    DEFSYM (Qsyntax_table_p, "syntax-table-p");
> +  DEFSYM (Qsyntax_ppss, "syntax-ppss-for-syntax.c");
                            ^^^^^^^^^^^^^^^^^^^^^^^^
Sorry, left-over debugging.  This should either be just `syntax-ppss',
or you need to (defalias 'syntax-ppss-for-syntax.c #'syntax-ppss).


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> Please don't think of using (nth 9 (snytax-ppss)) in such a high level
> function.  It's strictly internal stuff used for continuing a parse in
> the middle of something.  If you do really want to use it, we should
> consider making that element fully defined at the user level (and
> possibly stating any further elements are "internal").

We should document it, indeed.  It's been "stable" for decades and is
already used at various places in Elisp packages, not all of them
qualifying as "low-level".


        Stefan

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Dmitry.

On Fri, Mar 11, 2016 at 02:30:35PM +0200, Dmitry Gutov wrote:
> On 03/11/2016 02:08 PM, Alan Mackenzie wrote:

> > I disagree about the need to fix syntax-ppss - medium level functions
> > should be rigorous and determinate.

> This sentence makes no sense to me.

syntax-ppss is a "medum level function".  It is neither a primitive nor
a high level function.

Such a function should be "determinate", in that it should always do the
same thing given the same arguments and external conditions.

Such a function should be "rigorous", in that it should do the Right
Thing in any circumstances.

I'm saying there is a need to fix syntax-ppss, since it is neither
determinate nor rigorous.

Does that make sense?

-- 
Alan Mackenzie (Nuremberg, Germany).

RE: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Drew Adams]

> > Please don't think of using (nth 9 (snytax-ppss)) in such a high level
> > function.  It's strictly internal stuff used for continuing a parse in
> > the middle of something.  If you do really want to use it, we should
> > consider making that element fully defined at the user level (and
> > possibly stating any further elements are "internal").
> 
> We should document it, indeed.  It's been "stable" for decades and is
> already used at various places in Elisp packages, not all of them
> qualifying as "low-level".

See bug #22890 about defining `file-attributes' accessor macros, to
make code that would otherwise use (nth N SOMETHING) more readable.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22890

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Martin.

On Wed, Mar 09, 2016 at 08:58:23PM +0100, martin rudalics wrote:
>  > ... and so far there's been no
>  > feedback from anybody else who's tried it.

> With emacs -Q and non-optimized builds I've been profiling the following
> two primitives

> (defun foo ()
>    (interactive)
>    (while (not (eobp))
>      (c-end-of-defun)))

> (defun bar ()
>    (interactive)
>    (while (not (bobp))
>      (c-beginning-of-defun)))

> to scan the entire buffer of current master's xdisp.c.  The results of
> current master with or without your change are approximately the same
> ‘foo’ taking about 73 seconds, ‘bar’ about 8 minutes, in both cases the
> comment-cache version was slightly slower.

Thanks for taking the trouble to try this.  I get the following timings:

            without comment-cache           with comment-cache
foo                 15.64s                         7.89s
bar                 96.53s                        89.93s

So my results are the opposite from yours.  In particular for `foo', the
comment cache gave a dramatic speedup.

> However, ‘foo’ is about ten times slower than for a version of Emacs
> 24.2 which is about two times slower than for a version of Emacs 23.0.
> ‘bar’ is about 5 times slower than the Emacs 24.2 version which is 3.5
> times slower than the one from Emacs 23.0.

> So, for example, executing ‘bar’ for my Emacs 23.0 took 24.89 seconds
> (0.068 for an average call) versus 464.375 seconds (1.272 average) with
> the comment-cache version.  This means that performance has deteriorated
> by a factor of 18 over the past years.  This also means that I cannot
> use non-optimized builds for my daily work any more.

Sorry.  It seems this is a "popular" observation.  I'm going to have to
spend some time working out what can be taken out of CC Mode, and at
what cost in correctness.  I can't promise much progress quickly, but I
promise to put work into it soon.

> Maybe someone could try to test these two functions in a similar fashion
> so we have at least some numbers to base judgements on.  Sometimes it
> seems to me that I'm living in a different world ...

> Thanks, martin

P.S. I've just run `bar' with elp enabled (all `c-..' functions plus
`parse-partial-sexp' and `scan-lists' being instrumented), and I get the
following:

c-beginning-of-defun                           365         97.659041618  0.2675590181
c-beginning-of-decl-1                          1071        95.573583641  0.0892377064
c-syntactic-skip-backward                      2239        91.440043437  0.0408396799
c-in-knr-argdecl                               1071        90.988073058  0.0849561839
parse-partial-sexp                             392328      86.160691713  0.0002196139

c-beginning-of-statement-1                     1071        4.3989913499  0.0041073682
c-where-wrt-brace-construct                    365         4.0776264500  0.0111715793
c-in-function-trailer-p                        365         2.4348031740  0.0066706936
scan-sexps                                     12402       2.4236885029  0.0001954272

, so it seems fairly obvious now where the saving must come from.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Dmitry Gutov]

On 03/11/2016 03:04 PM, Alan Mackenzie wrote:

> syntax-ppss is a "medum level function".  It is neither a primitive nor
> a high level function.
>
> Such a function should be "determinate", in that it should always do the
> same thing given the same arguments and external conditions.
>
> Such a function should be "rigorous", in that it should do the Right
> Thing in any circumstances.
>
> I'm saying there is a need to fix syntax-ppss, since it is neither
> determinate nor rigorous.
>
> Does that make sense?

Oh. Yes, of course. You were replying to Andreas who stated that there's 
no such need. Sorry for the noise.

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Thu, Mar 10, 2016 at 06:10:58PM -0500, Stefan Monnier wrote:

[ .... ]

> or replace my previous patch with this one below, which is almost
> identical to the previous one, tho a bit cleaner and with a fix to make
> comment-use-syntax-ppss override open-paren-in-column-0-is-defun-start,
> as it should.

OK.  It wouldn't compile, so I had to correct it a little bit.

Anyhow, what is going on at the indicated lines, where the input values
of the end of comment are simply returned as the beginning of defun?

Perhaps more seriously, the scheme won't work if point-min is inside a
string.  In that case, strings and non-strings get swapped.  syntax-ppss
will return what it thinks is the beginning of the string enclosing POS,
but which in reality is the end " of the previous actual string.  This
could easily be inside an actual comment:

............"   "...." /*  "...."   */   /*  */   /*               */
|                               |                           |
point-min           return value of syntax-ppss            pos

This is not a valid position to return as a safe place.

> diff --git a/src/syntax.c b/src/syntax.c
> index 249d0d5..f2268da 100644
> --- a/src/syntax.c
> +++ b/src/syntax.c
> @@ -597,6 +597,26 @@ find_defun_start (ptrdiff_t pos, ptrdiff_t pos_byte)
>        && MODIFF == find_start_modiff)
>      return find_start_value;
 
> +  if (!NILP (Vcomment_use_syntax_ppss))
> +    {
> +      EMACS_INT modiffs = CHARS_MODIFF;
> +      Lisp_Object ppss = call1 (Qsyntax_ppss, make_number (pos));
> +      if (modiffs != CHARS_MODIFF)
> +	error ("syntax-ppss modified the buffer!");
> +      TEMP_SET_PT_BOTH (opoint, opoint_byte);
> +      Lisp_Object boc = Fnth (make_number (8), ppss);
> +      if (NUMBERP (boc))
> +        {
> +          find_start_value = XINT (boc);
> +          find_start_value_byte = CHAR_TO_BYTE (find_start_value);
> +        }
> +      else
> +        {
> +          find_start_value = pos;              <=====================
> +          find_start_value_byte = pos_byte;    <=====================
> +        }
> +      goto found;
> +    }
>    if (!open_paren_in_column_0_is_defun_start)
>      {
>        find_start_value = BEGV;
> @@ -864,6 +884,7 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte, ptrdiff_t stop,
>  	case Sopen:
>  	  /* Assume a defun-start point is outside of strings.  */
>  	  if (open_paren_in_column_0_is_defun_start
> +              && NILP (Vcomment_use_syntax_ppss))
>  	      && (from == stop
>  		  || (temp_byte = dec_bytepos (from_byte),
>  		      FETCH_CHAR (temp_byte) == '\n')))
> @@ -3647,6 +3668,11 @@ void
>  syms_of_syntax (void)
>  {
>    DEFSYM (Qsyntax_table_p, "syntax-table-p");
> +  DEFSYM (Qsyntax_ppss, "syntax-ppss-for-syntax.c");
> +  DEFVAR_LISP ("comment-use-syntax-ppss",
> +	       Vcomment_use_syntax_ppss,
> + 	       doc: /* Non-nil means `forward-comment' can use `syntax-ppss' internally.  */);
> +  Vcomment_use_syntax_ppss = Qt;
 
>    staticpro (&Vsyntax_code_object);
 

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> OK.  It wouldn't compile, so I had to correct it a little bit.

Hmm... I've seen your arrows but they weren't sufficient to make me see
what it was you needed to change.

> ............"   "...." /*  "...."   */   /*  */   /*               */
> |                               |                           |
> point-min           return value of syntax-ppss            pos

Give me a non-artificial recipe where this happens, and I'll think about
how it should best be solved.


        Stefan

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Stefan.

On Fri, Mar 11, 2016 at 05:35:05PM -0500, Stefan Monnier wrote:
> > OK.  It wouldn't compile, so I had to correct it a little bit.

> Hmm... I've seen your arrows but they weren't sufficient to make me see
> what it was you needed to change.

> > ............"   "...." /*  "...."   */   /*  */   /*               */
> > |                               |                           |
> > point-min           return value of syntax-ppss            pos

> Give me a non-artificial recipe where this happens, and I'll think about
> how it should best be solved.

No.  (forward-comment -n) must work with any comments, artificial or
not.  Here's a line out of a C file to help with your testing:

    char foo[] = "asdf asdf" "asdf"; /* "asdf" */ /*  */  /*   '"'"  */
                      ^

.  Narrow the buffer so that point-min is at the indicated position,
then try to get (forward-comment -1) from EOL working.

But it won't help you much.  On calling syntax-ppss, find_defun_start
cannot know whether point-min is inside a string or not.  When it is,
the parse state returned is useless.

Like I keep telling you, syntax-ppss is not suitable for use in
back_comment.  Sooner or later, you must come to accept that I am right.

All the time, my new code in branch comment-cache copes with the above
situation, and all other situations, without difficulty.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 411 bytes --]

Hi Alan,

On 03/11/2016 06:08 PM, Alan Mackenzie wrote:
> Like I keep telling you, syntax-ppss is not suitable for use in
> back_comment.  Sooner or later, you must come to accept that I am right.

Would it break many things to make syntax-ppss parse from point 1 instead of point-min? Would it solve the issue that you're describing? When is it desirable to parse from point-min?

Thanks!
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Stefan Monnier]

> .  Narrow the buffer so that point-min is at the indicated position,
> then try to get (forward-comment -1) from EOL working.

If the user narrows this way, he/she gets *some* behavior, because the
meaning of "narrowing" is vague.  It can sometimes mean "I want to focus
on this particular subpart of the buffer without forgetting that the
rest exists" and sometimes "I want to pretend the rest doesn't exist at
all", and The Right Thing to do will be different in those two cases.

So the fact that my patch changes this behavior from one to the other is
of no importance in general.  You can argue that both behaviors are The
Right Thing.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > That won't be enough: I use a package by Arthur that aggressively
  > keeps everything indented (which is great)

Perhaps the problem is that that package is too aggressive.  Perhaps
it should be more cautious about reindenting lines that have an
open-paren in column 0.

You could develop another convention for indicating the lines
which you want to have start in column 0 despite being nested
in another list.  Then the package could get all of them right
every time.

-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > > For instance, what would C-M-a do?  What would C-M-e do?

  > They would do the same as they currently do - except where an
  > inadvertant paren in C0 currently prevents the correct behaviour.

In objective terms, which cases are those?

Basically, I'm asking for a complete statement of the change
in behavior you propose.  With that, we could see all of its
consequences, the good ones and the bad ones.

-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

>> They would do the same as they currently do - except where an
>> inadvertant paren in C0 currently prevents the correct behaviour.
> In objective terms, which cases are those?
> Basically, I'm asking for a complete statement of the change
> in behavior you propose.  With that, we could see all of its
> consequences, the good ones and the bad ones.

Richard.  His changes's impacts are extremely minor.
More specifically the changes are limited as follows:
- only recognizing and skipping comments is affected.
- only when moving backward.
- only when there's an open-paren-in-column-0.
- only when that open paren is inside a string or a comment.
I can't imagine a single case where the change wouldn't be considered
a clear bugfix.

It does have also performance consequences (hopefully by eliminating
some pathological cases, without unduly slowing down the common case),
of course, but I don't think that's what you were worried about.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello again, Martin.

On Wed, Mar 09, 2016 at 08:58:23PM +0100, martin rudalics wrote:
>  > ... and so far there's been no
>  > feedback from anybody else who's tried it.

> With emacs -Q and non-optimized builds I've been profiling the following
> two primitives

> (defun foo ()
>    (interactive)
>    (while (not (eobp))
>      (c-end-of-defun)))

> (defun bar ()
>    (interactive)
>    (while (not (bobp))
>      (c-beginning-of-defun)))

> to scan the entire buffer of current master's xdisp.c.  The results of
> current master with or without your change are approximately the same
> ‘foo’ taking about 73 seconds, ‘bar’ about 8 minutes, in both cases the
> comment-cache version was slightly slower.

> However, ‘foo’ is about ten times slower than for a version of Emacs
> 24.2 which is about two times slower than for a version of Emacs 23.0.
> ‘bar’ is about 5 times slower than the Emacs 24.2 version which is 3.5
> times slower than the one from Emacs 23.0.

> So, for example, executing ‘bar’ for my Emacs 23.0 took 24.89 seconds
> (0.068 for an average call) versus 464.375 seconds (1.272 average) with
> the comment-cache version.  This means that performance has deteriorated
> by a factor of 18 over the past years.  This also means that I cannot
> use non-optimized builds for my daily work any more.

> Maybe someone could try to test these two functions in a similar fashion
> so we have at least some numbers to base judgements on.  Sometimes it
> seems to me that I'm living in a different world ...

There was a nasty little bug in a low level "get me to the beginning of
a literal" function which caused it to do repeated parse-partial-sexp
from BOB.  Could you try out the patch below, please, which should fix
it, and make `bar' go at the same speed as 'foo'.  I'd be interested in
your general impression as to whether CC Mode seems any faster because
of this fix.

Now, if only we had a nice fast method of determining whether we were in
a literal, and then getting out of it .....  :-)

diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 66b5369..3ce2c23 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -4277,7 +4277,8 @@ c-ssb-lit-begin
   ;; Use `parse-partial-sexp' from a safe position down to the point to check
   ;; if it's outside comments and strings.
   (save-excursion
-    (let ((pos (point)) safe-pos state)
+    (let ((pos (point)) safe-pos state
+	  (c-state-cache (copy-tree c-state-cache)))
       ;; Pick a safe position as close to the point as possible.
       ;;
       ;; FIXME: Consult `syntax-ppss' here if our cache doesn't give a good
@@ -4288,8 +4289,7 @@ c-ssb-lit-begin
 	(setq safe-pos-list (cdr safe-pos-list)))
       (unless (setq safe-pos (car-safe safe-pos-list))
 	(setq safe-pos (max (or (c-safe-position
-				 (point) (or c-state-cache
-					     (c-parse-state)))
+				 (point) (c-parse-state))
 				0)
 			    (point-min))
 	      safe-pos-list (list safe-pos)))


> Thanks, martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > There was a nasty little bug in a low level "get me to the beginning of
 > a literal" function which caused it to do repeated parse-partial-sexp
 > from BOB.  Could you try out the patch below, please, which should fix
 > it, and make `bar' go at the same speed as 'foo'.  I'd be interested in
 > your general impression as to whether CC Mode seems any faster because
 > of this fix.

It makes backwards scanning here slower by a factor of 2 on the release
branch ;-) Now there is one thing I didn't notice earlier: The behavior
of my functions on the release branch seems OK and already as fast as
for Emacs 23 (actually, backwards scanning seems even slightly faster).

My earlier comparisons were for current master since that's where your
comment-cache changes apparently should have been applied to.  So
something's strange going on - the release branch is faster than master
by a factor of 20 on both of my functions.  Can you investigate what's
going on here?

Thanks, martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Paul Eggert]

martin rudalics wrote:
> So
> something's strange going on - the release branch is faster than master
> by a factor of 20 on both of my functions.  Can you investigate what's
> going on here?

Possibly you're seeing the effect of emacs-25 commit 
7352c6c695db8b90b63c2601277d64a32507d2bb, which was merged to master only in 
commit 63efcc268635dea78c6bd80749eae4ee2c72d717 (within the past 24 hours). If 
your master predates the later commit that could explain the symptoms you're seeing.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > Possibly you're seeing the effect of emacs-25 commit
 > 7352c6c695db8b90b63c2601277d64a32507d2bb, which was merged to master
 > only in commit 63efcc268635dea78c6bd80749eae4ee2c72d717 (within the
 > past 24 hours). If your master predates the later commit that could
 > explain the symptoms you're seeing.

I now repeated the experiments with latest master and still see the
results I mentioned earlier.  That is, with the functions I gave
earlier, the release branch is faster than master by a factor of 20!

Could someone try to repeat with master _and_ release and post the
results?

(defun foo ()
   (interactive)
   (while (not (eobp))
     (c-end-of-defun)))

(defun bar ()
   (interactive)
   (while (not (bobp))
     (c-beginning-of-defun)))

(defun foofoo ()
   (interactive)
   (elp-instrument-function 'c-end-of-defun)
   (goto-char (point-min))
   (foo)
   (elp-results))

(defun foobar ()
   (interactive)
   (elp-instrument-function 'c-beginning-of-defun)
   (goto-char (point-max))
   (bar)
   (elp-results))

(find-file-noselect "../src/xdisp.c")
(switch-to-buffer-other-window "xdisp.c")

And in xdisp.c's window do M-x foofoo and M-x foobar.

Thanks, martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[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. ]]]

  > Richard.  His changes's impacts are extremely minor.
  > More specifically the changes are limited as follows:
  > - only recognizing and skipping comments is affected.
  > - only when moving backward.
  > - only when there's an open-paren-in-column-0.
  > - only when that open paren is inside a string or a comment.

That sounds good.  The things he was saying seemed to imply
a much broader change.

-- 
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: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

On Sat, Mar 12, 2016 at 07:46:39PM +0100, martin rudalics wrote:
>  > Possibly you're seeing the effect of emacs-25 commit
>  > 7352c6c695db8b90b63c2601277d64a32507d2bb, which was merged to master
>  > only in commit 63efcc268635dea78c6bd80749eae4ee2c72d717 (within the
>  > past 24 hours). If your master predates the later commit that could
>  > explain the symptoms you're seeing.

> I now repeated the experiments with latest master and still see the
> results I mentioned earlier.  That is, with the functions I gave
> earlier, the release branch is faster than master by a factor of 20!

> Could someone try to repeat with master _and_ release and post the
> results?

> (defun foo ()
>    (interactive)
>    (while (not (eobp))
>      (c-end-of-defun)))

> (defun bar ()
>    (interactive)
>    (while (not (bobp))
>      (c-beginning-of-defun)))

OK.  Here are my timings.  Just for the record I time things with this
macro:

    (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)))

, rather than with elp, which will make my times a bit less.  I've timed
`foo' and `bar' on optimised builds in the emacs-25 branch and the
comment-cache branch (incorporating the patch I posted earlier).  Here
are the timings in seconds:

              emacs-25              comment-cache
foo            2.535                    1.651
bar           13.685                    1.620

These timings (apart from `bar' on emacs-25) are surely acceptable.

There's one subtlety about the comment-cache branch: I've now renamed
`comment-depth' (etc.) to `literal-cache' (etc.), and set
`literal-cacheing-flag' (renamed from `comment-cacheing-flag') to t by
default.  However, I've only just committed the corresponding required
change to CC Mode, so this might have a bearing on your timings.

Other than that, I can't explain what's going on with your timings.

> (defun foofoo ()
>    (interactive)
>    (elp-instrument-function 'c-end-of-defun)
>    (goto-char (point-min))
>    (foo)
>    (elp-results))

> (defun foobar ()
>    (interactive)
>    (elp-instrument-function 'c-beginning-of-defun)
>    (goto-char (point-max))
>    (bar)
>    (elp-results))

Just for the record:

               emacs-25               comment-cache
foofoo           2.545                   1.643
foobar          13.729                   1.642

> (find-file-noselect "../src/xdisp.c")
> (switch-to-buffer-other-window "xdisp.c")

> And in xdisp.c's window do M-x foofoo and M-x foobar.

> Thanks, martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Andreas Röhler]

On 11.03.2016 13:08, Alan Mackenzie wrote:
> Hello, Andreas.
>
> On Fri, Mar 11, 2016 at 08:27:17AM +0100, Andreas Röhler wrote:
>

>>> Well, given that syntax-ppss is not suitable for fixing back_comment,
>>> the alternatives are not fixing it, substantially amending syntax-ppss,
>>> using my new code, or fixing it some other way.  I really think it
>>> should be fixed.  You don't want syntax-ppss to be changed.  Do you have
>>> an idea for an "other way"?
>> There is neither a need nor a way to fix syntax-ppss : Determining
>> syntax must check from beginning of buffer, as parse-partial-sexp does.
>> parse-partial-sexp is in C, it's fast.
> parse-partial-sexp starts scanning from where you ask it to, optionally
> starting with a parse state returned by a previous invocation.  Although
> it's fast, it's not all that fast when repeatedly run over large
> portions of a large buffer.  This is what syntax-ppss was intended to
> solve.
>
> The position syntax-ppss notionally starts scanning from is
> indeterminate.  Sometimes it's the beginning of buffer, sometimes it's
> the beginning of the visible portion of the buffer, sometimes it's some
> former beginning of visible portion.
>
> Its documentation is confused, too.  The entry in the elisp manual says
> "beginning of buffer".  The doc string string says "point-min".
>
> I disagree about the need to fix syntax-ppss - medium level functions
> should be rigorous and determinate.
>
>

Needed or not: it's impossible. The idea of a cache WRT to comments 
can't work. Every insert may start a new multiline-comment at pos 1. How 
a cache will detect that?  The results of syntax-ppss are hardly 
predictable.

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Dmitry Gutov]

On 03/12/2016 10:19 PM, Andreas Röhler wrote:

> Needed or not: it's impossible. The idea of a cache WRT to comments
> can't work. Every insert may start a new multiline-comment at pos 1. How
> a cache will detect that?

syntax-ppss-flush-cache in before-change-functions.

> The results of syntax-ppss are hardly
> predictable.

This is unsubstantiated.

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Alan Mackenzie]

Hello, Andreas.

On Sat, Mar 12, 2016 at 09:19:54PM +0100, Andreas Röhler wrote:


> On 11.03.2016 13:08, Alan Mackenzie wrote:
> > Hello, Andreas.

> > On Fri, Mar 11, 2016 at 08:27:17AM +0100, Andreas Röhler wrote:

> > I disagree about the need to fix syntax-ppss - medium level functions
> > should be rigorous and determinate.

> Needed or not: it's impossible. The idea of a cache WRT to comments 
> can't work. Every insert may start a new multiline-comment at pos 1. How 
> a cache will detect that?  The results of syntax-ppss are hardly 
> predictable.

The way it's done is that on any buffer change, the cache beyond the
point of the change is marked as stale in some way.  Any function
needing to use the cache first checks that the "stale position" is
beyond where it needs the cache for, and if it isn't, recalculates the
cache for the buffer between "stale position" and point.

Have a look at the documentation for `before-change-functions' and
`after-change-functions' sometime.  Marking the cache as stale is one of
the things they are used for.

There are quite a few caches which work like this.  Font-locking works
like this, too.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Andreas Röhler]

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



On 10.03.2016 15:28, Stefan Monnier wrote:
>>> (nth 8 (syntax-ppss)) works fine for me in something like ";; some
>>> comment in an emacs-lisp-mode buffer"; it returns the starting point
>>> of the comment.
>> Does it really? For me it returns nil when the point is as shown below:
>>    <point>;; some comment in an emacs-lisp-mode buffer
> That's correct, because point is *not* inside the comment.
>
>> In all these cases, the font trick works.
> Here it's because `get-text-property' returns the property of the
> character after point, and indeed this is ";" which is in the comment.
>
> IOW they're both correct because they don't ask the same thing.
>
>> In C mode, it returns nil in the following two positions:
>>
>>    /<point>/ asd
>>    <point>// asd
>>
>> As well as these two:
>>
>>    <point>/* asd */
>>    /<point>* asd */
> Indeed, this is a current problem with parse-partial-sexp (and hence
> syntax-ppss) and is directly related to what Alan was referring to.

Can't see any issue with parse-partial-sexp - see attachment.

>
> Given the way parse-partial-sexp works (i.e. it can only look at the
> text that precedes), even if we fix it, parse-partial-sexp wouldn't be
> able to tell you that <point> is inside a comment.

That's dead wrong: parse-partial-sexp may scan  from BOB - and it must 
in certain cases to detect being in comment or not. If it must scan from 
BOB or not depends from language resp. syntax of comments.

>    But it could tell
> you that it's in the middle of something that could turn out to be
> a comment.
>
>
>          Stefan
>
>


[-- Attachment #2: comment.png --]
[-- Type: image/png, Size: 138891 bytes --]

Re: How do you check if the current point is in a comment or a string? (Was Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.])

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 161 bytes --]

On 03/12/2016 03:45 PM, Andreas Röhler wrote:
> Can't see any issue with parse-partial-sexp - see attachment.

I have no idea what the attachment shows :/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/12/2016 07:08 PM, Alan Mackenzie wrote:

> Now, if only we had a nice fast method of determining whether we were in
> a literal, and then getting out of it .....  :-)

Why not

(save-restriction
   (prog-widen)
   (let* ((state (syntax-ppss))
          (lit-start (nth 8 state)))
     (when lit-start
       (goto-char lit-start))))

?

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 534 bytes --]

On 03/12/2016 03:56 PM, Dmitry Gutov wrote:
> On 03/12/2016 07:08 PM, Alan Mackenzie wrote:
> 
>> Now, if only we had a nice fast method of determining whether we were in
>> a literal, and then getting out of it .....  :-)
> 
> Why not
> 
> (save-restriction
>   (prog-widen)
>   (let* ((state (syntax-ppss))
>          (lit-start (nth 8 state)))
>     (when lit-start
>       (goto-char lit-start))))
> 
> ?

Does this break on 

  /<point>* test */

in C mode? (<point> indicate the position of the point)



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Sat, Mar 12, 2016 at 10:56:18PM +0200, Dmitry Gutov wrote:
> On 03/12/2016 07:08 PM, Alan Mackenzie wrote:

> > Now, if only we had a nice fast method of determining whether we were in
> > a literal, and then getting out of it .....  :-)

> Why not

> (save-restriction
>    (prog-widen)
>    (let* ((state (syntax-ppss))
>           (lit-start (nth 8 state)))
>      (when lit-start
>        (goto-char lit-start))))

> ?

In a large buffer, this involves scanning on average 10,000 characters
with parse-partial-sexp.  This is not what I meant by "fast".

What I had in mind looks more like (note the recent renaming of
`comment-depth' to `literal-cache'):

  (setq lc (get-text-property (point) 'literal-cache))
  (when (not (equal lc '(0 . 0)))
    (setq start (previous-single-property-change (point) 'literal-cache))
    (goto-char start)
    ;; Move back over opening delimiter(s).
    )

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/12/2016 11:29 PM, Clément Pit--Claudel wrote:

> Does this break on
>
>   /<point>* test */
>
> in C mode? (<point> indicate the position of the point)

If by break you mean does nothing, then yes. In what practical 
situations is this a problem?

There might be ways to alleviate it (make syntax-ppss scan forward a 
little, e.g. till the end of the line), but a naive solution might mess 
up how syntax-propertize works:

a) It wraps the calls to syntax-propertize-function in 
inhibit-modification-hooks.

b) That function is allowed to call syntax-ppss as well, and then modify 
the syntax-table property of any position after the call. If syntax-ppss 
is allowed to rely on the syntax class of '*', it might return (and 
cache) a value that will turn out to be false in the future.

*If* the issue is an actual problem, the best way to handle it might be 
in the caller of syntax-ppss. If the caller is not a part of 
syntax-propertize-function, or at least it doesn't inhibit modification 
hooks, it can try evaluating (nth 8 (syntax-ppss pos)) in several 
positions - from (point) to (+ (point) n -1), where n is the maximum 
length of a comment starter in the current language. When one of the 
calls returns a non-nil position (but one that's before point) => jump 
there.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/12/2016 11:58 PM, Alan Mackenzie wrote:

> In a large buffer, this involves scanning on average 10,000 characters
> with parse-partial-sexp.  This is not what I meant by "fast".

That severely depends on your usage. Have you tried it?

First, if your calls to syntax-ppss are performed with non-decreasing 
argument (or it decreases rarely), the cache in syntax-ppss-last will 
help a lot.

Second, I'm sure you can decrease syntax-ppss-max-span, if your language 
is expensive to parse.

Third, I *would* call it fast. Examples:

in syntax.el, (parse-partial-sexp 1 10000) takes ~0.0004sec.

in xdisp.c, it takes ~0.0003sec. Probably because it begins with a huge 
comment.

All right, let's get out of that comment:

(parse-partial-sexp 15082 25082) takes ~0.0005sec.

You can try it by evaluating

(benchmark 1 '(parse-partial-sexp 15082 25082))

> What I had in mind looks more like (note the recent renaming of
> `comment-depth' to `literal-cache'):
>
>   (setq lc (get-text-property (point) 'literal-cache))
>   (when (not (equal lc '(0 . 0)))
>     (setq start (previous-single-property-change (point) 'literal-cache))
>     (goto-char start)
>     ;; Move back over opening delimiter(s).
>     )

Yes, it's probably faster on average, by some constant multiplier. But 
it comes with a set of drawbacks that has already been brought up in the 
discussions.

Do you really call this code more than 100 times per user command?

Since the text property approach exchanges memory for performance, 
setting syntax-ppss-max-span to some lower value seems to be the thing 
to try first (yielding a similar effect).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > OK.  Here are my timings.  Just for the record I time things with this
 > macro:
 >
 >      (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)))
 >
 > , rather than with elp, which will make my times a bit less.  I've timed
 > `foo' and `bar' on optimised builds in the emacs-25 branch and the
 > comment-cache branch (incorporating the patch I posted earlier).  Here
 > are the timings in seconds:
 >
 >                emacs-25              comment-cache
 > foo            2.535                    1.651
 > bar           13.685                    1.620
 >
 > These timings (apart from `bar' on emacs-25) are surely acceptable.
 >
 > There's one subtlety about the comment-cache branch: I've now renamed
 > `comment-depth' (etc.) to `literal-cache' (etc.), and set
 > `literal-cacheing-flag' (renamed from `comment-cacheing-flag') to t by
 > default.  However, I've only just committed the corresponding required
 > change to CC Mode, so this might have a bearing on your timings.
 >
 > Other than that, I can't explain what's going on with your timings.
 >
 >> (defun foofoo ()
 >>     (interactive)
 >>     (elp-instrument-function 'c-end-of-defun)
 >>     (goto-char (point-min))
 >>     (foo)
 >>     (elp-results))
 >
 >> (defun foobar ()
 >>     (interactive)
 >>     (elp-instrument-function 'c-beginning-of-defun)
 >>     (goto-char (point-max))
 >>     (bar)
 >>     (elp-results))
 >
 > Just for the record:
 >
 >                 emacs-25               comment-cache
 > foofoo           2.545                   1.643
 > foobar          13.729                   1.642

I yet don't care about comment-cache or the other fix you proposed.  I
compared the emacs-25 branch with current _master_ because here the
latter is slower by a factor of 20.  And my measurements are for builds
without optimization only.

As long as this riddle is not resolved it doesn't make much sense for me
to test minor optimizations.  Please clarify this issue.

martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Daniel Colascione]

[-- Attachment #1.1: Type: text/plain, Size: 934 bytes --]

On 03/09/2016 12:53 PM, John Wiegley wrote:
>>>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
>> I concur that the current C mode is much slower than it was several releases
>> ago. My personal subjective impression is that it gets slower with every new
>> release, and I suspect that the main reason is our desire to support more
>> and more C quirks and subtleties. If this is true, I'd be glad to go back to
>> a simpler C mode, which sometimes mis-fontified or even (gasp!) mis-indented
>> an occasional rare construct, but to get back the speed we enjoyed in the
>> past.
> 
> I entirely agree, Eli.
> 

As someone who regularly works with C++ code written by people who want
to use every new language construct that comes along, cc-mode is already
barely usable from a correctness perspective. I find cc-mode's
performance adequate. Simplifying cc-mode further would make it and
Emacs much less useful for me.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Martin.

On Sun, Mar 13, 2016 at 10:26:10AM +0100, martin rudalics wrote:

> I compared the emacs-25 branch with current _master_ because here the
> latter is slower by a factor of 20.  And my measurements are for
> builds without optimization only.

> As long as this riddle is not resolved it doesn't make much sense for me
> to test minor optimizations.  Please clarify this issue.

OK.  I've just updated both emacs-25 and master, and built them normally
(i.e. with default optimisation and no debugging info).  The timings I
see (in seconds) are:

            emacs-25          master
foo          2.62              2.78
bar         13.64             13.93             

I honestly don't know why you're seeing a factor of 20 difference.  I
doubt it's got anything to do with CC Mode.  Perhaps you could tell me
what flags (e.g. CFLAGS) you're using in your non-optimised builds, and
I can rebuild and try again.

I also think that under 3 seconds is an acceptable running time for
`foo' and `bar'.  (Yesterday's patch brings `bar''s running time down to
that of `foo')

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > OK.  I've just updated both emacs-25 and master, and built them normally
 > (i.e. with default optimisation and no debugging info).  The timings I
 > see (in seconds) are:
 >
 >              emacs-25          master
 > foo          2.62              2.78
 > bar         13.64             13.93
 >
 > I honestly don't know why you're seeing a factor of 20 difference.

Probably because I build with debug information.  With optimized builds
I get more or less the same figures as you do.

 > I
 > doubt it's got anything to do with CC Mode.  Perhaps you could tell me
 > what flags (e.g. CFLAGS) you're using in your non-optimised builds,

CFLAGS='-O0 -g3' --with-wide-int --enable-checking=yes --enable-check-lisp-object-type=yes

 > and
 > I can rebuild and try again.

Please do that.

martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Martin.

On Sun, Mar 13, 2016 at 01:08:24PM +0100, martin rudalics wrote:
>  > OK.  I've just updated both emacs-25 and master, and built them normally
>  > (i.e. with default optimisation and no debugging info).  The timings I
>  > see (in seconds) are:

>  >              emacs-25          master
>  > foo          2.62              2.78
>  > bar         13.64             13.93

>  > I honestly don't know why you're seeing a factor of 20 difference.

> Probably because I build with debug information.  With optimized builds
> I get more or less the same figures as you do.

>  > I
>  > doubt it's got anything to do with CC Mode.  Perhaps you could tell me
>  > what flags (e.g. CFLAGS) you're using in your non-optimised builds,

> CFLAGS='-O0 -g3' --with-wide-int --enable-checking=yes --enable-check-lisp-object-type=yes

>  > and
>  > I can rebuild and try again.

> Please do that.

                 emacs-25          master
foo               23.25             24.70
bar              146.73            153.38

And just in case the problem might be with elp,

                 emacs-25          master
foofoo            22.69             24.07
foobar           146.98            152.33

I don't know what more I can do at this end.  A factor of 20 takes a lot
of explaining.  Just for the record, I've been running on Linux virtual
terminals.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > I don't know what more I can do at this end.

Neither do I.  Let's leave it there ;-)

martin

Re: Problems with syntax-ppss: Was [... Apply `comment-depth' text properties when calling `back_comment'.]

[Andreas Röhler]

On 12.03.2016 21:45, Alan Mackenzie wrote:

> Needed or not: it's impossible. The idea of a cache WRT to comments
> can't work. Every insert may start a new multiline-comment at pos 1. How
> a cache will detect that?  The results of syntax-ppss are hardly
> predictable.
> The way it's done is that on any buffer change, the cache beyond the
> point of the change is marked as stale in some way.  Any function
> needing to use the cache first checks that the "stale position" is
> beyond where it needs the cache for, and if it isn't, recalculates the
> cache for the buffer between "stale position" and point.
>
> Have a look at the documentation for `before-change-functions' and
> `after-change-functions' sometime.  Marking the cache as stale is one of
> the things they are used for.
>
> There are quite a few caches which work like this.  Font-locking works
> like this, too.
>

Ahh, see, cool solution. My apologies to Stefan, assume he deserves the 
credits here.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> As someone who regularly works with C++ code written by people who want
> to use every new language construct that comes along, cc-mode is already
> barely usable from a correctness perspective. I find cc-mode's
> performance adequate. Simplifying cc-mode further would make it and
> Emacs much less useful for me.

I'm lucky not to use C++ mode very often, but I agree that we need to be
very careful with this.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> From: Daniel Colascione <dancol@dancol.org>
> Date: Sun, 13 Mar 2016 01:30:00 -0800
> 
> As someone who regularly works with C++ code written by people who want
> to use every new language construct that comes along, cc-mode is already
> barely usable from a correctness perspective. I find cc-mode's
> performance adequate.

A 10-second delay before Emacs responds to a single keypress (see
bug#22844) is certainly not adequate performance.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Daniel Colascione]

[-- Attachment #1.1: Type: text/plain, Size: 601 bytes --]

On 03/13/2016 09:24 AM, Eli Zaretskii wrote:
>> From: Daniel Colascione <dancol@dancol.org>
>> Date: Sun, 13 Mar 2016 01:30:00 -0800
>>
>> As someone who regularly works with C++ code written by people who want
>> to use every new language construct that comes along, cc-mode is already
>> barely usable from a correctness perspective. I find cc-mode's
>> performance adequate.
> 
> A 10-second delay before Emacs responds to a single keypress (see
> bug#22844) is certainly not adequate performance.

I never hit that bug, and I don't want cc-mode dumbed down globally to
address it.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> Cc: rudalics@gmx.at, acm@muc.de, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Daniel Colascione <dancol@dancol.org>
> Date: Sun, 13 Mar 2016 09:27:13 -0700
> 
> > A 10-second delay before Emacs responds to a single keypress (see
> > bug#22844) is certainly not adequate performance.
> 
> I never hit that bug, and I don't want cc-mode dumbed down globally to
> address it.

CC mode is not supposed to address only your personal needs.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> Date: Sun, 13 Mar 2016 14:32:37 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel@gnu.org
> 
>  > I don't know what more I can do at this end.
> 
> Neither do I.  Let's leave it there ;-)

Here are my results, with today's emacs-25 and master:

            emacs-25     master    emacs-25.0.92    emacs-23.3
foo          33.375       33.89       2.859            1.375
bar         213.250      213.578     15.547            8.625

The two former versions are compiled the same as Martin's, but without
"--enable-check-lisp-object-type=yes".  The two latter versions are
optimized builds, and emacs-23.3 is (of course) without --wide-int.

Also note that I killed the buffer visiting xdisp.c between testing
'foo' and 'bar'.

Martin, could it be that --enable-check-lisp-object-type=yes makes
such a big difference on your system?

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Sun, Mar 13, 2016 at 12:16:38AM +0200, Dmitry Gutov wrote:
> On 03/12/2016 11:58 PM, Alan Mackenzie wrote:

> > In a large buffer, this involves scanning on average 10,000 characters
> > with parse-partial-sexp.  This is not what I meant by "fast".

> That severely depends on your usage. Have you tried it?

Have now, yes.

> First, if your calls to syntax-ppss are performed with non-decreasing 
> argument (or it decreases rarely), the cache in syntax-ppss-last will 
> help a lot.

OK, I'd overlooked that.

> Second, I'm sure you can decrease syntax-ppss-max-span, if your language 
> is expensive to parse.

I would think the speed of parse-partial-sexp is pretty much constant,
independent of the language.

> Third, I *would* call it fast. Examples:

[ .... ]

> (parse-partial-sexp 15082 25082) takes ~0.0005sec.

OK.

[ .... ]

> > What I had in mind looks more like (note the recent renaming of
> > `comment-depth' to `literal-cache'):

> >   (setq lc (get-text-property (point) 'literal-cache))
> >   (when (not (equal lc '(0 . 0)))
> >     (setq start (previous-single-property-change (point) 'literal-cache))
> >     (goto-char start)
> >     ;; Move back over opening delimiter(s).
> >     )

> Yes, it's probably faster on average, by some constant multiplier.

It will be much faster.  The only looping involved is over the levels of
an optimised binary tree (the text property intervals), not linearly
over 10k characters.  By the time syntax-ppss has finished consing down
its cache list, the text property stuff would be done.

> But it comes with a set of drawbacks that has already been brought up
> in the discussions.

That set has one member: the partial overlap in functionality with
syntax-ppss.  No intrinsic drawbacks have yet been pointed out.  But you
do so below (thanks!).

> Since the text property approach exchanges memory for performance, 

I've counted the intervals on xdisp.c.  Fully fontified, xdisp on master,
there are 55274 intervals with ~442192 conses (That's estimating 2
conses per symbol/property pair on the interval's plist).

In branch comment-cache, on a fully fontified, and fully cached xdisp.c
buffer, there are 55786 intervals with ~557860 conses, that's an extra
512 intervals, or less than 1%.  There are an extra ~115668 conses,
about an extra 25%.  I'll admit this is a drawback.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> Date: Sun, 13 Mar 2016 12:49:58 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel@gnu.org
> 
> Just for the record, I've been running on Linux virtual terminals.

Does that mean a text terminal?

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Eli.

On Sun, Mar 13, 2016 at 08:00:00PM +0200, Eli Zaretskii wrote:
> > Date: Sun, 13 Mar 2016 12:49:58 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel@gnu.org

> > Just for the record, I've been running on Linux virtual terminals.

> Does that mean a text terminal?

Yes.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > Here are my results, with today's emacs-25 and master:
 >
 >              emacs-25     master    emacs-25.0.92    emacs-23.3
 > foo          33.375       33.89       2.859            1.375
 > bar         213.250      213.578     15.547            8.625
 >
 > The two former versions are compiled the same as Martin's, but without
 > "--enable-check-lisp-object-type=yes".  The two latter versions are
 > optimized builds, and emacs-23.3 is (of course) without --wide-int.
 >
 > Also note that I killed the buffer visiting xdisp.c between testing
 > 'foo' and 'bar'.
 >
 > Martin, could it be that --enable-check-lisp-object-type=yes makes
 > such a big difference on your system?

I don't think so.  I currently switched all my debug builds to -O2 so
that I can at least do some practical work with the release version and
got the following results:


Emacs-25 CFLAGS='-O2 -g3 -gdwarf-2' --with-wide-int --enable-checking=yes --enable-check-lisp-object-type=yes

c-end-of-defun          365         5.3909999999  0.0147698630
c-beginning-of-defun    365        28.921999999   0.0792383561


Emacs-25 CFLAGS='-O2 -g3 -gdwarf-2' --with-wide-int --enable-checking=yes

c-end-of-defun  	365         5.3599999999  0.0146849315
c-beginning-of-defun  	365        28.906999999   0.0791972602


comment-cache CFLAGS='-O2 -g3 -gdwarf-2' --with-wide-int --enable-checking=yes --enable-check-lisp-object-type=yes

c-end-of-defun          365         3.8270000000  0.0104849315
c-beginning-of-defun    365        28.499999999   0.0780821917


master CFLAGS='-O2 -g3 -gdwarf-2' --with-wide-int --enable-checking=yes --enable-check-lisp-object-type=yes

c-end-of-defun          365         5.7809999999  0.0158383561
c-beginning-of-defun    365        29.608999999   0.0811205479


master CFLAGS='-O3'

c-end-of-defun  	365         3.9530000000  0.0108301369
c-beginning-of-defun 	365        20.124999999   0.0551369863


The only remarkable difference I see is that c-end-of-defun with the
comment-cache version is significantly faster.  The values for the
relatively slow c-beginning-of-defun practically don't change unless I
build without debugging information.

martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> In branch comment-cache, on a fully fontified, and fully cached xdisp.c
> buffer, there are 55786 intervals with ~557860 conses, that's an extra
> 512 intervals, or less than 1%.  There are an extra ~115668 conses,
> about an extra 25%.  I'll admit this is a drawback.

There's also the extra cost of applying all those text-properties plus
the cost of slowing down application of other text-properties (since
every time we modify intervals, we have to play with the lists of
properties of the intervals involved), plus the cost of slowing down
all lookups of other text-properties.

These were some of the reasons which made me choose a separate table for
syntax-ppss rather than using text-properties.

This said, I don't think it matters much.  FWIW, syntax-ppss uses a very
naive singly-linked list for the simple reason that I decided to "first
get something working", and I just haven't found a need to go back and
optimize this data-structure (which could be turned into a tree or even
an array, in case it mattered).

Another motivation to go with a separate table was that I figured
I would then be able to more easily manipulate the cache, e.g. in case
I wanted to keep several versions of the cache at the same time (e.g. one
version per syntax-table value being used, or one version per value of
`point-min').  But there again, I haven't had the need to go back and
add that kind of functionality yet.

IOW, I don't think the exact representation chosen matters much.

What I do think is a pity in your implementation (thinking of it as
a potential replacement for syntax-ppss) is that it has to do all the
hard work of computing a full "parse-partial-sexp state" (aka PPSS) and
then throws away the parenthesis part of the state to only keep the
string/comment part, making it unable to provide as much info as
syntax-ppss does, even though it had to work just as hard for it.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/13/2016 07:59 PM, Alan Mackenzie wrote:

>> That severely depends on your usage. Have you tried it?
>
> Have now, yes.

Any performance numbers on using this approach in C/l mode?

>> Second, I'm sure you can decrease syntax-ppss-max-span, if your language
>> is expensive to parse.
>
> I would think the speed of parse-partial-sexp is pretty much constant,
> independent of the language.

Well, it seems like that you would be calling it much more frequently 
than other packages. So you could adjust for that.

> It will be much faster.  The only looping involved is over the levels of
> an optimised binary tree (the text property intervals), not linearly
> over 10k characters.  By the time syntax-ppss has finished consing down
> its cache list, the text property stuff would be done.

Yes, it would be fast. But like we've noticed, calling syntax-ppss is 
also quick, and it basically has a constant upper bound, on average. 
It's hard to be *significantly* faster than that.

So I have to wonder why the "get out of a comment" feature is used in 
C/l mode so much that it becomes a bottleneck, and you get significant 
improvement in performance by dropping the caching logic to C. That is, 
of course, not a nice thing to ask considering the overall complexity of 
CC Mode, but still.

I don't see anything comparable to 10 second waiting described in 
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884, when doing a 
comparable operation in a 5000-line Ruby file.

>> But it comes with a set of drawbacks that has already been brought up
>> in the discussions.
>
> That set has one member: the partial overlap in functionality with
> syntax-ppss.  No intrinsic drawbacks have yet been pointed out.  But you
> do so below (thanks!).

Aside from everything that's been mentioned already, it's written in C, 
reducing the pool of developers willing to work on it. And no, it 
doesn't make it "simple and maintainable", at least from my perspective.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

>> I never hit that bug, and I don't want cc-mode dumbed down globally to
>> address it.
> CC mode is not supposed to address only your personal needs.

But we shouldn't disregard those needs either.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Paul Eggert]

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

Eli Zaretskii wrote:
> Here are my results, with today's emacs-25 and master:
>
>              emacs-25     master    emacs-25.0.92    emacs-23.3
> foo          33.375       33.89       2.859            1.375
> bar         213.250      213.578     15.547            8.625

I get somewhat different results: when comparing Emacs 24.2 to Emacs master, 
24.2 is 8% faster with -O2, 25% faster with -Og, and 3x faster with -O0. So, 
although there's a bit of slowdown otherwise (which should get looked at, at 
some point), the main culprit is -O0, and it probably is due to -O0's not 
inlining functions.

A quick build with --enable-profiling and CFLAGS=-O0 suggests that the primary 
bottlenecks with -O0 are CHAR_TABLE_REF_ASCII (18%), make_lisp_symbol (15%), 
builtin_lisp_symbol (11%), and PSEUDOVECTORP (6%).  I suppose we could spend 
some time trying to make these go faster when compiling with -O0, but I think 
our time is better spent asking developers to use -Og, or to use -O0 more 
selectively.

I used master commit 181e92c4e060a7ce4740b561375f9ec9f473f144 src/xdisp.c, with 
Emacs built on Fedora 23 x86-64 (GCC 5.3.1):

	      foo bar
   24.2   -O2  2.4  13
   24.2   -Og  3.0  17
   24.2   -O0  5.2  32
   master -O2  2.6  14
   master -Og  3.8  21
   master -O0 15    96

Fedora 23 can't build Emacs 23.3 due to some minor portability problems in 23.3, 
so I didn't benchmark that. I used the attached Lisp code to benchmark.



[-- Attachment #2: bench-rudalics.el --]
[-- Type: text/x-emacs-lisp, Size: 494 bytes --]

(defun foo ()
  (interactive)
  (while (not (eobp))
    (c-end-of-defun)))


(defun bar ()
  (interactive)
  (while (not (bobp))
    (c-beginning-of-defun)))

(defun bench ()
  (find-file "xdisp.c")
  (goto-char (point-min))
  (let* ((t1 (current-time))
         (t2 (progn
               (foo)
               (current-time)))
         (t3 (progn
               (bar)
               (current-time))))
    (cons (float-time (time-subtract t2 t1))
          (float-time (time-subtract t3 t2)))))

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> Yes, it would be fast. But like we've noticed, calling syntax-ppss is also
> quick, and it basically has a constant upper bound, on average. It's hard to
> be *significantly* faster than that.

> So I have to wonder why the "get out of a comment" feature is used in C/l
> mode so much that it becomes a bottleneck, and you get significant
> improvement in performance by dropping the caching logic to C. That is, of
> course, not a nice thing to ask considering the overall complexity of CC
> Mode, but still.

Hmm... I get the impression that maybe there's some confusion here.
The current "get out of a comment" feature doesn't use syntax-ppss.

It parses comments backward using ad-hoc logic to handle the (hopefully)
common cases that *can* be handled.  But in some cases, you can't really
parse comments backward and you need to jump back to a "safe spot"
(outside of comments/strings) and parse forward from there.  In the
current Emacs code this "safe spot" is either point-min or the nearest
open-paren-in-column-0.  syntax-ppss could provide this safe spot, of
course, but we don't do that yet (that's what my patch does).

> I don't see anything comparable to 10 second waiting described in
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884, when doing a comparable
> operation in a 5000-line Ruby file.

Ruby's comment syntax is easier to parse backward than C's (because
there's only one syntax style, rather than 2), so it will not fallback
on the forward parsing as often as in C.  Maybe that's part of the
explanation.  Then again, maybe it has nothing to do with it, and
depends on specifics of CC-mode.  I don't know enough about what Emacs
is doing during those famous 10s.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/14/2016 03:30 AM, Stefan Monnier wrote:

>> So I have to wonder why the "get out of a comment" feature is used in C/l
>> mode so much that it becomes a bottleneck, and you get significant
>> improvement in performance by dropping the caching logic to C. That is, of
>> course, not a nice thing to ask considering the overall complexity of CC
>> Mode, but still.
>
> Hmm... I get the impression that maybe there's some confusion here.
> The current "get out of a comment" feature doesn't use syntax-ppss.

Maybe I should rephrase that: why is it important to be a lot faster 
than syntax-ppss? Because speed seems to be the main advantage that 
Alan's proposal has.

Would using your syntax-ppss approach instead chip away at the reported 
80% improvement in the synthetic test, or will it be about as fast?

(As an aside: we do have `comment-beginning', which delegates to 
syntax-ppss. But it doesn't seem to be widely used.)

>> I don't see anything comparable to 10 second waiting described in
>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884, when doing a comparable
>> operation in a 5000-line Ruby file.
>
> Ruby's comment syntax is easier to parse backward than C's (because
> there's only one syntax style, rather than 2),

There are two:

#

and

=begin
=end

The latter one gets handled in ruby-syntax-propertize, so I don't know 
if it's relevant.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> Maybe I should rephrase that: why is it important to be a lot faster than
> syntax-ppss?

Ah, I see.  I wouldn't know: I tuned syntax-ppss speed a bit when
I first wrote it, but there's a lot of room for improvement, if needed,
I think.  I haven't done any of that work because I haven't found a need
for it.

> Because speed seems to be the main advantage that Alan's proposal has.

I haven't seen any comparison of Alan's proposal with mine, so I don't
know how they compare.  Their "cost profiles" are quite a bit different
(since my patch re-uses the existing cache whereas his has to fill
another one, but his cache can give him a more immediate answer once
it's filled), so it's not clear to me.

> Would using your syntax-ppss approach instead chip away at the reported 80%
> improvement in the synthetic test, or will it be about as fast?

I'd expect it to be about "same as now" for the normal case, and "same
as Alan's" for the "famous 10s" case.

> (As an aside: we do have `comment-beginning', which delegates to
> syntax-ppss. But it doesn't seem to be widely used.)

IIUC the interesting case is the one where the Elisp code doesn't
actually care about moving outside of a comment.  Instead the need to
skip over a comment comes indirectly via parse-sexp-ignore-comments.
So calling `comment-beginning' is not really an option (and of course
`comment-beginning' does more than find the beginning, since it also
looks for the beginning of the *content* of the comment).

> The latter one gets handled in ruby-syntax-propertize, so I don't know if
> it's relevant.

It's only relevant when it does appear, which should be fairly rare.
In contrast "\n" (the comment ender for the // A comments) appears
fairly often inside a /*...*/ comment.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > 24.2 is 8% faster with -O2, 25% faster with -Og, and 3x faster with -O0.
...
 > I suppose we could spend some time trying to make these go
 > faster when compiling with -O0, but I think our time is better spent
 > asking developers to use -Og, or to use -O0 more selectively.

I'd really appreciate some guidelines on this.  My restrictions are two:

(1) Rebuilding Emacs for the sake of debugging a particular problem is
in general _not_ an option because it takes almost 1/2 an hour here (and
another 1/2 hour to go back to the previous version).

(2) Maintaining separate -O0 and -O2 builds is no real option either.
I've done that in the past for master only but I never started to do
that for the release or other branches because it gets confusing and the
overhead is not negligible either.  On GNU/Linux I already produce
separate builds for Gtk, Motif, Lucid, no-toolkit and GNUStep for both
master and release plus an -O3 optimized Gtk build.

Currently, I've started using '-O2 -g3 -gdwarf-2' throughout but if
people think that -Og or any other option is preferable, I shall do so
(I think that I can live with a C-mode slowdown of 25%).

martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Martin.

On Mon, Mar 14, 2016 at 08:42:46AM +0100, martin rudalics wrote:
>  > 24.2 is 8% faster with -O2, 25% faster with -Og, and 3x faster with -O0.
> ...
>  > I suppose we could spend some time trying to make these go
>  > faster when compiling with -O0, but I think our time is better spent
>  > asking developers to use -Og, or to use -O0 more selectively.

> I'd really appreciate some guidelines on this.  My restrictions are two:

> (1) Rebuilding Emacs for the sake of debugging a particular problem is
> in general _not_ an option because it takes almost 1/2 an hour here (and
> another 1/2 hour to go back to the previous version).

Ouch!  It takes me around 3 - 4 minutes, including running ./configure.
I run "make clean" only in the src directory, not over the whole of
Emacs.  What are you doing that takes half an hour?

[ .... ]

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Mon, Mar 14, 2016 at 03:13:43AM +0200, Dmitry Gutov wrote:
> On 03/13/2016 07:59 PM, Alan Mackenzie wrote:

> >> That severely depends on your usage. Have you tried it?

> > Have now, yes.

> Any performance numbers on using this approach in C/l mode?

[ By "this approach" I'm taking it you mean scanning forward from safe
positions.]

No.  Except it uses a lot of parsing forward from safe positions anyway.

> >> Second, I'm sure you can decrease syntax-ppss-max-span, if your language
> >> is expensive to parse.

> > I would think the speed of parse-partial-sexp is pretty much constant,
> > independent of the language.

> Well, it seems like that you would be calling it much more frequently 
> than other packages. So you could adjust for that.

Maybe.

> > It will be much faster.  The only looping involved is over the levels of
> > an optimised binary tree (the text property intervals), not linearly
> > over 10k characters.  By the time syntax-ppss has finished consing down
> > its cache list, the text property stuff would be done.

> Yes, it would be fast. But like we've noticed, calling syntax-ppss is 
> also quick, and it basically has a constant upper bound, on average. 
> It's hard to be *significantly* faster than that.

It's two slow to use as a replacement for back_comment, in the sense I
would like to do - in place of scanning a comment backward character by
character, I want to use a cache (calculated by forward scanning) to
determine the beginning of a comment.  Having to scan forward lots of
characters (as opposed to a few) is out of the question for this
application.

> So I have to wonder why the "get out of a comment" feature is used in 
> C/l mode so much that it becomes a bottleneck, and you get significant 
> improvement in performance by dropping the caching logic to C. That is, 
> of course, not a nice thing to ask considering the overall complexity of 
> CC Mode, but still.

> I don't see anything comparable to 10 second waiting described in 
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884, when doing a 
> comparable operation in a 5000-line Ruby file.

Here's a quick summary of how that happened:  on L1661 of config.h, C
Mode had to scan back to the beginning of a statement.  That involved
going back over ~1600 lines of comments and preprocessor constructs.
When it got to the critical comment and tried to go back over it,
(forward-comment -1) said nil, because of that paren in column 0.  That
paren in column 0, although in a comment, was deemed to be the start of
a defun.  C Mode was then trying to parse "code" over a region with a
1600 line gap in the middle.  Hence the 10 second delay in seeing the
character echoed.  Paul has purged our code of parens in column 0.  But
it would be nice not to have the restriction.

> >> But it comes with a set of drawbacks that has already been brought up
> >> in the discussions.

> > That set has one member: the partial overlap in functionality with
> > syntax-ppss.  No intrinsic drawbacks have yet been pointed out.  But you
> > do so below (thanks!).

> Aside from everything that's been mentioned already, it's written in C, 
> reducing the pool of developers willing to work on it. And no, it 
> doesn't make it "simple and maintainable", at least from my perspective.

My point was that it is so simple that it _could_ be written in C, and
that without any great difficulties.  Parts of it can only be written in
C (the bits that ensure the cache is marked stale when certain
sytax-table text properties are set/cleared when
`inhibit-modification-hooks' is bound to non-nil).

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Stefan.

On Sun, Mar 13, 2016 at 06:49:53PM -0400, Stefan Monnier wrote:
> > In branch comment-cache, on a fully fontified, and fully cached xdisp.c
> > buffer, there are 55786 intervals with ~557860 conses, that's an extra
> > 512 intervals, or less than 1%.  There are an extra ~115668 conses,
> > about an extra 25%.  I'll admit this is a drawback.

> There's also the extra cost of applying all those text-properties ....

I've just timed this over xdisp.c.  A parse-partial-sexp from
(point-min) to (point-max) took 0.031s.  A further scan, applying the
`literal-cache' properties, took 0.054s.  That's 0.023s on a 1 MByte
buffer.  That's not too serious.

> .... plus the cost of slowing down application of other
> text-properties (since every time we modify intervals, we have to play
> with the lists of properties of the intervals involved), plus the cost
> of slowing down all lookups of other text-properties.

I imagine the cost of consing over three properties rather than just two
in an interval's plist will be negligible.  The impact of the extra
intervals will also be small.  Heck, we use two whole properties for
font-locking, and nobody notices any slow downs that causes.  One
implementation possibility would be to put `literal-cache' properties on
a distinct interval tree from text properties, thus speeding up both,
but it doesn't seem needed as yet.

> These were some of the reasons which made me choose a separate table for
> syntax-ppss rather than using text-properties.

OK.

> This said, I don't think it matters much.  FWIW, syntax-ppss uses a very
> naive singly-linked list for the simple reason that I decided to "first
> get something working", and I just haven't found a need to go back and
> optimize this data-structure (which could be turned into a tree or even
> an array, in case it mattered).

> Another motivation to go with a separate table was that I figured
> I would then be able to more easily manipulate the cache, e.g. in case
> I wanted to keep several versions of the cache at the same time (e.g. one
> version per syntax-table value being used, or one version per value of
> `point-min').  But there again, I haven't had the need to go back and
> add that kind of functionality yet.

> IOW, I don't think the exact representation chosen matters much.

> What I do think is a pity in your implementation (thinking of it as
> a potential replacement for syntax-ppss) is that it has to do all the
> hard work of computing a full "parse-partial-sexp state" (aka PPSS) and
> then throws away the parenthesis part of the state to only keep the
> string/comment part, making it unable to provide as much info as
> syntax-ppss does, even though it had to work just as hard for it.

Retaining all the information would be much harder and much more
resource intensive.  That why I said the comment-cache implementation is
for a different purpose from syntax-ppss.

syntax-ppss throws away a lot more information that comment-cache does -
it retains a parse state only every 20,000 characters, discarding
everything else between those points.  This makes its cache unsuitable
for using the way I'd like to in back_comment.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> Cc: acm@muc.de, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 13 Mar 2016 18:15:38 -0700
> 
> 	      foo bar
>    24.2   -O2  2.4  13
>    24.2   -Og  3.0  17
>    24.2   -O0  5.2  32
>    master -O2  2.6  14
>    master -Og  3.8  21
>    master -O0 15    96
> 
> Fedora 23 can't build Emacs 23.3 due to some minor portability problems in 23.3, 
> so I didn't benchmark that. I used the attached Lisp code to benchmark.

For completeness, here are my results including v24.2, compiled with
optimizations:

            emacs-25     master    emacs-25.0.92  emacs-24.2   emacs-23.3
foo          33.375       33.89       2.859         2.312        1.375
bar         213.250      213.578     15.547        10.125        8.625

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Sun, 13 Mar 2016 21:13:46 -0400
> Cc: rudalics@gmx.at, Daniel Colascione <dancol@dancol.org>, emacs-devel@gnu.org,
> 	acm@muc.de
> 
> >> I never hit that bug, and I don't want cc-mode dumbed down globally to
> >> address it.
> > CC mode is not supposed to address only your personal needs.
> 
> But we shouldn't disregard those needs either.

No, of course not.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

Hi Alan,

On 03/14/2016 02:23 PM, Alan Mackenzie wrote:

>> Any performance numbers on using this approach in C/l mode?
>
> [ By "this approach" I'm taking it you mean scanning forward from safe
> positions.]

No, using syntax-ppss's cache. Including the syntax-ppss-last variable.

> No.  Except it uses a lot of parsing forward from safe positions anyway.

IIRC, Stefan posted a patch, which was like 10 lines long.

Could you please try it already, so we can move on to discussing the 
actual performance problems of syntax-ppss, instead of theoretical ones?

> It's two slow to use as a replacement for back_comment, in the sense I
> would like to do - in place of scanning a comment backward character by
> character, I want to use a cache (calculated by forward scanning) to
> determine the beginning of a comment.  Having to scan forward lots of
> characters (as opposed to a few) is out of the question for this
> application.

The approach X is out of the question, because my approach Y is usually 
N times faster!

Why is it out of the question? Can I say "premature optimization"?

>> So I have to wonder why the "get out of a comment" feature is used in
>> C/l mode so much that it becomes a bottleneck, and you get significant
>> improvement in performance by dropping the caching logic to C. That is,
>> of course, not a nice thing to ask considering the overall complexity of
>> CC Mode, but still.
>
>> I don't see anything comparable to 10 second waiting described in
>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884, when doing a
>> comparable operation in a 5000-line Ruby file.
>
> Here's a quick summary of how that happened:  on L1661 of config.h, C
> Mode had to scan back to the beginning of a statement.  That involved
> going back over ~1600 lines of comments and preprocessor constructs.

I went back to the revision a589e9a and rebuilt Emacs. I see the problem 
described in 22884. However, the line 1661 already is a beginning of a 
statement. It contains:

#define _DARWIN_USE_64_BIT_INODE 1

Are we talking about the same file?

> When it got to the critical comment and tried to go back over it,
> (forward-comment -1) said nil, because of that paren in column 0.  That
> paren in column 0, although in a comment, was deemed to be the start of
> a defun.  C Mode was then trying to parse "code" over a region with a
> 1600 line gap in the middle.
> Hence the 10 second delay in seeing the
> character echoed.  Paul has purged our code of parens in column 0.  But
> it would be nice not to have the restriction.

(parse-partial-sexp 1 (point)) on line 1661 takes ~0.002s here.

(parse-partial-sexp 1 (point-max)) takes just a little above that.

How do these timings translate into whole seconds of waiting after 
pressing '/'?

> My point was that it is so simple that it _could_ be written in C, and
> that without any great difficulties.

"It was so simple that I made it more complex"? I mentioned C as a 
drawback, and it is.

> Parts of it can only be written in
> C (the bits that ensure the cache is marked stale when certain
> sytax-table text properties are set/cleared when
> `inhibit-modification-hooks' is bound to non-nil).

These would have to be carefully considered, but if they make sense, 
they would have to be ported to syntax-ppss too somehow.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Eli Zaretskii]

> Date: Mon, 14 Mar 2016 08:42:46 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: acm@muc.de, emacs-devel@gnu.org
> 
> Currently, I've started using '-O2 -g3 -gdwarf-2' throughout but if
> people think that -Og or any other option is preferable, I shall do so
> (I think that I can live with a C-mode slowdown of 25%).

It depends on how much you need to debug on the C level.  -O2 is
almost impossible to debug, -Og is significantly better (but still
some functions cannot be stepped into, and some backtraces will lie).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Paul Eggert]

On 03/14/2016 12:42 AM, martin rudalics wrote:
>
> I'd really appreciate some guidelines on this.  My restrictions are two:
>
> (1) Rebuilding Emacs for the sake of debugging a particular problem is
> in general _not_ an option because it takes almost 1/2 an hour here (and
> another 1/2 hour to go back to the previous version).

Yes, it's a pain. It just now took me 9 real-time minutes to go back 
from emacs-25 (commit 4235d2d9eaa3b64d3172f6c60f1e71704795af89) to 
commit 7352c6c695db8b90b63c2601277d64a32507d2bb; this requires a rebuild 
of 'configure' and a re-run of 'configure' and a rebuild of all .o and 
many .elc and info files (including the dreaded "Processing OKURI-NASI 
entries ..." message, I guess because we switched to Unicode 9.0.0 
between those two revisions). I used 'make -j5' on a six-year-old 
four-core desktop (AMD Phenom II X4 910e) running Fedora 23 (GCC 5.3.1), 
with the default optimization of -O2.

When I make a smaller switch, it's faster to rebuild.

> Currently, I've started using '-O2 -g3 -gdwarf-2' throughout but if
> people think that -Og or any other option is preferable, I shall do so
> (I think that I can live with a C-mode slowdown of 25%). 

It might make sense to switch to -Og. This is not quite as good for 
debugging, but in my experience it's usually good enough. More 
specifically, you might try building with CFLAGS='-g3 -Og'.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Mon, Mar 14, 2016 at 06:15:13PM +0200, Dmitry Gutov wrote:
> On 03/14/2016 02:23 PM, Alan Mackenzie wrote:

> >> Any performance numbers on using this approach in C/l mode?

> > [ By "this approach" I'm taking it you mean scanning forward from safe
> > positions.]

> No, using syntax-ppss's cache. Including the syntax-ppss-last variable.

CC Mode doesn't use syntax-ppss.  It would be too much work to put it in,
particularly as that function's future is unclear.

> IIRC, Stefan posted a patch, which was like 10 lines long.

Sorry, patch for what?  I've lost the context.

> Could you please try it already, so we can move on to discussing the 
> actual performance problems of syntax-ppss, instead of theoretical ones?

It would be a lot of work.  Perhaps you might like to undertake it.

> > It's two slow to use as a replacement for back_comment, in the sense I
> > would like to do - in place of scanning a comment backward character by
> > character, I want to use a cache (calculated by forward scanning) to
> > determine the beginning of a comment.  Having to scan forward lots of
> > characters (as opposed to a few) is out of the question for this
> > application.

> The approach X is out of the question, because my approach Y is usually 
> N times faster!

> Why is it out of the question? Can I say "premature optimization"?

You could, but it would sound a bit silly.  back_comment currently works
by scanning backward a character at a time.  Each character will take
about the same time to scan (give or take a factor of, perhaps, 2) as a
character being scanned forward in parse-partial-sexp.

Compare scanning backwards over a 100 character comment using the current
back_comment with scanning forwards up to 20,000 characters to get a
parse state on the comment's end position.  CC Mode does a fair bit of
scanning backwards of comments sequentially.  Even syntax-last-pos (or
whatever it's called) won't help much here.

But if you want to do the experiment, be my guest.

> >> So I have to wonder why the "get out of a comment" feature is used in
> >> C/l mode so much that it becomes a bottleneck, and you get significant
> >> improvement in performance by dropping the caching logic to C. That is,
> >> of course, not a nice thing to ask considering the overall complexity of
> >> CC Mode, but still.

> >> I don't see anything comparable to 10 second waiting described in
> >> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22884, when doing a
> >> comparable operation in a 5000-line Ruby file.

> > Here's a quick summary of how that happened:  on L1661 of config.h, C
> > Mode had to scan back to the beginning of a statement.  That involved
> > going back over ~1600 lines of comments and preprocessor constructs.

> I went back to the revision a589e9a and rebuilt Emacs. I see the problem 
> described in 22884. However, the line 1661 already is a beginning of a 
> statement. It contains:

> #define _DARWIN_USE_64_BIT_INODE 1

> Are we talking about the same file?

Possibly.  config.h is different between different runs of ./configure.
In the file Paul was complaining about, there was a comment opener at the
start of the line.  I think it was actually the line you've cited, but
commented out.

> > When it got to the critical comment and tried to go back over it,
> > (forward-comment -1) said nil, because of that paren in column 0.  That
> > paren in column 0, although in a comment, was deemed to be the start of
> > a defun.  C Mode was then trying to parse "code" over a region with a
> > 1600 line gap in the middle.
> > Hence the 10 second delay in seeing the
> > character echoed.  Paul has purged our code of parens in column 0.  But
> > it would be nice not to have the restriction.

> (parse-partial-sexp 1 (point)) on line 1661 takes ~0.002s here.

> (parse-partial-sexp 1 (point-max)) takes just a little above that.

> How do these timings translate into whole seconds of waiting after 
> pressing '/'?

They don't.  It was CC Mode's indentation engine's scanning, not the raw
parse-partial-sexp scanning.

> > My point was that it is so simple that it _could_ be written in C, and
> > that without any great difficulties.

> "It was so simple that I made it more complex"? I mentioned C as a 
> drawback, and it is.

Lots of Emacs is written in C.  In this particular case, it is a good
idea to avoid the possible complications and pitfalls of calling lisp
from C when it is not necessary, particularly given the lack of any
involved code which would make lisp the preferred language.

> > Parts of it can only be written in
> > C (the bits that ensure the cache is marked stale when certain
> > sytax-table text properties are set/cleared when
> > `inhibit-modification-hooks' is bound to non-nil).

> These would have to be carefully considered, but if they make sense, 
> they would have to be ported to syntax-ppss too somehow.

That wouldn't be a bad idea, once the future of that function becomes
clear.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/14/2016 07:29 PM, Alan Mackenzie wrote:

> CC Mode doesn't use syntax-ppss.  It would be too much work to put it in,
> particularly as that function's future is unclear.

Could you please avoid FUD like that?

>> IIRC, Stefan posted a patch, which was like 10 lines long.
>
> Sorry, patch for what?  I've lost the context.

That one:

http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg00593.html
http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg00619.html

Any other grunt work I can assist with?

>> Could you please try it already, so we can move on to discussing the
>> actual performance problems of syntax-ppss, instead of theoretical ones?
>
> It would be a lot of work.  Perhaps you might like to undertake it.

Applying a patch and doing the measurements is a lot of work?

>> The approach X is out of the question, because my approach Y is usually
>> N times faster!
>
>> Why is it out of the question? Can I say "premature optimization"?
>
> You could, but it would sound a bit silly.

Considering the kind of arguments you provide, it's looking less and 
less silly with each message.

>> (parse-partial-sexp 1 (point)) on line 1661 takes ~0.002s here.
>
>> (parse-partial-sexp 1 (point-max)) takes just a little above that.
>
>> How do these timings translate into whole seconds of waiting after
>> pressing '/'?
>
> They don't.  It was CC Mode's indentation engine's scanning, not the raw
> parse-partial-sexp scanning.

I guess we found the problem, then. And the cache's performance is 
unlikely to change much.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Mon, Mar 14, 2016 at 07:52:27PM +0200, Dmitry Gutov wrote:
> On 03/14/2016 07:29 PM, Alan Mackenzie wrote:

> > CC Mode doesn't use syntax-ppss.  It would be too much work to put it in,
> > particularly as that function's future is unclear.

> Could you please avoid FUD like that?

There's no FUD in my last paragraph, it's an accurate representation of
the facts.  I am not prepared to spend the time needed to adapt CC Mode
to use syntax-ppss, and wouldn't be even if it worked properly.

> >> IIRC, Stefan posted a patch, which was like 10 lines long.

> > Sorry, patch for what?  I've lost the context.

> That one:

> http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg00593.html
> http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg00619.html

OK.  Stefan posted such a patch.  Now what was your point?

> Any other grunt work I can assist with?

I'm still trying to work out the point you're trying to make.

> >> Could you please try it already, so we can move on to discussing the
> >> actual performance problems of syntax-ppss, instead of theoretical ones?

> > It would be a lot of work.  Perhaps you might like to undertake it.

> Applying a patch and doing the measurements is a lot of work?

Ah, so by "it" in "please try it already" you meant "apply Stefan's
patch".  That wasn't clear.  Sorry.

Yes, applying that patch and doing measurements would be a lot of work.
You're welcome to do it if you're interested enough.

I have tried out Stefan's patch, and for the vast majority of
comments on which it actually works, I've no doubt it will work fast
enough.  Its speed isn't the issue.

Stefan's patch doesn't actually fix what I see as the root cause of bug
#22884, namely that comments get scanned backwards.  At some time in the
future this backward scanning will cause more bugs.

What has been tried is Martin Rudalics's `foo' and `bar' functions
(which repeatedly perform `c-end-of-defun' and `c-beginning-of-defun'
respectively).  I'm not the only person who has noticed a dramatic
increase in speed for the comment-cache branch in these
(unrepresentative) tests.  If you're interested, look at some of the
timings in the posts branching off of Martin's first post in this
thread.

[ .... ]

> >> How do these timings translate into whole seconds of waiting after
> >> pressing '/'?

> > They don't.  It was CC Mode's indentation engine's scanning, not the raw
> > parse-partial-sexp scanning.

> I guess we found the problem, then. And the cache's performance is 
> unlikely to change much.

It will make no difference to the scenario in #22884, no, and it was
never envisaged that it would.  What all these things have in common is
problems caused by scanning comments backwards.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

Hi Alan,

On 03/14/2016 08:46 PM, Alan Mackenzie wrote:

>>> CC Mode doesn't use syntax-ppss.  It would be too much work to put it in,
>>> particularly as that function's future is unclear.
>
>> Could you please avoid FUD like that?
>
> There's no FUD in my last paragraph, it's an accurate representation of
> the facts.

Not at all. syntax-ppss's future is rather clear, and it's here to stay, 
after years of successfully being in use.

> I am not prepared to spend the time needed to adapt CC Mode
> to use syntax-ppss, and wouldn't be even if it worked properly.

That's a separate issue. Right now we're discussing how to best 
implement "comment cache", and if it's needed at all.

> I'm still trying to work out the point you're trying to make.

Apparently, the point is that you've been offered a simpler solution for 
the same problem, and that you basically ignored it.

> Ah, so by "it" in "please try it already" you meant "apply Stefan's
> patch".  That wasn't clear.  Sorry.

Isn't what this subthread is about? The alternative approach?

> Yes, applying that patch and doing measurements would be a lot of work.
> You're welcome to do it if you're interested enough.

I might, but you'd first have to let me know what to test. If the 
comment cache patch doesn't actually help with 22884, why are we even 
discussing it?

> I have tried out Stefan's patch, and for the vast majority of
> comments on which it actually works, I've no doubt it will work fast
> enough.  Its speed isn't the issue.

Ah, so the speed advantage of using text properties is not that much of 
advantage. Correct?

> Stefan's patch doesn't actually fix what I see as the root cause of bug
> #22884, namely that comments get scanned backwards.  At some time in the
> future this backward scanning will cause more bugs.

Can you produce a test case where it fails? This time without involving 
narrowing, please.

> What has been tried is Martin Rudalics's `foo' and `bar' functions
> (which repeatedly perform `c-end-of-defun' and `c-beginning-of-defun'
> respectively).  I'm not the only person who has noticed a dramatic
> increase in speed for the comment-cache branch in these
> (unrepresentative) tests.  If you're interested, look at some of the
> timings in the posts branching off of Martin's first post in this
> thread.

Do you have any non-synthetic tests? (while (not (eobp)) (end-of-defun)) 
is not something that is likely to occur is a user's interation with 
Emacs with any regularity.

For a synthetic test, by the way, a 60%-80% increase in performance is 
not that impressive.

> It will make no difference to the scenario in #22884, no, and it was
> never envisaged that it would.  What all these things have in common is
> problems caused by scanning comments backwards.

syntax-ppss helps with avoiding that.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > Ouch!  It takes me around 3 - 4 minutes, including running ./configure.

Here I spend at least five minutes configuring.

 > I run "make clean" only in the src directory, not over the whole of
 > Emacs.  What are you doing that takes half an hour?

Nothing special, checking Lisp object types maybe.  It's an eight year
old machine I'm on.  I recall you had a slow machine too, once.

martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > It depends on how much you need to debug on the C level.  -O2 is
 > almost impossible to debug, -Og is significantly better (but still
 > some functions cannot be stepped into, and some backtraces will lie).

So far I debugged exclusively with -O0.  I'm prepared that things will
get worse in this regard.  Maybe I'll try -Og.

Thanks, martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[martin rudalics]

 > It might make sense to switch to -Og. This is not quite as good for
 > debugging, but in my experience it's usually good enough. More
 > specifically, you might try building with CFLAGS='-g3 -Og'.

OK.  I'll try that.

Thanks, martin

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Martin.

On Mon, Mar 14, 2016 at 08:41:24PM +0100, martin rudalics wrote:
>  > Ouch!  It takes me around 3 - 4 minutes, including running ./configure.

> Here I spend at least five minutes configuring.

>  > I run "make clean" only in the src directory, not over the whole of
>  > Emacs.  What are you doing that takes half an hour?

> Nothing special, checking Lisp object types maybe.  It's an eight year
> old machine I'm on.  I recall you had a slow machine too, once.

Oh yes, I remember that, too.  ;-)  But my current machine is 6 years
old.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

On Mon, Mar 14, 2016 at 09:33:25PM +0200, Dmitry Gutov wrote:
> On 03/14/2016 08:46 PM, Alan Mackenzie wrote:

> >>> CC Mode doesn't use syntax-ppss.  It would be too much work to put it in,
> >>> particularly as that function's future is unclear.

> >> Could you please avoid FUD like that?

> > There's no FUD in my last paragraph, it's an accurate representation of
> > the facts.

> Not at all. syntax-ppss's future is rather clear, and it's here to stay, 
> after years of successfully being in use.

I meant, its deficiencies need fixing, and it's not clear at this stage
how that's to be done.  I've said elsewhere what I expect to happen:
that it will be superseded by a different function with the same name.

> > I am not prepared to spend the time needed to adapt CC Mode
> > to use syntax-ppss, and wouldn't be even if it worked properly.

> That's a separate issue. Right now we're discussing how to best 
> implement "comment cache", and if it's needed at all.

Are we?  Oh, all right then, let's discuss that.

> Apparently, the point is that you've been offered a simpler solution for 
> the same problem, and that you basically ignored it.

The simpler solution is a solution to a subset of the problem, more of a
workaround than a solution in my view, as I've already said.

[ .... ]

> > Yes, applying that patch and doing measurements would be a lot of work.
> > You're welcome to do it if you're interested enough.

> I might, but you'd first have to let me know what to test.

Working out what to test is the time consuming bit.

> If the comment cache patch doesn't actually help with 22884, why are
> we even discussing it?

Because it solves the problem which was the ultimate cause of bug
#22884.

> > I have tried out Stefan's patch, and for the vast majority of
> > comments on which it actually works, I've no doubt it will work fast
> > enough.  Its speed isn't the issue.

> Ah, so the speed advantage of using text properties is not that much of 
> advantage. Correct?

It's currently unknown.  That's one reason I'd like to get it merged
with master, so as people could try it out.  As already discussed, on
certain restricted tests it's significantly faster.

> > Stefan's patch doesn't actually fix what I see as the root cause of bug
> > #22884, namely that comments get scanned backwards.  At some time in the
> > future this backward scanning will cause more bugs.

> Can you produce a test case where it fails? This time without involving 
> narrowing, please.

Of course not.  The code is twisted, with lots of special cases,
exceptions, and so on.  It even notes in its comments certain corner
cases which won't work.  It will fail again.

Anyhow, when you're testing something like this, what's the point of
only testing with nice easy cases?  The code must work correctly with or
without narrowing.

> > What has been tried is Martin Rudalics's `foo' and `bar' functions
> > (which repeatedly perform `c-end-of-defun' and `c-beginning-of-defun'
> > respectively).  I'm not the only person who has noticed a dramatic
> > increase in speed for the comment-cache branch in these
> > (unrepresentative) tests.  If you're interested, look at some of the
> > timings in the posts branching off of Martin's first post in this
> > thread.

> Do you have any non-synthetic tests? (while (not (eobp)) (end-of-defun)) 
> is not something that is likely to occur is a user's interaction with 
> Emacs with any regularity.

No, I don't have any such tests.

> For a synthetic test, by the way, a 60%-80% increase in performance is 
> not that impressive.

It's slightly more impressive than a 60% decrease in performance.  Note
that the test wasn't specially written to exhibit high performance (like
some "benchmark" tests are arranged to do).

> > It will make no difference to the scenario in #22884, no, and it was
> > never envisaged that it would.  What all these things have in common is
> > problems caused by scanning comments backwards.

> syntax-ppss helps with avoiding that.

Not the way Stefan's patch uses it, it won't.  If you look at my patch
in any detail, you'll see the former back_comment renamed to
old_back_comment.  It's due to be removed entirely.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> The simpler solution is a solution to a subset of the problem, more of a
> workaround than a solution in my view, as I've already said.

Could you make a bug report for the more general problem than just the
one exposed in bug#22884, then?


        Stefan "who has no idea what is this more general problem"

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Stefan Monnier]

> I've just timed this over xdisp.c.  A parse-partial-sexp from
> (point-min) to (point-max) took 0.031s.  A further scan, applying the
> `literal-cache' properties, took 0.054s.  That's 0.023s on a 1 MByte
> buffer.  That's not too serious.

Agreed.

> syntax-ppss throws away a lot more information that comment-cache does -
> it retains a parse state only every 20,000 characters, discarding
> everything else between those points.

That was just the original choice.  As discussed, this could be easily
changed if/when needed.  So far it's proved perfectly adequate.


        Stefan

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

On 03/14/2016 11:20 PM, Alan Mackenzie wrote:

> I meant, its deficiencies need fixing, and it's not clear at this stage
> how that's to be done.  I've said elsewhere what I expect to happen:
> that it will be superseded by a different function with the same name.

And I've replied to it already. The "deficiencies" aren't fixed yet 
because they haven't bothered anyone enough yet. The narrowing thing is 
relatively minor, compared e.g. to bug#23019 you've filed recently. 
Fixing that one would at least change the "internal data" of 
parse-partial-sexp.

>> Apparently, the point is that you've been offered a simpler solution for
>> the same problem, and that you basically ignored it.
>
> The simpler solution is a solution to a subset of the problem, more of a
> workaround than a solution in my view, as I've already said.

It does fix #22884 as described, though. I've checked.

>> [...] you'd first have to let me know what to test.
>
> Working out what to test is the time consuming bit.

The reproducible test case is a normal part of a patch submission, in 
the absence of a prior bug report. A patch looking for a problem would 
be silly.

>> If the comment cache patch doesn't actually help with 22884, why are
>> we even discussing it?
>
> Because it solves the problem which was the ultimate cause of bug
> #22884.

And yet it doesn't solve 22884? How odd.

>> Ah, so the speed advantage of using text properties is not that much of
>> advantage. Correct?
>
> It's currently unknown.  That's one reason I'd like to get it merged
> with master, so as people could try it out.  As already discussed, on
> certain restricted tests it's significantly faster.

Why don't we just collect the actual cases where CC Mode is slow, and 
test improvements against those? Instead of pushing changes to master 
and using the early adopters as one big distributed testing facility.

I mean, anticipating unknown problems sounds nice, but it's hardly the 
most important thing, given we have plenty of known ones.

>> Can you produce a test case where it fails? This time without involving
>> narrowing, please.
>
> Of course not.  The code is twisted, with lots of special cases,
> exceptions, and so on.  It even notes in its comments certain corner
> cases which won't work.  It will fail again.

High-level Lisp and abstractions are hard, let's write everything in 
assembly. Then it'll surely never fail.

If the code is as unreliable as you say, producing a problem case should 
be trivial.

> Anyhow, when you're testing something like this, what's the point of
> only testing with nice easy cases?  The code must work correctly with or
> without narrowing.

In other words, you can't give a single problem example which wouldn't 
involve narrowing? And which *would* work with text property-base cache.

> It's slightly more impressive than a 60% decrease in performance.  Note
> that the test wasn't specially written to exhibit high performance (like
> some "benchmark" tests are arranged to do).

If someone provides an arbitrary piece Lisp code, and a patch which 
speeds up it execution by say 100%, the usefulness of the patch can 
still be questioned if the code is never a hot spot in practice.

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Alan Mackenzie]

Hello, Dmitry.

First of all, I am most unhappy to read your post to which I will reply
in part.  You have trimmed so much away that the context has been totally
lost.  You have removed all mention of "syntax-ppss", leaving just lots
of "it"s.  You have removed references to CC Mode which would have
allowed other parts of your post to make sense.  This makes both of us
look like idiots arguing over trivialities.  

> And yet it [ the comment-cache branch ] doesn't solve 22884? How odd.

You have the audacity to state the falsehood that the comment-cache
branch doesn't fix bug #22884.  OF COURSE that code fixes bug #22884, and
you only needed to try it out to verify that.  Or you could have asked me
directly instead of formulating vague round-about questions which I was
foolish enough to answer directly.  Do you honestly think I'd commit a
patch for a bug which knowingly didn't fix that bug?

All I can say is that if your aim were to undermine me and discredit me
in place of honestly countering my arguments, your last post is a very
good example of how you could go about doing it.  I believe, and hope,
that it was not deliberate, and I'm asking you to be careful that you do
not write anything of that kind again to me.

OK, some details.

On Thu, Mar 17, 2016 at 02:47:10AM +0200, Dmitry Gutov wrote:
> On 03/14/2016 11:20 PM, Alan Mackenzie wrote:

[ Here "it" is syntax-ppss. ]

> > I meant, its deficiencies need fixing, and it's not clear at this stage
> > how that's to be done.  I've said elsewhere what I expect to happen:
> > that it will be superseded by a different function with the same name.

> And I've replied to it already. The "deficiencies" aren't fixed yet 
> because they haven't bothered anyone enough yet.

They bother me considerably, and they bother John, too.

> The narrowing thing is relatively minor, ....

I think your reasoning is that because you personally don't use
narrowing, it is of no importance.  I use it a lot, and one other person
has said he uses it a lot, too.  Narrowing is part of Emacs, just as much
as major modes or font locking, and "all" functionality must work with
it.

I also disagree with you that "sort of works" is good enough.  Maybe it
comes from my background in embedded systems, where the cost of failure
is inordinately high, hence rigorous testing is the norm.  The idea of a
function with undefined functionality (such as syntax-ppss) going into a
release because "nobody's complained about it" would inspire contempt and
disbelief in any of my colleagues.  Maybe that's just me.  Maybe "sort of
works" is good enough for Emacs.  I don't believe it is.  Bald tyres on a
car "sort of work" - until they don't.

> .... compared e.g. to bug#23019 you've filed recently.  Fixing that one
> would at least change the "internal data" of parse-partial-sexp.

I have a fix for it which I have not yet committed.

[ .... ]

> I mean, anticipating unknown problems sounds nice, but it's hardly the 
> most important thing, given we have plenty of known ones.

It's a matter of economy of effort.  When I come to fix a problem, I
always ask myself what was the misunderstanding or misconception or even
confusion which gave rise to it in the first place.  Where else could the
same misunderstanding lead to further problems?  If a fix, as well as
fixing the immediate bug, can also prevent further similar failures (not
necessarily identified) at the same time, it is a more economical use of
effort than just making a quick fix.  Remember, Emacs is ~30 years old,
and might well be in use for another 30 years.  That's an awful lot of
opportunity for hidden bugs to reveal themselves.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] comment-cache 223d16f 2/3: Apply `comment-depth' text properties when calling `back_comment'.

[Dmitry Gutov]

Hi Alan,

On 03/17/2016 08:47 PM, Alan Mackenzie wrote:
> You have trimmed so much away that the context has been totally
> lost.  You have removed all mention of "syntax-ppss", leaving just lots
> of "it"s.

Sorry about that? Carrying around too much context makes me 
uncomfortable when writing, or reading, emails. If anything, the lack of 
context was not a cause for misunderstanding on my side.

> You have removed references to CC Mode which would have
> allowed other parts of your post to make sense.  This makes both of us
> look like idiots arguing over trivialities.

Honestly, I'm not sure which references would have helped, and how; CC 
Mode was last mentioned directly (not in a quote) in the grand{7}parent 
of this message.

> You have the audacity to state the falsehood that the comment-cache
> branch doesn't fix bug #22884.  OF COURSE that code fixes bug #22884,

I'm sorry for misunderstanding, and hence misstating the state of 
affairs with comment-cache. But if you reread your previous email (or 
two), I believe you can agree that the fault is not entirely mine.

To be continued privately.

> [ Here "it" is syntax-ppss. ]
>
>>> I meant, its deficiencies need fixing, and it's not clear at this stage
>>> how that's to be done.  I've said elsewhere what I expect to happen:
>>> that it will be superseded by a different function with the same name.
>
>> And I've replied to it already. The "deficiencies" aren't fixed yet
>> because they haven't bothered anyone enough yet.
>
> They bother me considerably, and they bother John, too.

I have now posted the big and scary patch to the bug thread, to address 
this complicated issue.

>> The narrowing thing is relatively minor, ....
>
> I think your reasoning is that because you personally don't use
> narrowing, it is of no importance.

No, I'm saying that because the nature of the problem is clear, and the 
ways we can fix it don't affect the usage of syntax-ppss in the proposed 
patch.

> I also disagree with you that "sort of works" is good enough.

Did I claim that?

> Maybe it
> comes from my background in embedded systems, where the cost of failure
> is inordinately high, hence rigorous testing is the norm.

My background is in high level programming, where there's usually a lot 
of concepts in a system. Not entirely unlike what we have in Emacs.

The idea of someone introducing a parallel, subtly incompatible, 
implementation of an existing widely used facility, without studying it 
carefully and making sure to unify the duplicated parts (or at least 
making an honest effort toward that), is nothing to celebrate either.

> The idea of a
> function with undefined functionality (such as syntax-ppss) going into a
> release because "nobody's complained about it" would inspire contempt and
> disbelief in any of my colleagues.  Maybe that's just me.  Maybe "sort of
> works" is good enough for Emacs.  I don't believe it is.  Bald tyres on a
> car "sort of work" - until they don't.

This is volunteer work: it's not strict enough because nobody has 
invested the necessary work. Even so, we don't get to ignore or remove 
syntax-ppss just because it doesn't meet your standards, without 
providing an adequate replacement.

> [ .... ]
>
>> I mean, anticipating unknown problems sounds nice, but it's hardly the
>> most important thing, given we have plenty of known ones.
>
> It's a matter of economy of effort.  When I come to fix a problem, I
> always ask myself what was the misunderstanding or misconception or even
> confusion which gave rise to it in the first place.  Where else could the
> same misunderstanding lead to further problems?

That's a good sentiment.

> If a fix, as well as
> fixing the immediate bug, can also prevent further similar failures (not
> necessarily identified) at the same time, it is a more economical use of
> effort than just making a quick fix.  Remember, Emacs is ~30 years old,
> and might well be in use for another 30 years.  That's an awful lot of
> opportunity for hidden bugs to reveal themselves.

The stuff about not duplicating things is a matter of economy of effort, 
too.

Producing adequate test cases for patches, that justify the choices 
made, is a matter of economy of effort too, but in a different respect.

Trying to fix an issue one doesn't have a good understanding of, can 
lead to wasted effort as well. Both in the process and down the line, if 
the vaguely beneficial improvement gets merged.