Performance degradation from long lines

Performance degradation from long lines

[mithraeum]

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

A recent r/emacs thread[1] discussed the long standing[2][3]
issue of Emacs performing horribly on very long lines.

In summary, it seems that it might be possible to significantly
improve performance by altering Emacs' display engine to use simpler
techniques at cost of increased maintenance for the extra code that
will be needed, and the loss of some display capability.
I encourage reading the original thread for details.

To me it seems that the aforementioned loss of display capabilities
is not relevant to terminal Emacs, as it doesn't have those capabilities
in the first place.  So none of the objections apply except those
regarding extra maintenance and the effort to make this work.

In the case of gui Emacs, the reduction of display capability is real,
but it seems prefereable to freezing Emacs or slowing it down to a crawl.
It also seems better than, or at least a viable alternative to, opening
files with M-x find-file-literally, which loses font-lock and leaves
the buffer stuck in Fundamental mode.

In the r/emacs thread I was encouraged to post to emacs-dev to get
the input of more developers, so here I am.

I had three other ideas, which I did not yet have a chance to mention
in the thread.

First is that Emacs could have a user-configurable timeout after which
it could warn the user that it's taking an unexpectedly long time to
open a file (or find the end of a line) and ask whether to abort,
continue trying, or drop down to a mode with reduced display
capabilities (which could be either the equivalent of
find-file-literally or the above simplified display techniques, if
they're implemented).

The second idea is to use to ignore anything that would slow Emacs
down while doing a simple count of the line length, and if the line
length so far is over a certain threshold to ask the same questions as
above about aborting, continuing, or dropping down to a fail-safe
mode.

The third is to use some of Emacs' new async abilities to try to open
the file.  It might still take forever to open, but at least it shouldn't
freeze all of Emacs in the meantime.

Any thoughts?

[1] - https://old.reddit.com/r/emacs/comments/9qtpak/what_would_it_take_to_make_emacs_perform_well_on/

[2] - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=13675

[3] - https://github.com/codygman/figure-out-emacs-long-lines-issue/blob/master/figuring-out-emacs-display-issues.org

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

Re: Performance degradation from long lines

[Stefan Monnier]

> In summary, it seems that it might be possible to significantly
> improve performance by altering Emacs' display engine to use simpler
> techniques at cost of increased maintenance for the extra code that
> will be needed, and the loss of some display capability.
> I encourage reading the original thread for details.

I think it would be worthwhile to have a specific minor mode we can
enable for those cases.

> In the case of gui Emacs, the reduction of display capability is real,
> but it seems prefereable to freezing Emacs or slowing it down to a crawl.

AFAIK there are *several* unrelated features that can cause very serious
slow downs in the presence of long lines.  So we need to attack them one
by one.  A dedicated minor mode would be a good place to accumulate
the workarounds.

> I had three other ideas, which I did not yet have a chance to mention
> in the thread.

These seem to concentrate on trying to detect the slow case and partly
reduce its impact.  So, they're largely orthogonal.  I don't expect that
make-thread would make a noticeable difference, but auto-detction of
performance problems might be helpful.


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

On 2018-10-25 12:59, mithraeum wrote:
> The second idea is to use to ignore anything that would slow Emacs
> down while doing a simple count of the line length, and if the line
> length so far is over a certain threshold to ask the same questions
> as above about aborting, continuing, or dropping down to a fail-safe
> mode.

Broadly speaking, this is what so-long.el is about.  It does not help
with the redisplay code itself, but it disables elisp-visible options
which it considers may affect performance under long-line conditions.

https://savannah.nongnu.org/projects/so-long

The behaviour is automatic in accordance with how the user has
configured the library -- there is no prompting -- but C-c C-c will
revert back to the original mode if that is desired.

As it happens I've been working on this again in recent days, and
intend to release this to GNU ELPA in the near future.  I haven't
finished testing all the new changes, so there may be new bugs, but
the latest work-in-progress code can be found at:

http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip

For the example presented at https://emacs.stackexchange.com/q/598,
visiting the file one_line.json hangs Emacs 26.1 for over three
minutes (for me, using emacs -Q), whereas with so-long.el it is usable
almost instantly.  If you actually proceed with editing the file, then
performance still degrades as you get further into that enormous line;
but avoiding that initial hang seems like an obvious win.

If there was some future work on the redisplay to provide a "reduced
display capabilities" option or mode, configuring so-long to enable
that should be trivial.


-Phil

Re: Performance degradation from long lines

[Stefan Monnier]

> As it happens I've been working on this again in recent days, and
> intend to release this to GNU ELPA in the near future.  I haven't
> finished testing all the new changes, so there may be new bugs, but
> the latest work-in-progress code can be found at:
>
> http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip

Looking forward to seeing it in elpa.git,


        Stefan

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 25 Oct 2018 08:44:35 -0400
> 
> > As it happens I've been working on this again in recent days, and
> > intend to release this to GNU ELPA in the near future.  I haven't
> > finished testing all the new changes, so there may be new bugs, but
> > the latest work-in-progress code can be found at:
> >
> > http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip
> 
> Looking forward to seeing it in elpa.git,

Why not in core?

Re: Performance degradation from long lines

[Stefan Monnier]

>> > As it happens I've been working on this again in recent days, and
>> > intend to release this to GNU ELPA in the near future.  I haven't
>> > finished testing all the new changes, so there may be new bugs, but
>> > the latest work-in-progress code can be found at:
>> > http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip>> 
>> Looking forward to seeing it in elpa.git,
> Why not in core?

So users of already released Emacsen can use it.  But in core is fine as well.


        Stefan

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Wed, 24 Oct 2018 23:59:02 +0000
> From: mithraeum <mithraeum@protonmail.com>
> 
> In the case of gui Emacs, the reduction of display capability is real,
> but it seems prefereable to freezing Emacs or slowing it down to a crawl.

It is only preferable if the resulting display will be acceptable by
enough users to justify the costs of developing and maintaining the
separate "simplified" display code.

> It also seems better than, or at least a viable alternative to, opening
> files with M-x find-file-literally, which loses font-lock and leaves
> the buffer stuck in Fundamental mode.

Again, only if the result of the "simplified" display will be more
acceptable than Fundamental mode by enough users.  The significant
advantage of find-file-literally is that it already exists, and so
incurs exactly zero costs.

> In the r/emacs thread I was encouraged to post to emacs-dev to get
> the input of more developers, so here I am.

The main motivation for my request to start a discussion here was to
find out whether such a "simplified" display will be acceptable on GUI
frames by enough users.  In r/emacs thread, I described some features
that will be lost (I can repeat here if people prefer that); please
consult that if you want to make up your mind and provide opinions.
IMO, the costs for developing and maintaining such a "simplified"
display code will only be justified if enough people will consider the
resulting display both acceptable and significantly preferable to what
find-file-literally already provides.

> First is that Emacs could have a user-configurable timeout after which
> it could warn the user that it's taking an unexpectedly long time to
> open a file (or find the end of a line) and ask whether to abort,
> continue trying, or drop down to a mode with reduced display
> capabilities (which could be either the equivalent of
> find-file-literally or the above simplified display techniques, if
> they're implemented).

First, it isn't visiting the file (i.e. reading it into a buffer) that
takes time; in fact, the time to visit is almost entirely unaffected
by line length.  What takes time is the display of the initial
windowful of the file's text (and following redisplay as result of
editing).  So I guess the proposal is to have a time-out on that
initial display and on redisplay cycles.

Second, I'm not sure we need to measure the time; line length is a
reliable enough indication of expected display slowdown, so we could
simply look at the typical line lengths instead, which is much
simpler to implement.

And third, it seems like so-long mode already implements this idea, so
perhaps we should simply add it to the core and integrate it by
default.

> The second idea is to use to ignore anything that would slow Emacs
> down while doing a simple count of the line length, and if the line
> length so far is over a certain threshold to ask the same questions as
> above about aborting, continuing, or dropping down to a fail-safe
> mode.

See above.

Please note that these ideas are a kind of "band-aid": they don't
solve the root causes for the slow display, they just disable some
features which make that slow display even slower, and by that make
the critical line length larger.  But you can still create a file with
lines so long that even in Fundamental mode it will be painfully slow
to move through and edit.

> The third is to use some of Emacs' new async abilities to try to open
> the file.  It might still take forever to open, but at least it shouldn't
> freeze all of Emacs in the meantime.

There are no such abilities; the concurrency features in Emacs 26
don't allow running the display code in a separate thread.  Emacs Lisp
threads are mostly cooperative, and only one thread is allowed to run
at any given time.  And the current design of Emacs is such that
redisplay must run from a Lisp thread.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 24 Oct 2018 20:27:10 -0400
> 
> AFAIK there are *several* unrelated features that can cause very serious
> slow downs in the presence of long lines.  So we need to attack them one
> by one.

"Attack" in what sense?  If you mean something similar to what so-long
mode does, i.e. turn off those features, then I think we should simply
add that mode to core Emacs.  If you mean something else, please give
some idea regarding the "attack" approach.

Personally, I think any real solution (as opposed to band-aids) of
this issue should "attack" the root cause.  (Not that I'm opposed to
having band-aids as interim solutions as long as we don't have
anything better.)  So if someone is interested, I can describe in more
detail the root cause for the slow redisplay in these cases.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: emacs-devel@gnu.org
> Date: Thu, 25 Oct 2018 09:30:16 -0400
> 
> >> > As it happens I've been working on this again in recent days, and
> >> > intend to release this to GNU ELPA in the near future.  I haven't
> >> > finished testing all the new changes, so there may be new bugs, but
> >> > the latest work-in-progress code can be found at:
> >> > http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip>> 
> >> Looking forward to seeing it in elpa.git,
> > Why not in core?
> 
> So users of already released Emacsen can use it.  But in core is fine as well.

OK, let's do both (assuming that Phil agrees).

Re: Performance degradation from long lines

[Stefan Monnier]

>> AFAIK there are *several* unrelated features that can cause very serious
>> slow downs in the presence of long lines.  So we need to attack them one
>> by one.
> "Attack" in what sense?

All the senses you mention.

> Personally, I think any real solution (as opposed to band-aids) of
> this issue should "attack" the root cause.  (Not that I'm opposed to
> having band-aids as interim solutions as long as we don't have
> anything better.)  So if someone is interested, I can describe in more
> detail the root cause for the slow redisplay in these cases.

I'll take a real solution over band-aids any day, of course.

The band-aids like so-long.el play a useful role, tho.  Not only they
provide short-term relief in the absence of a real solution, but also
they help pin-point the different root causes.


        Stefan

Re: Performance degradation from long lines

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> First, it isn't visiting the file (i.e. reading it into a buffer) that
> takes time; in fact, the time to visit is almost entirely unaffected
> by line length.  What takes time is the display of the initial
> windowful of the file's text (and following redisplay as result of
> editing).  So I guess the proposal is to have a time-out on that
> initial display and on redisplay cycles.

BTW (I already discussed this with Eli some time ago), we already have a
solution in Emacs: longlines.el.  It's obsoleted and a bit complicated,
but something like

#+begin_src emacs-lisp
(add-hook 'find-file-hook
          (defun my-find-file-care-about-long-lines ()
            (save-excursion
              (goto-char (point-min))
              (when (and (not (eq major-mode 'image-mode))
                         (search-forward-regexp ".\\{2000\\}" 50000 t)
                         (y-or-n-p "Very long lines detected - enable longlines-mode? "))
                (require 'longlines)
                (longlines-mode +1)))))
#+end_src

solves the problem (though, using a heuristic in this case).

If someone would want write a slim version of longlines, it could be
part of a solution.


Michael.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: mithraeum <mithraeum@protonmail.com>,  emacs-devel@gnu.org
> Date: Thu, 25 Oct 2018 19:18:40 +0200
> 
> BTW (I already discussed this with Eli some time ago), we already have a
> solution in Emacs: longlines.el.

AFAIR, longlines indeed alleviates the problem but not always.

Re: Performance degradation from long lines

[Clément Pit-Claudel]

On 24/10/2018 19.59, mithraeum wrote:
> To me it seems that the aforementioned loss of display capabilities
> is not relevant to terminal Emacs, as it doesn't have those capabilities
> in the first place.  So none of the objections apply except those
> regarding extra maintenance and the effort to make this work.

I don't understand this part.  Eli listed the following capabilities:

* bold, italics, and other important face attributes
* Some characters are not covered by fixed-pitch fonts, so they cannot be displayed. The usual box-with-hex-code display of such characters will also be lost, so we will be left with displaying them as ? or somesuch.
* Character composition will also be lost, so bye-bye prettify-symbols-mode and support for some scripts that cannot be displayed without composed characters.
* Some characters (mostly from East-Asian scripts) need to be displayed at twice the width, so either you give up those scripts, or you are back to the original problem.

Many (all?) of these features are available in the terminal, too (at least the following: bold, underline, italics, characters that don't have fixed-pitched fonts, prettify-symbols-mode, and wide characters).  What did you mean by "terminal Emacs doesn't have those capabilities in the first place"?

Clément.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Thu, 25 Oct 2018 13:53:20 -0400
> 
> * bold, italics, and other important face attributes
> * Some characters are not covered by fixed-pitch fonts, so they cannot be displayed. The usual box-with-hex-code display of such characters will also be lost, so we will be left with displaying them as ? or somesuch.
> * Character composition will also be lost, so bye-bye prettify-symbols-mode and support for some scripts that cannot be displayed without composed characters.
> * Some characters (mostly from East-Asian scripts) need to be displayed at twice the width, so either you give up those scripts, or you are back to the original problem.
> 
> Many (all?) of these features are available in the terminal, too (at least the following: bold, underline, italics, characters that don't have fixed-pitched fonts, prettify-symbols-mode, and wide characters).  What did you mean by "terminal Emacs doesn't have those capabilities in the first place"?

Character attributes (bold, italics) on a TTY don't affect the size of
the characters.  The box-with-hex-code and character composition are
not available on a TTY.  Wide characters _are_ available, but they
count as 2 characters, so again they don't affect the metrics.
Prettify-symbols-mode will only work if the replacement is a single
character, not a composition of several characters in the same cell.

Re: Performance degradation from long lines

[Clément Pit-Claudel]

On 25/10/2018 15.14, Eli Zaretskii wrote:
> Character attributes (bold, italics) on a TTY don't affect the size of
> the characters.  The box-with-hex-code and character composition are
> not available on a TTY.  Wide characters _are_ available, but they
> count as 2 characters, so again they don't affect the metrics.
> Prettify-symbols-mode will only work if the replacement is a single
> character, not a composition of several characters in the same cell.

Thanks for the clarification :)

Re: Performance degradation from long lines

[Phil Sainty]

On 2018-10-26 04:08, Eli Zaretskii wrote:
>> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
>> >> Looking forward to seeing it in elpa.git,
>> > Why not in core?
>> So users of already released Emacsen can use it.  But in core is fine 
>> as well.
> OK, let's do both (assuming that Phil agrees).

Sure, I have no objections to so-long being integrated in core.

I hadn't intended to suggest this myself because it *is* a
band-aid solution -- in an ideal world the root causes would
somehow be mitigated in a cleaner fashion, and so-long would
become obsolete -- but in the interim it certainly serves a
purpose.

I'll focus on the ELPA release first in any case, but we can
discuss inclusion in core in more detail after that.


-Phil

Re: Performance degradation from long lines

[mithraeum]

On Thursday, October 25, 2018 10:18 AM, Michael Heerdegen wrote:
>
> BTW (I already discussed this with Eli some time ago), we already have a
> solution in Emacs: longlines.el. It's obsoleted and a bit complicated,
> but something like ...

I just tested this solution on a 268K minified JSON file with 272307
characters on one line, and it worked amazingly well compared to the
status quo of Emacs freezing when it encounters files with long lines
even a tiny fraction of that length.

It opened in a total of about 5 seconds, with it taking about 2 seconds
before it prompted me as to whether to open it in longline mode or not,
and about 3 more seconds to open the rest.

At that point I had a fully font-locked version of the file opened
in a JSON mode buffer.

This made me very happy, and I think I wouldn't mind living with
this solution as is until the root cause is solved, especially
if I could get it to work on compilation-mode buffers as well,
which I anticipate should be just a matter of adding it to a
compilation-mode hook.

I also tested this solution on a 663K minified JSON file with 671209
characters on one line, and this one took a little over a minute
to open because it had a couple of long lines on the first screen,
with each line measuring over 1000 characters.  This is still better
than what I'm used to, and once I was able to control-g to a useable
buffer after just 30 seconds.

There are two main issues with this solution:

First, that it's still slow when there are some long lines in the
resulting buffer.

Second, that there is no indication in resulting buffer as to where
the end of each real line is.  I think Emacs has some ways to provide
visual indicators of line endings, so I might try some of those out
and see how they work with this mode.

This still doesn't fix the root issue, but as a workaround it looks
pretty good.

Re: Performance degradation from long lines

[mithraeum]

> On 2018-10-25 12:59, mithraeum wrote:
>
> Broadly speaking, this is what so-long.el is about. It does not help
> with the redisplay code itself, but it disables elisp-visible options
> which it considers may affect performance under long-line conditions.
>
> https://savannah.nongnu.org/projects/so-long

I tried out so-long's wip branch on the same files I tested
longlines.el on, and here are the results:

First I tested it on a 268K minified JSON file with 272307 characters
on one line, and it opened in about 3 seconds.

All the text remained on one line, as in the original, but there was
no font-lock, and I was in so-long mode (which, from what I understand,
is much like Fundamental mode), not json-mode.

Then I tried opening the 663K minified JSON file with 671209
characters on one line.  It took a little over 4 minutes to open.
I then tried it again and gave up after about 7 minutes, and when
hitting control-g and escape a bunch of times did nothing, I did a
"kill -s SIGUSR2" on the emacs daemon process and was dropped in to
the debugger, from which I quit and was able to continue using Emacs
as before.

I have no idea how typical these file sizes and line lengths are compared
to what other Emacs users encounter on a regular basis.  I have a feeling
that they're larger than usual, but really I don't know.  In the cases
where the user encounters lines as long as that second test file, it
doesn't really look like a workable solution.

Also, I should mention that this second file had a ton of non-ascii,
unicode characters in it.  I'm not sure if that made a difference, but
it might.

The nice/not-nice aspect of so-long-mode is that, when it works, the
user is still dealing with the original line lengths in the file.
This is advantageous when one wants to see the file as it was.

It's not so great, though, as a lot of the typical editing commands
really aren't geared towards working with lines that long, so it's
much more convenient to work with lines the length provided by
longlines.el

I think in a realistic scenario, when I run across such minified
files, I'd probably want to run them through a beautifier, then
switch them to a mode that supports font-lock (like the native
json-mode), and after I'm done editing re-minify them if need be
(maybe outside of Emacs, or in Emacs but after re-enabling
so-long-mode or longlines.el).

For my own use cases, however, I think most of the time I probably
wouldn't be editing these super long files in the first place,
but just want to view their beautified versions with font-lock on.

In any event, it's great to have both of these options in Emacs
already.  Their presence and usefulness should be more widely known.

If longlines.el needs some updating to make it non-obsolete or
to deal with its own issues, that could be helpful too.

Re: Performance degradation from long lines

[Phil Sainty]

On 2018-10-26 14:46, mithraeum wrote:
> I tried out so-long's wip branch on the same files I tested
> longlines.el on, and here are the results:

Thanks for testing.

> Then I tried opening the 663K minified JSON file with 671209
> characters on one line.  It took a little over 4 minutes to open.

There are some fairly conservative default thresholds configured
at present.  In particular, `so-long-max-lines' is currently set
to 5, meaning that, following any initial comments/whitespace,
only the first 5 lines will be checked.

I would guess that in the case of this second file, the long lines
did not occur until after that default threshold?

Check M-x customize-group so-long for config options.

I am completely open to changing the default values.  I have never
had a firm idea of what they should be set to by default.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

On 2018-10-26 14:46, mithraeum wrote:
> Then I tried opening the 663K minified JSON file with 671209
> characters on one line.

Just for comparison, the test file I've mostly been using is
the one from https://emacs.stackexchange.com/q/598 which is a
18M single-line JSON file with 18922366 characters on one line,
which opens in about a second on my machine when so-long has
been enabled.


-Phil

Re: Performance degradation from long lines

[mithraeum]

On Thursday, October 25, 2018 7:39 PM, Phil Sainty wrote:
>
> I would guess that in the case of this second file, the long lines
> did not occur until after that default threshold?

No.  The entire file consisted of one single line.

Re: Performance degradation from long lines

[Phil Sainty]

On 2018-10-26 15:58, mithraeum wrote:
> No.  The entire file consisted of one single line.

Curious.  The only other idea that springs to mind is that
Emacs thought the line was a comment, in which case disabling
`so-long-skip-leading-comments' would make a difference.
Otherwise it certainly sounds like a bug; presumably with
the new changes to `so-long-line-detected-p'.

If you're able to send me the file, could you do so?


thanks,
-Phil

Re: Performance degradation from long lines

[mithraeum]

On Thursday, October 25, 2018 9:43 PM, Phil Sainty wrote:
>
> If you're able to send me the file, could you do so?

No problem.  I'll send a copy directly to you so I don't spam the list with it.

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Fri, 26 Oct 2018 11:34:22 +1300
> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: emacs-devel@gnu.org
> 
> > OK, let's do both (assuming that Phil agrees).
> 
> Sure, I have no objections to so-long being integrated in core.

Thanks.

> I'll focus on the ELPA release first in any case, but we can
> discuss inclusion in core in more detail after that.

Please initiate the discussion about inclusion in core once you are
done with putting the package on ELPA.

Re: Performance degradation from long lines

[mithraeum]

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

I was just starting to do an exercise on a programming practice
website, and happened to paste some example data from the site
in to my Emacs buffer, without thinking anything of it.

It happened to be over 2000 characters on one line and Emacs froze
for a while.  Fortunately, it wasn't much more, or the freeze
would have been even longer.

Somehow, I think we have to find a solution that works when
any long line has to be displayed in a buffer, not just when
we open a file.

By the way, I tried adding the longlines.el workaround function to my
compilation-mode-hook and running a program that printed 10000
characters on a single line to the compilation-mode buffer, and
the longlines.el workaround did not help.  Emacs still froze.

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

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Fri, 26 Oct 2018 00:59:11 +0000
> From: mithraeum <mithraeum@protonmail.com>
> 
> There are two main issues with this solution:
> 
> First, that it's still slow when there are some long lines in the
> resulting buffer.
> 
> Second, that there is no indication in resulting buffer as to where
> the end of each real line is.  I think Emacs has some ways to provide
> visual indicators of line endings, so I might try some of those out
> and see how they work with this mode.
> 
> This still doesn't fix the root issue, but as a workaround it looks
> pretty good.

Thanks.

So I guess we should ask volunteers to strip down longlines and then
enhance it to address the above issues.  Then we can bring it back
from its "obsolescence" and document as an interim solution for these
situations.

Re: Performance degradation from long lines

[Ihor Radchenko]

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


 > > It also seems better than, or at least a viable alternative to, opening
 > > files with M-x find-file-literally, which loses font-lock and leaves
 > > the buffer stuck in Fundamental mode.

 > Again, only if the result of the "simplified" display will be more
 > acceptable than Fundamental mode by enough users.  The significant
 > advantage of find-file-literally is that it already exists, and so
 > incurs exactly zero costs.

I would like to point out that the long line problem also exists for
non-file buffers.
Specifically, evaluating debug-on-entry for org-mode buffer parsing
functions often leads to ridiculously long lines (pretty much the whole
buffer is transformed into a single line), which takes forever to
display.
find-file-literally does not help with this problem at all.

Best,
Ihor


Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Wed, 24 Oct 2018 23:59:02 +0000
>> From: mithraeum <mithraeum@protonmail.com>
>> 
>> In the case of gui Emacs, the reduction of display capability is real,
>> but it seems prefereable to freezing Emacs or slowing it down to a crawl.
>
> It is only preferable if the resulting display will be acceptable by
> enough users to justify the costs of developing and maintaining the
> separate "simplified" display code.
>
>> It also seems better than, or at least a viable alternative to, opening
>> files with M-x find-file-literally, which loses font-lock and leaves
>> the buffer stuck in Fundamental mode.
>
> Again, only if the result of the "simplified" display will be more
> acceptable than Fundamental mode by enough users.  The significant
> advantage of find-file-literally is that it already exists, and so
> incurs exactly zero costs.
>
>> In the r/emacs thread I was encouraged to post to emacs-dev to get
>> the input of more developers, so here I am.
>
> The main motivation for my request to start a discussion here was to
> find out whether such a "simplified" display will be acceptable on GUI
> frames by enough users.  In r/emacs thread, I described some features
> that will be lost (I can repeat here if people prefer that); please
> consult that if you want to make up your mind and provide opinions.
> IMO, the costs for developing and maintaining such a "simplified"
> display code will only be justified if enough people will consider the
> resulting display both acceptable and significantly preferable to what
> find-file-literally already provides.
>
>> First is that Emacs could have a user-configurable timeout after which
>> it could warn the user that it's taking an unexpectedly long time to
>> open a file (or find the end of a line) and ask whether to abort,
>> continue trying, or drop down to a mode with reduced display
>> capabilities (which could be either the equivalent of
>> find-file-literally or the above simplified display techniques, if
>> they're implemented).
>
> First, it isn't visiting the file (i.e. reading it into a buffer) that
> takes time; in fact, the time to visit is almost entirely unaffected
> by line length.  What takes time is the display of the initial
> windowful of the file's text (and following redisplay as result of
> editing).  So I guess the proposal is to have a time-out on that
> initial display and on redisplay cycles.
>
> Second, I'm not sure we need to measure the time; line length is a
> reliable enough indication of expected display slowdown, so we could
> simply look at the typical line lengths instead, which is much
> simpler to implement.
>
> And third, it seems like so-long mode already implements this idea, so
> perhaps we should simply add it to the core and integrate it by
> default.
>
>> The second idea is to use to ignore anything that would slow Emacs
>> down while doing a simple count of the line length, and if the line
>> length so far is over a certain threshold to ask the same questions as
>> above about aborting, continuing, or dropping down to a fail-safe
>> mode.
>
> See above.
>
> Please note that these ideas are a kind of "band-aid": they don't
> solve the root causes for the slow display, they just disable some
> features which make that slow display even slower, and by that make
> the critical line length larger.  But you can still create a file with
> lines so long that even in Fundamental mode it will be painfully slow
> to move through and edit.
>
>> The third is to use some of Emacs' new async abilities to try to open
>> the file.  It might still take forever to open, but at least it shouldn't
>> freeze all of Emacs in the meantime.
>
> There are no such abilities; the concurrency features in Emacs 26
> don't allow running the display code in a separate thread.  Emacs Lisp
> threads are mostly cooperative, and only one thread is allowed to run
> at any given time.  And the current design of Emacs is such that
> redisplay must run from a Lisp thread.
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Fri, 26 Oct 2018 06:40:25 +0000
> From: mithraeum <mithraeum@protonmail.com>
> 
> I was just starting to do an exercise on a programming practice
> website, and happened to paste some example data from the site
> in to my Emacs buffer, without thinking anything of it.
> 
> It happened to be over 2000 characters on one line and Emacs froze
> for a while.  Fortunately, it wasn't much more, or the freeze
> would have been even longer.

In what major mode did that happen?  I don't think I ever saw any
problems with 2000-character lines, it begins happening for me only
when lines are much longer, like tens of thousands of characters.

Also, what happens if you use switch to Fundamental mode in that case?

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 26 Oct 2018 15:00:09 +0800
> 
> I would like to point out that the long line problem also exists for
> non-file buffers.
> Specifically, evaluating debug-on-entry for org-mode buffer parsing
> functions often leads to ridiculously long lines (pretty much the whole
> buffer is transformed into a single line), which takes forever to
> display.
> find-file-literally does not help with this problem at all.

The equivalent of find-file-literally for non-file buffers is use a
unibyte buffer in Fundamental mode.

Re: Performance degradation from long lines

[Ihor Radchenko]

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

Maybe I miss something, but it is not trivial thing to do.
I just tried to switch debug buffer to fundamental mode and run
debugger-step-through.
It lead to the buffer changing back to debug-mode.

Not running debugger commands in debugger buffer would be useless.

I imagine that similar issues may exist for other types of non-file buffers.


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 26 Oct 2018 15:00:09 +0800
>> 
>> I would like to point out that the long line problem also exists for
>> non-file buffers.
>> Specifically, evaluating debug-on-entry for org-mode buffer parsing
>> functions often leads to ridiculously long lines (pretty much the whole
>> buffer is transformed into a single line), which takes forever to
>> display.
>> find-file-literally does not help with this problem at all.
>
> The equivalent of find-file-literally for non-file buffers is use a
> unibyte buffer in Fundamental mode.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Performance degradation from long lines

[mithraeum]

On Friday, October 26, 2018 12:26 AM, Eli Zaretskii wrote:
>
> In what major mode did that happen? I don't think I ever saw any
> problems with 2000-character lines, it begins happening for me only
> when lines are much longer, like tens of thousands of characters.

It happened in python-mode.

Also, I have to confess that I'm on an old, slow laptop.

Perhaps you're seeing better performance simply because you have
a more powerful system.

> Also, what happens if you use switch to Fundamental mode in that case?

It pasted in instantly.  There was no slowdown of Emacs at all.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: mithraeum@protonmail.com, emacs-devel@gnu.org
> Date: Fri, 26 Oct 2018 15:36:53 +0800
> 
> Maybe I miss something, but it is not trivial thing to do.

Not in all cases, but certainly in some.

How about asking Org developers to do something about these cases,
like not using the entire buffer as a literal string argument, to
avoid such problems?

Re: Performance degradation from long lines

[mithraeum]

On Friday, October 26, 2018 12:36 AM, Ihor Radchenko <yantar92@gmail.com> wrote:
>
> Maybe I miss something, but it is not trivial thing to do.
> I just tried to switch debug buffer to fundamental mode and run
> debugger-step-through.
> It lead to the buffer changing back to debug-mode.
>
> Not running debugger commands in debugger buffer would be useless.
>
> I imagine that similar issues may exist for other types of non-file buffers.

Yes.  This is a serious concern for the various terminal emulation
modes within Emacs.

While using these modes, there are plenty of dangerous shell commands which
may output huge amounts of output, possibly all on one line.

This is the main reason why I avoid using those modes.  I don't want to
risk freezing my Emacs session by accidentally running a command that
generates a mountain of output.

This is not a concern in a real terminal, but it is in Emacs.

Also, turning something like term-mode to Fundamental mode would likely
face similar problems to what you encountered.

I just tried it and, not surprisingly, it completely broke it.
It made it read only, and I'm getting errors like "error in process
filter: Symbol’s value as variable is void: term-pending-delete-marker"
and even changing it to read-write typing in to it and hitting RETURN
doesn't result in a command being executed.

So Fundamental mode alone is not a solution for these modes.

Re: Performance degradation from long lines

[Ihor Radchenko]

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

> Not in all cases, but certainly in some.

I can say the same about the whole long lines problem.

> How about asking Org developers to do something about these cases,
> like not using the entire buffer as a literal string argument, to
> avoid such problems?

Well, in org-mode, the buffer is parsed into s-exp containing all the
buffer elements and the associated text, which can be even longer that
the buffer string itself.
So, the long line is what the parser returns.
This approach is a part of the core implementation of the org-mode and
cannot be changed easily. 

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@gmail.com>
>> Cc: mithraeum@protonmail.com, emacs-devel@gnu.org
>> Date: Fri, 26 Oct 2018 15:36:53 +0800
>> 
>> Maybe I miss something, but it is not trivial thing to do.
>
> Not in all cases, but certainly in some.
>
> How about asking Org developers to do something about these cases,
> like not using the entire buffer as a literal string argument, to
> avoid such problems?
>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Fri, 26 Oct 2018 07:47:56 +0000
> From: mithraeum <mithraeum@protonmail.com>
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> On Friday, October 26, 2018 12:26 AM, Eli Zaretskii wrote:
> >
> > In what major mode did that happen? I don't think I ever saw any
> > problems with 2000-character lines, it begins happening for me only
> > when lines are much longer, like tens of thousands of characters.
> 
> It happened in python-mode.

So I guess some features of python-mode are "guilty" for such a
tremendous slow-down.  It would be interesting to know which ones.

Alternatively, it could be some minor mode you have enabled.

> Also, I have to confess that I'm on an old, slow laptop.

How slow?  Can you tell its CPU spec?

> Perhaps you're seeing better performance simply because you have
> a more powerful system.

My main development machine is a 6.5 year old box with Core i7-2600
CPU clocked at 3.4 GHz.

> > Also, what happens if you use switch to Fundamental mode in that case?
> 
> It pasted in instantly.  There was no slowdown of Emacs at all.

I guess that means the modes I use routinely are not as expensive as
python-mode.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: mithraeum@protonmail.com, emacs-devel@gnu.org
> Date: Fri, 26 Oct 2018 16:05:25 +0800
> 
> > Not in all cases, but certainly in some.
> 
> I can say the same about the whole long lines problem.

Would it help to "say the same"?

I'm trying to come up with ideas to help people get bearable
performance in practical use cases.  I can shut up if people don't
want to hear about partial solutions that don't work in all cases.

Assigning blame and/or venting steam will not help us make any
progress in this area, you know.

> > How about asking Org developers to do something about these cases,
> > like not using the entire buffer as a literal string argument, to
> > avoid such problems?
> 
> Well, in org-mode, the buffer is parsed into s-exp containing all the
> buffer elements and the associated text, which can be even longer that
> the buffer string itself.
> So, the long line is what the parser returns.
> This approach is a part of the core implementation of the org-mode and
> cannot be changed easily. 

Then perhaps we could find a solution specific to backtrace buffers,
like displaying ellipsis or a special button instead of too-long
lines.

Re: Performance degradation from long lines

[mithraeum]

On Friday, October 26, 2018 1:30 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Also, I have to confess that I'm on an old, slow laptop.
>
> How slow? Can you tell its CPU spec?

About 5 or 6 years old.  When I got it, it was the cheapest, slowest
laptop in a big box electronics store.  It has an AMD E1-1500,
which is a 2-core CPU which runs at 1477.298 MHz
according to /proc/cpuinfo

Re: Performance degradation from long lines

[Ihor Radchenko]

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

       > Would it help to "say the same"?

       > I'm trying to come up with ideas to help people get bearable
       > performance in practical use cases.  I can shut up if people don't
       > want to hear about partial solutions that don't work in all cases.

       > Assigning blame and/or venting steam will not help us make any
       > progress in this area, you know.

Sorry, I meant no offence there.
I just wanted to say that, if possible, we should try to find
workarounds for non-file buffers.
Ideally, these workarounds should work in all the non-file buffers
without a need to address the issue for each buffer type separately.

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@gmail.com>
>> Cc: mithraeum@protonmail.com, emacs-devel@gnu.org
>> Date: Fri, 26 Oct 2018 16:05:25 +0800
>> 
>> > Not in all cases, but certainly in some.
>> 
>> I can say the same about the whole long lines problem.
>
> Would it help to "say the same"?
>
> I'm trying to come up with ideas to help people get bearable
> performance in practical use cases.  I can shut up if people don't
> want to hear about partial solutions that don't work in all cases.
>
> Assigning blame and/or venting steam will not help us make any
> progress in this area, you know.
>
>> > How about asking Org developers to do something about these cases,
>> > like not using the entire buffer as a literal string argument, to
>> > avoid such problems?
>> 
>> Well, in org-mode, the buffer is parsed into s-exp containing all the
>> buffer elements and the associated text, which can be even longer that
>> the buffer string itself.
>> So, the long line is what the parser returns.
>> This approach is a part of the core implementation of the org-mode and
>> cannot be changed easily. 
>
> Then perhaps we could find a solution specific to backtrace buffers,
> like displaying ellipsis or a special button instead of too-long
> lines.

-- 
Ihor Radchenko,
PhD Student
Singapore University of Technology and Design,
8 Somapah Road Singapore 487372
Email: yantar92@gmail.com, ihor_radchenko@mymail.sutd.edu.sg
Tel: +6584017977

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Fri, 26 Oct 2018 08:56:21 +0000
> From: mithraeum <mithraeum@protonmail.com>
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> > How slow? Can you tell its CPU spec?
> 
> About 5 or 6 years old.  When I got it, it was the cheapest, slowest
> laptop in a big box electronics store.  It has an AMD E1-1500,
> which is a 2-core CPU which runs at 1477.298 MHz
> according to /proc/cpuinfo

That's still about 57% of what I have here, so 2000 characters still
sounds too few to trigger such slowdown, and I therefore still think
it's something in python-mode or in some minor mode you enable.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: mithraeum@protonmail.com, emacs-devel@gnu.org
> Date: Fri, 26 Oct 2018 16:58:09 +0800
> 
> Sorry, I meant no offence there.

None taken.

> I just wanted to say that, if possible, we should try to find
> workarounds for non-file buffers.

Agreed.

> Ideally, these workarounds should work in all the non-file buffers
> without a need to address the issue for each buffer type separately.

I suggest to file a bug report specifically about backtrace buffers
with long lines, preferably with an example.  I think it is an
important special use case for which we could implement a good
solution that is relatively simple and effective.

Re: Performance degradation from long lines

[Noam Postavsky]

On Fri, 26 Oct 2018 at 05:08, Eli Zaretskii <eliz@gnu.org> wrote:

> I suggest to file a bug report specifically about backtrace buffers
> with long lines, preferably with an example.  I think it is an
> important special use case for which we could implement a good
> solution that is relatively simple and effective.

I'd just like to point out that getting backtrace buffers with long
lines in 26.1 is likely due to Bug#31559 which is fixed on the
emacs-26 branch (so the backtrace expressions should be abbreviated
sufficiently that the long lines problem is not triggered).

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

Re: Performance degradation from long lines

[mithraeum]

On Friday, October 26, 2018 1:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> Then perhaps we could find a solution specific to backtrace buffers,
> like displaying ellipsis or a special button instead of too-long
> lines.

Could lines in backtrace buffers / debug-mode, compilation-mode, and
the various Emacs term modes be wrapped like longlines.el wraps them,
but do it on-the-fly as the lines come in instead of just once for
the entire buffer?

If so, that might be a decent workaround.

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Fri, 26 Oct 2018 05:46:35 -0400
> Cc: yantar92@gmail.com, mithraeum@protonmail.com, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> On Fri, 26 Oct 2018 at 05:08, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I suggest to file a bug report specifically about backtrace buffers
> > with long lines, preferably with an example.  I think it is an
> > important special use case for which we could implement a good
> > solution that is relatively simple and effective.
> 
> I'd just like to point out that getting backtrace buffers with long
> lines in 26.1 is likely due to Bug#31559 which is fixed on the
> emacs-26 branch (so the backtrace expressions should be abbreviated
> sufficiently that the long lines problem is not triggered).
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31559

Right.  If we have a specific example of the problem, we could verify
that the emacs-26 branch already solves it.

Re: Performance degradation from long lines

[Stefan Monnier]

> I'll focus on the ELPA release first in any case, but we can
> discuss inclusion in core in more detail after that.

AFAIC putting it in "both" means adding it to emacs.git's master branch
and using elpa.gnu.org's ability to distribute packages built from the
their emacs.git source to also make a GNU ELPA package out of it.

So if it goes to emacs.git, there's no point putting it in elpa.git
first (and I don't want to have it in both places).


        Stefan

Re: Performance degradation from long lines

[Stefan Monnier]

> Ideally, these workarounds should work in all the non-file buffers
> without a need to address the issue for each buffer type separately.

Of course.  But I doubt such an ideal solution exists :-(


        Stefan

Re: Performance degradation from long lines

[Stefan Monnier]

>> Then I tried opening the 663K minified JSON file with 671209
>> characters on one line.
> Just for comparison, the test file I've mostly been using is
> the one from https://emacs.stackexchange.com/q/598 which is a
> 18M single-line JSON file with 18922366 characters on one line,
> which opens in about a second on my machine when so-long has
> been enabled.

I think Mithraeum's point is a good one: it would be worthwhile also to
see what it is that the user wants to do with those kinds of files,
because the slowness is but one of the problems.

E.g. if you take your above example, maybe it does take just a second to
open in so-long-mode, but what happens when you do M-> ?

Is the "show the first screen" use case the main one (after which the
user will see that the file format is not the right one, will change it
(e.g. by pretty-printing it), and try again), or do we also want to care
about what happens if the user then goes on to navigate in this buffer?


        Stefan

Re: Performance degradation from long lines

[Stefan Monnier]

> About 5 or 6 years old.  When I got it, it was the cheapest, slowest
> laptop in a big box electronics store.  It has an AMD E1-1500,
> which is a 2-core CPU which runs at 1477.298 MHz
> according to /proc/cpuinfo

Ah, yes, it's basically the same as the E350, i.e. better than the
original Atom used in netbooks but not very much.

I used to use such a system (a Zotac E350) and my basic performance tests
on it puts it at a factor of 2 slower than my Mac Mini with a Core 2 duo
T7600 at 2.33GHz (and about 3 times as fast as the Allwinner A20
in my BananaPi).

So it is indeed a fairly poor performer (most of my current machines use
processors comparable to that Core 2 duo, but I think they're
considered pretty slow nowadays).

Still, Emacs should be usable on such systems.


        Stefan

Re: Performance degradation from long lines

[Alexander Shukaev]

Modes like `so-long' are not useful at all to resolve these issues 
(because, example it does not prevent the initial activation of the 
offending major mode, but rather only disables it afterwards).  The only 
way I found to reliably mitigate the problem in an semi-automated manner 
consists of two parts.  First,

(require 'longlines)
;;
(defcustom global-longlines-threshold
   line-number-display-limit-width
   "\
Maximum line length permitted before applying `longlines-mode'.

See `global-longlines-p'."
   :group 'longlines
   :type 'integer)
;;
(defun global-longlines-p
     (&optional buffer)
   "\
Test all lines of buffer BUFFER for `global-longlines-threshold'.

Return t if `global-longlines-threshold' is exceeded."
   (with-current-buffer (or buffer (current-buffer))
     (when (integerp global-longlines-threshold)
       (save-restriction
         (widen)
         (save-excursion
           (goto-char (point-min))
           (let ((inhibit-field-text-motion t))
             (catch 'break
               (while (not (eobp))
                 (when (> (- (line-end-position 1) (point))
                          global-longlines-threshold)
                   (throw 'break t))
                 (forward-line 1)))))))))
;;
;;;###autoload
(defun turn-on-longlines-mode-maybe ()
   (interactive)
   (when (global-longlines-p)
     (longlines-mode)))
;;
;;;###autoload
(define-globalized-minor-mode global-longlines-mode longlines-mode
   turn-on-longlines-mode-maybe
   :group 'longlines)
;;
(provide 'global-longlines)

and then configure

(add-hook 'after-init-hook #'global-longlines-mode)

Secondly,

(defgroup inhibit-set-auto-mode
   nil
   "Inhibit `set-auto-mode'."
   :group 'files
   :prefix 'inhibit-set-auto-mode-)
;;
(defcustom inhibit-set-auto-mode-functions
   nil
   "\
List of functions to be called to try to inhibit `set-auto-mode'.

Only used by `inhibit-set-auto-mode'.  If one of them returns
non-nil, inhibit `set-auto-mode' and the rest are not called."
   :group 'inhibit-set-auto-mode
   :type 'hook)
;;
(defun inhibit-set-auto-mode--around-advice
     (function &rest ...)
   (if (run-hook-with-args-until-success 'inhibit-set-auto-mode-functions)
       (let (enable-local-variables
             interpreter-mode-alist
             magic-mode-alist
             auto-mode-alist
             magic-fallback-mode-alist)
         (apply function ...))
     (apply function ...)))
;;
;;;###autoload
(define-minor-mode inhibit-set-auto-mode
   "\
Toggle inhibiting `set-auto-mode'.

With a prefix argument ARG, enable the mode if ARG is positive,
and disable it otherwise.  If called from Lisp, enable the mode
if ARG is omitted or nil.

See `inhibit-set-auto-mode-functions'."
   :group 'inhibit-set-auto-mode
   :global t
   (if inhibit-set-auto-mode
       (advice-add #'set-auto-mode
                   :around
                   #'inhibit-set-auto-mode--around-advice
                   '((depth . 100)))
     (advice-remove #'set-auto-mode
                    #'inhibit-set-auto-mode--around-advice)))
;;
(provide 'inhibit-set-auto-mode)

and then configure

(add-hook 'after-init-hook #'inhibit-set-auto-mode)

(defun inhibit-set-auto-mode-line-number-display-limit-function ()
   (and (integerp line-number-display-limit)
        (> (buffer-size) line-number-display-limit)))
(add-hook 'inhibit-set-auto-mode-functions
           #'inhibit-set-auto-mode-line-number-display-limit-function)

Regards,
Alexander

Re: Performance degradation from long lines

[Gemini Lasswell]

Ihor Radchenko <yantar92@gmail.com> writes:

> I would like to point out that the long line problem also exists for
> non-file buffers.
> Specifically, evaluating debug-on-entry for org-mode buffer parsing
> functions often leads to ridiculously long lines (pretty much the whole
> buffer is transformed into a single line), which takes forever to
> display.
> find-file-literally does not help with this problem at all.

Have you tried debugging org-mode with an Emacs built from master?  I've
made several improvements to the *Backtrace* buffers in master,
including doing a much better job of abbreviating the printing of very
large data structures, so that lines don't get so ridiculously long.

Re: Performance degradation from long lines

[Stefan Monnier]

>           (let ((inhibit-field-text-motion t))
>             (catch 'break
>               (while (not (eobp))
>                 (when (> (- (line-end-position 1) (point))
>                          global-longlines-threshold)
>                   (throw 'break t))
>                 (forward-line 1)))))))))

I think you can speed this up with the code below (I'd expect a speed up
of about a factor 2 since (line-end-position 1) does about the same as
(forward-line 1)):

    (catch 'break
      (while
          (unless (eobp)
            (or (> (+ (point) global-longlines-threshold)
                      (progn (forward-line 1) (point)))
                (throw 'break t)))))

> (defun inhibit-set-auto-mode-line-number-display-limit-function ()
>   (and (integerp line-number-display-limit)
>        (> (buffer-size) line-number-display-limit)))
> (add-hook 'inhibit-set-auto-mode-functions
>           #'inhibit-set-auto-mode-line-number-display-limit-function)

Hmm... there seems to be a discrepancy between the identifier and the
code: it seems unrelated to line-number(s).  Maybe some left-over from
a previous version of the code?


        Stefan

Re: Performance degradation from long lines

[Alexander Shukaev]

On 10/26/18 6:18 PM, Stefan Monnier wrote:
>>            (let ((inhibit-field-text-motion t))
>>              (catch 'break
>>                (while (not (eobp))
>>                  (when (> (- (line-end-position 1) (point))
>>                           global-longlines-threshold)
>>                    (throw 'break t))
>>                  (forward-line 1)))))))))
> 
> I think you can speed this up with the code below (I'd expect a speed up
> of about a factor 2 since (line-end-position 1) does about the same as
> (forward-line 1)):
> 
>      (catch 'break
>        (while
>            (unless (eobp)
>              (or (> (+ (point) global-longlines-threshold)
>                        (progn (forward-line 1) (point)))
>                  (throw 'break t)))))

Thanks for the tip!

> 
>> (defun inhibit-set-auto-mode-line-number-display-limit-function ()
>>    (and (integerp line-number-display-limit)
>>         (> (buffer-size) line-number-display-limit)))
>> (add-hook 'inhibit-set-auto-mode-functions
>>            #'inhibit-set-auto-mode-line-number-display-limit-function)
> 
> Hmm... there seems to be a discrepancy between the identifier and the
> code: it seems unrelated to line-number(s).  Maybe some left-over from
> a previous version of the code?
> 
> 
>          Stefan
> 
> 

You are right, sorry about `line-number-display-limit', it should of 
course be something more reasonable, e.g. (8 * 1024 * 1024) for 8GiG cap.

Re: Performance degradation from long lines

[Stefan Monnier]

> You are right, sorry about `line-number-display-limit', it should of course
> be something more reasonable, e.g. (8 * 1024 * 1024) for 8GiG cap.

Then maybe it should simply re-use large-file-warning-threshold.


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

On 27/10/18 4:34 AM, Alexander Shukaev wrote:
> Modes like `so-long' are not useful at all to resolve these issues
> (because, example it does not prevent the initial activation of the
> offending major mode, but rather only disables it afterwards).

In my experience, the initial activation of the major mode is not
part of the problem.  Could you point out an example where that is
a contributing factor?

Conversely, knowing which mode was selected is not only beneficial,
but (I think) crucial, as we *need* to filter out cases where it's
100% fine for lines to be extremely long (any binary file format,
for example).  so-long.el tests the original major mode against
`so-long-target-modes' to perform that filtering.

As such I don't think bypassing `set-auto-mode' is an option.
Or at minimum you need to know what that mode was, even if it is
never called.

e.g. I tried opening a 50M bzipped tar file with your code several
minutes ago, and I'm still waiting for that instance to finish
thinking about it.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

On 26/10/18 2:46 PM, mithraeum wrote:
> First I tested it on a 268K minified JSON file with 272307 characters
> on one line, and it opened in about 3 seconds.
> 
> Then I tried opening the 663K minified JSON file with 671209
> characters on one line.  It took a little over 4 minutes to open.

We've established the cause of this, off-list.  It turned out to be a
minor mode `dtrt-indent-mode' https://github.com/jscheid/dtrt-indent

Once that was added to `so-long-minor-modes' the test file opened
quickly.


I'll include my general comments from our discussion:

>> It's definitely possible that some minor mode or user option is
>> responsible. I'm targeting a handful of things by default, but
>> there will unquestionably be others which could/should be added.
>>
>> I don't think a whitelist of "things which are safe" is at all
>> practical, though; so a blacklist of "things to disable" seemed
>> like the only practical approach, and therefore it's always
>> possible for any given user to be using something which would
>> exacerbate the problem, but which so-long doesn't know about.
>>
>> I was unsure from the outset whether it was appropriate for a GNU
>> ELPA package to make explicit reference to packages which were not
>> available in core or through GNU ELPA, so I'm already aware of a
>> few modes which might potentially make a difference which aren't
>> in the default config. These are the additional modes that I
>> listed on the Emacs Wiki page when I was originally working on
>> this library:
>>
>> diff-hl-mode
>> diff-hl-amend-mode
>> diff-hl-flydiff-mode
>> hl-sexp-mode
>> idle-highlight-mode
>> rainbow-delimiters-mode

Obviously dtrt-indent-mode can be added to that list.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

On 27/10/18 1:48 AM, Stefan Monnier wrote:
> AFAIC putting it in "both" means adding it to emacs.git's master branch
> and using elpa.gnu.org's ability to distribute packages built from the
> their emacs.git source to also make a GNU ELPA package out of it.
> 
> So if it goes to emacs.git, there's no point putting it in elpa.git
> first (and I don't want to have it in both places).

I'm imagining that a version in core for Emacs 27+ might differ slightly
from the GNU ELPA package.

The first thing which springs to mind is `so-long-check-header-modes'
which is a verbatim copy of some code from `set-auto-mode' which I
wasn't able to access the results of otherwise:

http://git.savannah.nongnu.org/cgit/so-long.git/tree/so-long.el?h=641f8eaaa#n464

Adding so-long.el to core would mean we could potentially tweak things
in `set-auto-mode' such that so-long didn't need to duplicate any code;
but that improved version of so-long.el couldn't be used with earlier
versions of Emacs.

In addition to that, I'm uncertain where the current discussions are
going to lead to wrt core improvements, as there are a number of
different ideas being thrown around, and I'm wondering whether we'll
end up amalgamating several things together.

So my current feeling is that I should put the existing version into
elpa.git separate from a core version in emacs.git.  If it transpires
that the core version can also be used as the ELPA version, then it's
probably simple enough to change the ELPA recipe at that time to build
it from the emacs.git source instead?


-Phil

RE: Performance degradation from long lines

[Drew Adams]

Just one comment about this thread -

Could we please rename so-long.el to
so-long-and-thanks-for-all-the-fish.el?

Re: Performance degradation from long lines

[Stefan Monnier]

>>> I was unsure from the outset whether it was appropriate for a GNU
>>> ELPA package to make explicit reference to packages which were not
>>> available in core or through GNU ELPA, so I'm already aware of a

It's perfectly OK to mention (e.g. test for equality and such) pretty
much any Elisp package (assuming it's Free Software).

When introducing such a blacklist of modes, it's always good to also add
some way for a mode to voluntarily flag itself as "black", so the list
has a chance to shrink over time.

E.g. we could add a symbol property `slow-on-long-lines` which
font-lock-mode could set to announce that it assumes lines aren't
terribly long.


        Stefan

Re: Performance degradation from long lines

[Stefan Monnier]

> I'm imagining that a version in core for Emacs 27+ might differ slightly
> from the GNU ELPA package.

I'll leave it up to you.  But I usually find it pretty easy to maintain
code that works on old versions which making use of new features in
new versions.  And it's perfectly OK to have such backward-compatibility
code in emacs.git.

In contrast I find it painful in the long run to have to manually
propagate patches installed in emacs.git to elpa.git and vice versa
(especially since you can't just `git merge`).


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

On 28/10/18 4:32 AM, Drew Adams wrote:
> Just one comment about this thread -
> 
> Could we please rename so-long.el to
> so-long-and-thanks-for-all-the-fish.el?

Ha :)

Entertaining as that would be, I fear it would create quite the
confusing combination of pop-culture references in the code.

RE: Performance degradation from long lines

[Drew Adams]

> > Just one comment about this thread -
> >
> > Could we please rename so-long.el to
> > so-long-and-thanks-for-all-the-fish.el?
> 
> Ha :)
> 
> Entertaining as that would be, I fear it would create quite the
> confusing combination of pop-culture references in the code.

I just forgot the smily. ;-)

Re: Performance degradation from long lines

[Phil Sainty]

On 25/10/18 4:26 PM, Phil Sainty wrote:
> As it happens I've been working on this again in recent days, and
> intend to release this to GNU ELPA in the near future.  I haven't
> finished testing all the new changes, so there may be new bugs, but
> the latest work-in-progress code can be found at:
> 
> http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip

There were indeed some new bugs (primarily a dumb one which meant it
was *only* testing the first line).  Hopefully now all fixed.


> If there was some future work on the redisplay to provide a "reduced
> display capabilities" option or mode, configuring so-long to enable
> that should be trivial.

With this in mind, along with the discussion of longlines-mode, I've
introduced `so-long-action' (perhaps rename to `so-long-function'?)
which provides the following behaviours:

(*) Change major mode to so-long-mode (default)
( ) Disable minor modes and override variables
( ) Enable longlines-mode
( ) Custom function
( ) Do nothing

"Disable minor modes and override variables" means the buffer's original
major mode is retained, but all the *other* things which so-long-mode
would cause to happen still take place.


-Phil

Re: Performance degradation from long lines

[Ihor Radchenko]

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

Just tried *Backtrace* with emacs -Q from master (did not test on my
config since my config does not work with master).
Works perfectly on `org-element-parse-buffer` with 1.5Mb org buffer,
while I just had to kill my 26.1 emacs on the same task after 3 minutes
waiting. 


Gemini Lasswell <gazally@runbox.com> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> I would like to point out that the long line problem also exists for
>> non-file buffers.
>> Specifically, evaluating debug-on-entry for org-mode buffer parsing
>> functions often leads to ridiculously long lines (pretty much the whole
>> buffer is transformed into a single line), which takes forever to
>> display.
>> find-file-literally does not help with this problem at all.
>
> Have you tried debugging org-mode with an Emacs built from master?  I've
> made several improvements to the *Backtrace* buffers in master,
> including doing a much better job of abbreviating the printing of very
> large data structures, so that lines don't get so ridiculously long.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Date: Wed, 31 Oct 2018 21:05:07 +0800
> Cc: Eli Zaretskii <eliz@gnu.org>, mithraeum <mithraeum@protonmail.com>,
> 	emacs-devel@gnu.org
> 
> Just tried *Backtrace* with emacs -Q from master (did not test on my
> config since my config does not work with master).
> Works perfectly on `org-element-parse-buffer` with 1.5Mb org buffer,
> while I just had to kill my 26.1 emacs on the same task after 3 minutes
> waiting. 

Great, thanks for telling us.

Re: Performance degradation from long lines

[Stefan Monnier]

> I'm imagining that a version in core for Emacs 27+ might differ slightly
> from the GNU ELPA package.

It's usually pretty easy to unify the different version with tests on
emacs-version, fboundp, and friends.


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

On 9/12/18 12:08 PM, Stefan Monnier wrote:
> It's usually pretty easy to unify the different version with tests on
> emacs-version, fboundp, and friends.

Yes, I've swung around to the benefits of that approach in the interim,
so I'm expecting to go with a single-version of so-long, with the ELPA
version sourced from the Emacs repository, as previously suggested.

I've implemented a lot of changes based on the earlier conversations,
which is why it's still not finished; but I think I'm fairly close to
that point once again.  I'm only working on it occasionally, but would
guess that I'll have something for people to look at again within a
couple of weeks.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

so-long.el is once again ready for people to look at.

As before, the current code is available at:

http://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip

It's become more of a framework for allowing different 'actions' to be
defined and employed to mitigate performance issues.  Users can choose
the action they prefer, and new actions can easily be added, so
hopefully any alternative approaches can be easily integrated.

Each action comprises two functions (one to enable the behaviour, and
one to revert it), and all actions are available via the new menu.

The default actions are still:

- Change major mode to so-long-mode (default)
- Disable minor modes and override variables
- Enable longlines-mode

The system is now more general and extensible though, and the actions
are now more apparent for the end user.

The automated behaviour is still performed only during `set-auto-mode',
so I'm not dealing with cases other than visiting files; but in
principle I think one could write code to detect the introduction of
long lines in more dynamic situations and invoke one of the so-long
actions in response, so that could be considered in future.

I've tested this in 27, 26.1, and 25.3.  The changes to `normal-mode'
in 26.1 wrt when local variables are processed means that 26+ can use
local variables to affect the so-long behaviour whereas earlier versions
cannot; but the bulk of the features work in 25.3.  The original library
worked in 24.3 (which is still set as the minimum version), but I find
I am unable to build Emacs 24 on my current system, so that is untested.

There are no automated tests, and I don't have experience in writing
those, so if anyone was willing to assist on that front, that would be
fantastic.

Per previous discussions, my expectation is that we'll add this to
Emacs 27, and build the GNU ELPA package for older Emacsen from that.

The following changelog summaries the recent work towards version 1.0
(which is what I'll release this as):

- New user option `so-long-action'.
- New user option `so-long-action-alist' defining alternative actions.
- New user option `so-long-variable-overrides'.
- New user option `so-long-skip-leading-comments'.
- New user option `so-long-file-local-mode-function'.
- New user option `so-long-predicate'.
- New variable and function `so-long-function'.
- New variable and function `so-long-revert-function'.
- New command `so-long' to invoke `so-long-function' interactively.
- New command `so-long-revert' to invoke `so-long-revert-function'.
- Support retaining the original major mode while still disabling
  minor modes and overriding variables.
- Support `longlines-mode' as a `so-long-action' option.
- Added "So Long" menu, including all selectable actions.
- Added mode-line indicator, user option `so-long-mode-line-label',
  and faces `so-long-mode-line-active', `so-long-mode-line-inactive'.
- Renamed `so-long-mode-enabled' to `so-long-enabled'.
- Refactored the default hook values using variable overrides
  (and returning all the hooks to nil default values).
- Performance improvements for `so-long-detected-long-line-p'.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

On 28/10/18 11:15 AM, Stefan Monnier wrote:
> When introducing such a blacklist of modes, it's always good to also add
> some way for a mode to voluntarily flag itself as "black", so the list
> has a chance to shrink over time.
> 
> E.g. we could add a symbol property `slow-on-long-lines` which
> font-lock-mode could set to announce that it assumes lines aren't
> terribly long.

I've not implemented this suggestion at this point -- I thought it
warranted further discussion, and I wanted to get the other changes
dealt with first.

It's not entirely clear to me how this should be managed.

One approach is that modes which identify themselves this way would
be disabled automatically, and users would have no control over it.
That would probably be easiest to implement, but I don't particularly
like the idea that users lose control and/or visibility over what's
being done.

However, any approach whereby users do have control would seem to
imply some significant complications to the code and user interface;
especially as modes flagged in this way are not necessarily loaded
at the time when the user is customizing the so-long options.

The very simplest thing I can think of is a single boolean user option
to say "deal with any flagged modes too" (the currently-loaded instances
could be displayed in the UI).

Potentially `custom-add-frequent-value' could be employed to present
flagged modes as checkbox options which then remain visible even if
the user disables them; but that only works for 'hook' and 'alist'
custom types, and `so-long-minor-modes' is just a list of symbols.

If the user does get to confirm each flagged mode and save that
configuration in their custom file, then there's the issue of modes
which were flagged at one time (and saved by the user), but were
subsequently improved such that the symbol property is no longer
needed.  In that situation it would be nice to differentiate between
"modes that the user saved on purpose" and "modes which the user
saved because they used to be flagged".  In general, it would be
nice if the user could distinguish the automated settings from the
ones which they personally selected, in a non-cumbersome manner.

I suppose the list of modes could become an alist, with an associated
classification for each mode (e.g. 'auto for modes which should be
dealt with based upon their symbol property).  That would be a breaking
change, as `so-long-minor-modes' has existed as a simple list since
early 2016, so some kind of migration or other backwards-compatibility
support would be needed.

I expect that all this would apply to `so-long-variable-overrides'
as well.

I'm not sure this is necessary for an initial release, mind, but it
does seem like a good thing to provide at some point.

How would other people approach it?


-Phil

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Stefan Monnier]

> +FIXME: +++ once all documentation has been added.

No need for this FIXME: the whole point of this system is that you don't
need to do anything special to mark entries as "not yet handled" (any `**`
header without a preceding `---` or `+++` has not been handled yet), so
it's "fail safe".

> +;; Installation
> +;; ------------
> +;; And add the following to your init file to enable the library:
> +;;
> +;; (when (require 'so-long nil :noerror)
> +;;   (so-long-enable))

I think the above should just be

    (so-long-auto-mode 1)

and so-long-auto-mode (just my suggestion for a name, feel free to
chose something else) should be an autoloaded global minor mode which
activates so-long-mode.

On the design side, I think you should merge `so-long` and
`so-long-mode` into a single function and make that function
a (buffer-local) minor-mode (i.e. not have any major mode, just use
fundamental-mode instead).  Making it a minor mode rather than
a major-mode will also make it easier to remember the previous
major-mode without any need for a change-major-mode-hook hack.

> +(defvar so-long-enabled t
> +  "Set to nil to prevent `so-long' from being triggered automatically.")

Loading a file should not change the behavior of Emacs, so the default
should be nil unless/until we decide to preload this package.

Rather than defadvice, please use advice-add.

Other than that, looks great, thanks.


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

I've pushed a branch scratch/so-long with release-candidate code for
including so-long.el in Emacs 27 (and for the GNU ELPA package).

http://git.savannah.gnu.org/cgit/emacs.git/log?h=scratch/so-long

Nothing very significant has changed in the library since my last two
messages at the end of December, so all of that still applies.  I've
improved the documentation further and polished up a few rough edges.

I've added a node for the Emacs manual with a rudimentary description
of the issue.  This is mostly to acknowledge the problem, and point
readers to the new library and its Commentary.  This could doubtless
be expanded upon by others.


-Phil

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Phil Sainty]

Hi Stefan,

Thanks for the feedback.  I didn't spot this until after I'd posted my
last message to the "Performance degradation from long lines" thread.


On 11/01/19 7:07 AM, Stefan Monnier wrote:
>> +FIXME: +++ once all documentation has been added.
> 
> No need for this FIXME: the whole point of this system is that you don't
> need to do anything special to mark entries as "not yet handled" (any `**`
> header without a preceding `---` or `+++` has not been handled yet), so
> it's "fail safe".

That FIXME was just for me -- I knew that I wanted to add something to
the manual, but hadn't done so yet, and didn't want to mark the NEWS
item +++ until I'd added that, and I didn't want to forget to mark it
+++ once I'd done so :)  The current branch has this fixed.


>> +;; Installation
>> +;; ------------
>> +;; And add the following to your init file to enable the library:
>> +;;
>> +;; (when (require 'so-long nil :noerror)
>> +;;   (so-long-enable))
> 
> I think the above should just be
> 
>     (so-long-auto-mode 1)
> 
> and so-long-auto-mode (just my suggestion for a name, feel free to
> chose something else) should be an autoloaded global minor mode which
> activates so-long-mode.

I actually spent some time experimenting with that idea last month,
as it seemed like a good idea to me as well, but it got sufficiently
awkward that I dropped the notion as being more hassle than it was
worth.

If I define a global mode, then there's another variable which is
ostensibly tracking the 'enabled' state of the functionality, which
seemed ugly.  I tried using :variable in the mode definition to make
that use `so-long-enabled' internally, but then in testing I found
that I could de-sync the mode and that variable (I forget how, but
possibly by calling `so-long-disable' which I need to keep for
backwards-compatibility), and then it just struck me that I was
making more work and a bit of a headache for myself in an effort to
allow people to add '(global-so-long-mode)' in their init files
instead of '(so-long-enable)' and I immediately lost all interest
in spending more time on achieving that.

`so-long-enable' is autoloaded, so I could drop the test for
(require 'so-long nil :noerror) in the Commentary.  That's a hold-
over from the earlier releases, and it seemed harmless to keep, but
I'm happy to remove it, as it's not needed for either 27 or ELPA.


> On the design side, I think you should merge `so-long` and
> `so-long-mode` into a single function and make that function
> a (buffer-local) minor-mode (i.e. not have any major mode, just use
> fundamental-mode instead).  Making it a minor mode rather than
> a major-mode will also make it easier to remember the previous
> major-mode without any need for a change-major-mode-hook hack.

These changes are too significant, so I don't wish to do any of that.

There's backwards-compatibility with the earlier releases to consider
(so-long-mode has always been a major mode); and I also see a benefit
to retaining a major mode option (I have a use-case for using it as a
file-local 'mode' on one project, and I've had cause to suggest the
same thing to other users).  I did consider turning `so-long' into a
`so-long-minor-mode' at one point, but I think it either seemed like
a fairly needless change, or possibly the idea of implementing a
feature which was designed to mess with minor mode states *as* a minor
mode seemed wrong to me.  'M-x so-long' is also shorter :)


>> +(defvar so-long-enabled t
>> +  "Set to nil to prevent `so-long' from being triggered automatically.")
> 
> Loading a file should not change the behavior of Emacs, so the default
> should be nil unless/until we decide to preload this package.

That var does nothing at all if (so-long-enable) hasn't been called;
but offhand I think it's fine to set it nil by default, so I'll do that.


> Rather than defadvice, please use advice-add.

I know defadvice and I don't know advice-add, so perhaps someone else
can write those changes.  Such changes will need to work in Emacs 25 at
least, and ideally back to 24.3 (but as I can no longer build 24.x on
my machine, I've not actually tested the current library on that version
myself, so I'm not certain that I've retained compatibility with 24.3 in
this release).

I have vague recollections that advice-add is very different when it
comes to ad-get-arg, so note in particular that the signature to
hack-local-variables changed in 26.1.  That's not an issue for the
advice I've written, but if it would be for nadvice, then that needs
to be accounted for in any rewrite.


> Other than that, looks great, thanks.

Cheers,

-Phil

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@IRO.UMontreal.CA>,
>  mithraeum <mithraeum@protonmail.com>
> Date: Sat, 12 Jan 2019 14:03:14 +1300
> 
> I've pushed a branch scratch/so-long with release-candidate code for
> including so-long.el in Emacs 27 (and for the GNU ELPA package).
> 
> http://git.savannah.gnu.org/cgit/emacs.git/log?h=scratch/so-long

Thanks.  I think we can land this on master, but see a few minor
comments below, mostly about documentation.

  +** so-long.el helps to mitigate performance problems with long lines.
  +When `so-long-enable' has been called, visiting a file with very long
  +lines will (subject to configuration) cause the user's preferred
  +`so-long-action' to be automatically invoked (by default, the buffer's
  +major mode is replaced by `so-long-mode').  In extreme cases this can
  +prevent delays of several minutes, and make Emacs responsive almost
  +immediately.  Type 'M-x so-long-commentary' for full documentation.

Please use quoting 'like this' consistently.

  +;; This library advises `set-auto-mode' (in order to react after Emacs has
  +;; chosen the major mode for a buffer), and `hack-local-variables' (so that we
  +;; may behave differently when a file-local mode is set).  In earlier versions
  +;; of Emacs (< 26.1) we also advise `hack-one-local-variable' (to prevent a
  +;; file-local mode from restoring the original major mode if we had changed it).

I'd prefer not to use advices in bundled code.  Is it feasible to
instead include hooks in Emacs that so-long-mode could use?  If the
problem is older versions of Emacs, I'm okay with using advices only
for those old versions.

  +(defgroup so-long nil
  +  "Prevent unacceptable performance degradation with very long lines."
  +  :prefix "so-long"
  +  :group 'convenience)
  +
  +(defcustom so-long-threshold 250
  +  "Maximum line length permitted before invoking `so-long-function'.
  +
  +See `so-long-detected-long-line-p' for details."
  +  :type 'integer
  +  :package-version '(so-long . "1.0")
  +  :group 'so-long)

Please add :version tags to all the customizable options.

  +(defcustom so-long-target-modes
  +  '(prog-mode css-mode sgml-mode nxml-mode)
  +  "`so-long' affects only these modes and their derivatives.

I've heard complaints from users of JASON and JavaScript -- should the
default value cover those as well?

  +(defun so-long-action-alist-setter (option value)
  +  "The :set function for `so-long-action-alist'."

For non-internal functions, please reference the arguments in the
first line of the doc string.

  +(defun so-long-change-major-mode ()
  +  "Ensures that `so-long-mode' knows the original `major-mode'
  +even when invoked interactively.

"Ensure", to be consistent with our style of writing the first
sentence of the doc strings.  Also, please make the first sentence fit
on a single line -- this is important for Apropos commands, which only
display one line.

  +(defun so-long-menu ()
  +  "Dynamically generate the \"So Long\" menu."
  +  ;; (info "(elisp) Menu Example")

How about providing some help-echo for this menu?

  +(defun so-long-menu-item-revert ()
  +  "Invoke `so-long-revert'."

This doc string is not very informative, and neither is that of
so-long-revert.  How about adding more detailed description here?  I
believe this is an important command that users will want to know
about.

Thanks again for working on this.

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Stefan Monnier]

> If I define a global mode, then there's another variable which is
> ostensibly tracking the 'enabled' state of the functionality, which
> seemed ugly.

I don't see why.  If you don't want to get rid of the so-long-enabled
variable, then just make it a defvaralias and you're done.

> that I could de-sync the mode and that variable (I forget how, but
> possibly by calling `so-long-disable' which I need to keep for
> backwards-compatibility),

Redefine so-long-enable as an obsolete function that just calls
(so-long-auto-mode 1).

Side note: I mistakenly thought so-long was pretty much a brand new
package.  How far back does it go?

> and then it just struck me that I was making more work and a bit of
> a headache for myself in an effort to allow people to add
> '(global-so-long-mode)' in their init files instead of
> '(so-long-enable)' and I immediately lost all interest in spending
> more time on achieving that.

The issue is consistency (a minor mode is a standard interface and UI to
enable&disable&test a feature) and ease of use (a global minor mode can
be enabled via Customize).

> `so-long-enable' is autoloaded, so I could drop the test for
> (require 'so-long nil :noerror) in the Commentary.  That's a hold-
> over from the earlier releases, and it seemed harmless to keep,

Users will copy it and wonder why Emacs has to be so complicated, and it
will then fail the day the file is renamed.

> but I'm happy to remove it, as it's not needed for either 27 or ELPA.

That would be good.

>> On the design side, I think you should merge `so-long` and
>> `so-long-mode` into a single function and make that function
>> a (buffer-local) minor-mode (i.e. not have any major mode, just use
>> fundamental-mode instead).  Making it a minor mode rather than
>> a major-mode will also make it easier to remember the previous
>> major-mode without any need for a change-major-mode-hook hack.
>
> These changes are too significant, so I don't wish to do any of that.

I think they're just some minor shuffling of code.

> There's backwards-compatibility with the earlier releases to consider
> (so-long-mode has always been a major mode);

Then use another name for the minor mode and make so-long-mode be the
combination of fundamental-mode + so-long-minor-mode.

> and I also see a benefit to retaining a major mode option (I have
> a use-case for using it as a file-local 'mode' on one project, and
> I've had cause to suggest the same thing to other users).

Minor modes can also be used on the "mode:" thingy, tho I don't think
that should make a big difference.

> I did consider turning `so-long' into a
> `so-long-minor-mode' at one point, but I think it either seemed like
> a fairly needless change, or possibly the idea of implementing a
> feature which was designed to mess with minor mode states *as* a minor
> mode seemed wrong to me.  'M-x so-long' is also shorter :)

The point of a minor mode is to provide a standard UI to enable *and*
disable something.

>> Rather than defadvice, please use advice-add.
> I know defadvice and I don't know advice-add, so perhaps someone else
> can write those changes.

E.g.

    (defadvice hack-local-variables (after so-long--file-local-mode disable)
      "..."
      ;; The first arg to `hack-local-variables' is HANDLE-MODE since Emacs 26.1,
      ;; and MODE-ONLY in earlier versions.  In either case we are interested in
      ;; whether it has the value `t'.
      (and (eq (ad-get-arg 0) t)
           ad-return-value ; A file-local mode was set.
           (so-long-handle-file-local-mode ad-return-value)))

can be turned into

    (advice-add 'hack-local-variables :around #'so-long--hlv)
    (defun so-long--hlv (orig-fun &optional handle-mode &rest args)
      ;; The first arg to `hack-local-variables' is HANDLE-MODE since Emacs 26.1,
      ;; and MODE-ONLY in earlier versions.  In either case we are interested in
      ;; whether it has the value `t'.
      (let ((retval (apply orig-fun handle-mode args)))
        (and (eq handle-mode t)
             retval ; A file-local mode was set.
             (so-long-handle-file-local-mode retval))))

BTW, while reading that code, I couldn't quite understand what this
advice is about.  I think a comment describing a use-case would be valuable.

> Such changes will need to work in Emacs 25 at least, and ideally back
> to 24.3

advice-add was added to Emacs-24.4 and it is available in GNU ELPA
for earlier Emacsen, so it's OK.


        Stefan

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Stefan Monnier]

>>> On the design side, I think you should merge `so-long` and
>>> `so-long-mode` into a single function and make that function
>>> a (buffer-local) minor-mode (i.e. not have any major mode, just use
>>> fundamental-mode instead).  Making it a minor mode rather than
>>> a major-mode will also make it easier to remember the previous
>>> major-mode without any need for a change-major-mode-hook hack.
>> These changes are too significant, so I don't wish to do any of that.
> I think they're just some minor shuffling of code.

Also, I'm wondering which part you don't like.  There are fundamentally
two aspects in my suggestion:
- merge the two
- use a minor mode
I think the most important for the user is the first: the difference
between M-x so-long RET and M-x so-long-mode RET seems fairly subtle to
me and I expect users will have trouble remembering which is which or
even understand that they don't do the same.


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

On 13/01/19 12:08 AM, Eli Zaretskii wrote:
>> http://git.savannah.gnu.org/cgit/emacs.git/log?h=scratch/so-long
> 
> Thanks.  I think we can land this on master, but see a few minor
> comments below, mostly about documentation.
> 
> Please use quoting 'like this' consistently [in NEWS].

Fixed.


>   +;; This library advises `set-auto-mode' (in order to react after Emacs has
>   +;; chosen the major mode for a buffer), and `hack-local-variables' (so that we
>   +;; may behave differently when a file-local mode is set).  In earlier versions
>   +;; of Emacs (< 26.1) we also advise `hack-one-local-variable' (to prevent a
>   +;; file-local mode from restoring the original major mode if we had changed it).
> 
> I'd prefer not to use advices in bundled code.  Is it feasible to
> instead include hooks in Emacs that so-long-mode could use?  If the
> problem is older versions of Emacs, I'm okay with using advices only
> for those old versions.

This harks back to my earlier comments here:
https://lists.gnu.org/archive/html/emacs-devel/2018-10/msg00551.html

I was subsequently convinced by the argument that building the GNU ELPA
release directly from the core version for Emacs 27 was the sensible
approach, so the library as-is is intended to be backwards-compatible
(minimum version is presently 24.4).

I'm sure that we could introduce some new changes and hooks to 27 to
avoid the need for the advice, but I think that could be version 1.1.


>   +(defgroup so-long nil
>   +  "Prevent unacceptable performance degradation with very long lines."
>   +  :prefix "so-long"
>   +  :group 'convenience)
>   +
>   +(defcustom so-long-threshold 250
>   +  "Maximum line length permitted before invoking `so-long-function'.
>   +
>   +See `so-long-detected-long-line-p' for details."
>   +  :type 'integer
>   +  :package-version '(so-long . "1.0")
>   +  :group 'so-long)
> 
> Please add :version tags to all the customizable options.

It hadn't occurred to me that the defgroup should specify a version,
but I can add that.

I thought that :package-version functioned as an alternative to
:version which worked both for the core library and for the GNU ELPA
package for earlier emacs versions?

Libraries like rst.el and mh-e.el seem to use it this way.

I already have:

(add-to-list 'customize-package-emacs-version-alist
             '(so-long ("1.0" . "27.1")))

And :package-version '(so-long . "1.0") for all options.

Do I need to add :version "27.1" as well?


>   +(defcustom so-long-target-modes
>   +  '(prog-mode css-mode sgml-mode nxml-mode)
>   +  "`so-long' affects only these modes and their derivatives.
> 
> I've heard complaints from users of JASON and JavaScript -- should the
> default value cover those as well?

JSON rather than JASON, I imagine?

js-mode is derived from prog-mode, so both of those are covered.


>   +(defun so-long-action-alist-setter (option value)
>   +  "The :set function for `so-long-action-alist'."
> 
> For non-internal functions, please reference the arguments in the
> first line of the doc string.

That's pretty internal.  I'll rename that and `so-long-action-type'
with a so-long-- prefix.


>   +(defun so-long-change-major-mode ()
>   +  "Ensures that `so-long-mode' knows the original `major-mode'
>   +even when invoked interactively.
> 
> "Ensure", to be consistent with our style of writing the first
> sentence of the doc strings.

Done.

> Also, please make the first sentence fit on a single line -- this
> is important for Apropos commands, which only display one line.

In this instance I had intentionally wrapped the line early so that
the first line was coherent on its own, even though it was part of a
longer sentence.  i.e.:

"Ensures that `so-long-mode' knows the original `major-mode'"

If that's not sufficient, I can rephrase it over multiple sentences.


>   +(defun so-long-menu ()
>   +  "Dynamically generate the \"So Long\" menu."
>   +  ;; (info "(elisp) Menu Example")
> 
> How about providing some help-echo for this menu?

I'd tried that initially, but I found it so glitchy that I removed it.
As I recall, there was a tendency for a blank popup to appear a lot of
the time, which wasn't useful, and wasn't something I wanted to debug.

The Help sub-menu contains an option for reading the Commentary,
which covers the various menu items, so I'm happy with that for
documentation.


>   +(defun so-long-menu-item-revert ()
>   +  "Invoke `so-long-revert'."
> 
> This doc string is not very informative, and neither is that of
> so-long-revert.  How about adding more detailed description here?
> I believe this is an important command that users will want to know
> about.

True.  I don't think there's very much to add, but I shall elaborate
somewhat on the latter's documentation.

I think the `so-long-menu-item-revert' docstring can remain as a
simple cross-reference to `so-long-revert', or else I could do this:

;; Duplicate the `so-long-revert' documentation for the menu item.
(put 'so-long-menu-item-revert 'function-documentation
     (documentation 'so-long-revert t))


-Phil

Re: Performance degradation from long lines

[Eli Zaretskii]

> Cc: mithraeum@protonmail.com, monnier@IRO.UMontreal.CA, emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Sun, 10 Mar 2019 23:22:02 +1300
> 
> >   +(defgroup so-long nil
> >   +  "Prevent unacceptable performance degradation with very long lines."
> >   +  :prefix "so-long"
> >   +  :group 'convenience)
> >   +
> >   +(defcustom so-long-threshold 250
> >   +  "Maximum line length permitted before invoking `so-long-function'.
> >   +
> >   +See `so-long-detected-long-line-p' for details."
> >   +  :type 'integer
> >   +  :package-version '(so-long . "1.0")
> >   +  :group 'so-long)
> > 
> > Please add :version tags to all the customizable options.
> 
> It hadn't occurred to me that the defgroup should specify a version,
> but I can add that.

I meant defcustoms.

> I thought that :package-version functioned as an alternative to
> :version which worked both for the core library and for the GNU ELPA
> package for earlier emacs versions?

AFAIR, when we discussed this, the intent was to put this in core?  I
have no clear idea what is required for ELPA.

> > Also, please make the first sentence fit on a single line -- this
> > is important for Apropos commands, which only display one line.
> 
> In this instance I had intentionally wrapped the line early so that
> the first line was coherent on its own, even though it was part of a
> longer sentence.  i.e.:
> 
> "Ensures that `so-long-mode' knows the original `major-mode'"
> 
> If that's not sufficient, I can rephrase it over multiple sentences.

I think we want this rephrased.  Just put a period at the end of the
first line, and then add something to the beginning of the rest to get
the next sentence.

> >   +(defun so-long-menu ()
> >   +  "Dynamically generate the \"So Long\" menu."
> >   +  ;; (info "(elisp) Menu Example")
> > 
> > How about providing some help-echo for this menu?
> 
> I'd tried that initially, but I found it so glitchy that I removed it.

??? I'm surprised: I never saw nor hear any complaints about help-echo
for menus.

> I think the `so-long-menu-item-revert' docstring can remain as a
> simple cross-reference to `so-long-revert', or else I could do this:
> 
> ;; Duplicate the `so-long-revert' documentation for the menu item.
> (put 'so-long-menu-item-revert 'function-documentation
>      (documentation 'so-long-revert t))

I wonder how will that work with i18n.

Re: Performance degradation from long lines

[martin rudalics]

 >>> How about providing some help-echo for this menu?
 >>
 >> I'd tried that initially, but I found it so glitchy that I removed it.
 >
 > ??? I'm surprised: I never saw nor hear any complaints about help-echo
 > for menus.

ISTR that they don't work when turning off the blinking cursor.

martin

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Sun, 10 Mar 2019 14:36:05 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: mithraeum@protonmail.com, monnier@IRO.UMontreal.CA, 
>  emacs-devel@gnu.org
> 
>  >>> How about providing some help-echo for this menu?
>  >>
>  >> I'd tried that initially, but I found it so glitchy that I removed it.
>  >
>  > ??? I'm surprised: I never saw nor hear any complaints about help-echo
>  > for menus.
> 
> ISTR that they don't work when turning off the blinking cursor.

I awlays thought it was specific to MS-Windows.  And anyway, not
showing the help-echo is a far cry from displaying empty tip frames.

Re: Performance degradation from long lines

[Stefan Monnier]

>>>> How about providing some help-echo for this menu?
>>> I'd tried that initially, but I found it so glitchy that I removed it.
>> ??? I'm surprised: I never saw nor hear any complaints about help-echo
>> for menus.
> ISTR that they don't work when turning off the blinking cursor.

FWIW, I have blinking cursor turned off and I see all the help-echo when
using the menu bar.  If that didn't work, I'd consider it a bug.


        Stefan

Re: Performance degradation from long lines

[Phil Sainty]

On 2019-03-11 02:46, Eli Zaretskii wrote:
>> From: martin rudalics <rudalics@gmx.at>
>>  >>> How about providing some help-echo for this menu?
>>  >>
>>  >> I'd tried that initially, but I found it so glitchy that I removed 
>> it.
>>  >
>>  > ??? I'm surprised: I never saw nor hear any complaints about 
>> help-echo
>>  > for menus.
>> 
>> ISTR that they don't work when turning off the blinking cursor.
> 
> I awlays thought it was specific to MS-Windows.  And anyway, not
> showing the help-echo is a far cry from displaying empty tip frames.

FWIW I use --with-x-toolkit=lucid and I don't use blink-cursor-mode; but 
a
very quick test of the Help menu doesn't exhibit any issues, so it's 
surely
not a general problem with help-echo.  I may have been doing-it-wrong, 
or
perhaps triggering an edge-case.

I'm not 100% that I remembered the symptom accurately.  I'll have to try 
it
again later to see if I can reproduce the issue and provide better 
information
in a separate a bug report.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

On 2019-03-11 01:58, Eli Zaretskii wrote:
>> I thought that :package-version functioned as an alternative to
>> :version which worked both for the core library and for the GNU ELPA
>> package for earlier emacs versions?
> 
> AFAIR, when we discussed this, the intent was to put this in core?
> I have no clear idea what is required for ELPA.

The intent is to put it in core, and also to build an ELPA package from
that same core source file, for use with emacs versions < 27.

:package-version had looked to me like the way to handle versioning in
this scenario, so that the same code accounted for both situations.


-Phil

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Mon, 11 Mar 2019 12:31:16 +1300
> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: mithraeum@protonmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> :package-version had looked to me like the way to handle versioning in
> this scenario, so that the same code accounted for both situations.

Does it?  The :version tag is important for Emacs to know which
variables were added/changed in what Emacs version, to say something
like "this variable was added in Emacs version X.Y.Z", and also to
support commands like customize-changed.  Does :package-version
support all that?

Re: Performance degradation from long lines

[Phil Sainty]

On 2019-03-11 16:35, Eli Zaretskii wrote:
> Does it?  The :version tag is important for Emacs to know which
> variables were added/changed in what Emacs version, to say something
> like "this variable was added in Emacs version X.Y.Z", and also to
> support commands like customize-changed.  Does :package-version
> support all that?

e.g. C-h v so-long-action concludes with:

"This variable was introduced, or its default value was changed, in
version 1.0 of the so-long package that is part of Emacs 27.1."

(elisp) Common Keywords says of :package-version

      This keyword specifies that the item was first introduced in
      PACKAGE version VERSION, or that its meaning or default value was
      changed in that version.  This keyword takes priority over
      ‘:version’.

Re: Performance degradation from long lines

[martin rudalics]

 > FWIW, I have blinking cursor turned off and I see all the help-echo when
 > using the menu bar.  If that didn't work, I'd consider it a bug.

As Eli said it's a MS-Windows issue.

martin

Re: Performance degradation from long lines

[Eli Zaretskii]

> Date: Mon, 11 Mar 2019 16:48:08 +1300
> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: mithraeum@protonmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On 2019-03-11 16:35, Eli Zaretskii wrote:
> > Does it?  The :version tag is important for Emacs to know which
> > variables were added/changed in what Emacs version, to say something
> > like "this variable was added in Emacs version X.Y.Z", and also to
> > support commands like customize-changed.  Does :package-version
> > support all that?
> 
> e.g. C-h v so-long-action concludes with:
> 
> "This variable was introduced, or its default value was changed, in
> version 1.0 of the so-long package that is part of Emacs 27.1."
> 
> (elisp) Common Keywords says of :package-version
> 
>       This keyword specifies that the item was first introduced in
>       PACKAGE version VERSION, or that its meaning or default value was
>       changed in that version.  This keyword takes priority over
>       ‘:version’.

OK, so it's not :package-version that does the trick, it's the fact
that you added so-long to customize-package-emacs-version-alist.

Although I wonder whether users of Emacs 26.2 will become confused
when they see in "C-h v" a reference to Emacs 27.1...

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Phil Sainty]

TL;DR:

1. Things I've changed:

a) Per your suggestion, the automated behaviour is now enabled and
   disabled via a global minor mode, `global-so-long-mode'.

b) The 'overrides-only action is now a buffer-local minor mode,
   which can be used/toggled by itself; so users now have a minor
   mode (`so-long-minor-mode') which will toggle the default
   performance mitigations.

c) Switched from defadvice to nadvice.

d) Improved / clarified documentation.


2. Things I'm not changing:

a) `so-long-mode' remains a major mode.

b) The `so-long' and `so-long-revert' commands remain as they were;
   dispatching to the action functions set in `so-long-action-alist'.


As before, the latest code is here:

https://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip



Responses in depth:


On 13/01/19 4:11 AM, Stefan Monnier wrote:
> The issue is consistency (a minor mode is a standard interface and
> UI to enable&disable&test a feature) and ease of use (a global minor
> mode can be enabled via Customize).
>
> Redefine so-long-enable as an obsolete function that just calls
> (so-long-auto-mode 1).

Ok, using the Customize interface to enable the library seemed like a
good reason to do this.  I've added `global-so-long-mode' which runs
the enable/disable behaviour appropriately, with `so-long-enable' and
`so-long-disable' redefined as suggested, and the documentation
changed to refer to the global mode.



> Side note: I mistakenly thought so-long was pretty much a brand new
> package.  How far back does it go?

The initial release was in January 2016, and version 0.7.6 (being the
latest release until the recent enhancements) was in July 2016.

Until the recent work, the `so-long-mode' major mode was the only
mitigation the library provided, and so all of the behaviours were
tied to that.



>>> Rather than defadvice, please use advice-add.
>
> E.g.
>
>     (defadvice hack-local-variables (after so-long--file-local-mode
disable)
>       "..."
>       ;; The first arg to `hack-local-variables' is HANDLE-MODE since
Emacs 26.1,
>       ;; and MODE-ONLY in earlier versions.  In either case we are
interested in
>       ;; whether it has the value `t'.
>       (and (eq (ad-get-arg 0) t)
>            ad-return-value ; A file-local mode was set.
>            (so-long-handle-file-local-mode ad-return-value)))
>
> can be turned into
>
>     (advice-add 'hack-local-variables :around #'so-long--hlv)
>     (defun so-long--hlv (orig-fun &optional handle-mode &rest args)
>       ;; The first arg to `hack-local-variables' is HANDLE-MODE since
Emacs 26.1,
>       ;; and MODE-ONLY in earlier versions.  In either case we are
interested in
>       ;; whether it has the value `t'.
>       (let ((retval (apply orig-fun handle-mode args)))
>         (and (eq handle-mode t)
>              retval ; A file-local mode was set.
>              (so-long-handle-file-local-mode retval))))

This is changing 'after' advice into 'around' advice, which means that
your version changes the return value, so that's definitely not
equivalent.

I've changed it to explicitly return retval.  Can nadvice not do
'after' advice which knows the arguments?



>     (defadvice hack-local-variables (after so-long--file-local-mode
disable)
>
> BTW, while reading that code, I couldn't quite understand what this
> advice is about.  I think a comment describing a use-case would be
> valuable.

That advice and `so-long-check-header-modes' each handle one of the
ways in which a file-local 'mode' could be set.  If the file does
indeed have a file-local mode, then `so-long-handle-file-local-mode'
is called.

This is on the basis that file-local modes present a special-case --
when a file contains long lines yet a file-local mode was also set, it
is reasonable to suspect that, despite the long lines, the mode is
actually safe to use in practice (as if the mode was prohibitively
slow with that file, the author who added the file-local mode would
probably not have done so).  That's guess-work as far as so-long is
concerned of course, so `so-long-file-local-mode-function' is a user
option which lets the user decide how those situations should be
handled (with the default behaviour being fairly conservative).

The "Files with a file-local 'mode'" section of the Commentary says
much the same, but I'd not yet added that at the time you queried
this.  I think that ought to provide the missing context for readers
(as otherwise I think all the advice is documented pretty clearly?)



>> Such changes will need to work in Emacs 25 at least, and ideally
>> back to 24.3
>
> advice-add was added to Emacs-24.4 and it is available in GNU ELPA
> for earlier Emacsen, so it's OK.

So: Package-Requires: ((emacs "24.3") nadvice), yes?

No -- experimentally that causes Emacs 26.1 to install nadvice from
ELPA instead of deferring to the library it already has.

Can I tell package.el that it's an ELPA requirement only if nadvice.el
can't already be located?  (Is that not what Package-Requires should
mean by default?)

For now I've bumped the required Emacs version to 24.4 instead.



On 13/01/19 10:01 AM, Stefan Monnier wrote:
>>> On the design side, I think you should merge `so-long` and
>>> `so-long-mode` into a single function and make that function a
>>> (buffer-local) minor-mode (i.e. not have any major mode, just use
>>> fundamental-mode instead).  Making it a minor mode rather than a
>>> major-mode will also make it easier to remember the previous
>>> major-mode without any need for a change-major-mode-hook hack.
>>
>> These changes are too significant, so I don't wish to do any of that.
>
> I think they're just some minor shuffling of code.
>
> Also, I'm wondering which part you don't like.

It's partly that I've put a lot of effort into the way it is now, and
the idea of reworking things at this stage is honestly painful.  I'd
already tried a few different approaches along the way to see what
seemed to work well in practice, and heavily reworked some aspects
in order to make it more extensible and easier to customize for the
user; and spent a lot of time figuring out edge cases and
backwards-compatibility issues with earlier versions of Emacs, sorting
them all out, and polishing it all to a release state I was happy
with, including working pretty hard on documenting everything.

I'll concur that it seems more complicated than it might be; but it's
flexible, and it works well.

My gut says that the changes you're suggesting will inevitably entail
a lot more of those time-consuming activities aside from any "minor
shuffling of code" (and I don't think it's going to be as minor as
that); so assuming it would also be me doing the rework, the notion
holds no appeal.

Moreover, I just don't think merging `so-long' and `so-long-mode' is
the right thing to do.  I do appreciate that your suggestion *sounds*
like it would be clean and simple (and consequently an appealing
improvement), but I don't think it actually provides the same
functionality (or doing so winds up adding other new complications).

Having a major mode is important to the usability.

Even though I've added the ability to choose different 'actions', I've
kept `so-long-mode' as the default on purpose, because I think it's
important that when the automated behaviour kicks in, the reason is
very much "in your face".  When the user visits a file and everything
looks wrong ("What's happened? Where is all my syntax highlighting?"),
a major mode makes it VERY apparent what the cause is, and the
explanation is an easy 'C-h m' away, with the needed documentation
right at the top of the *Help* buffer, rather than buried somewhere
within a long list of minor modes (which the user wouldn't necessarily
otherwise realise they were looking for).

There are some other side-benefits of having a major mode option as
well, but I believe this default makes the effects more obvious, and
consequently the library more usable.  The major mode also means that
for this default case I can safely bind the familiar 'C-c C-c'
sequence to the revert function, which then makes it super easy for
users to revert in false-positive cases; so it's hardly even a bother
if that happens.



> There are fundamentally two aspects in my suggestion:
> - merge the two
> - use a minor mode
>
> I think the most important for the user is the first: the difference
> between M-x so-long RET and M-x so-long-mode RET seems fairly subtle
> to me and I expect users will have trouble remembering which is
> which or even understand that they don't do the same.

This is a documentation problem, so I've addressed it as such.  At the
time it hadn't been sufficiently obvious to me that the two would be
confused, because I knew the differences so well myself.



>> There's backwards-compatibility with the earlier releases to
>> consider (so-long-mode has always been a major mode);
>
> Then use another name for the minor mode and make so-long-mode be the
> combination of fundamental-mode + so-long-minor-mode.

If there's no actual major mode, then we lose the benefits of having
this functionality available in the form of a major mode.

Also, `so-long-mode' does very specific things, while `so-long' does
whatever `so-long-action' tells it to; so doing the equivalent of the
latter in fundamental-mode is not necessarily similar to the current
major mode.



>> I have a use-case for using it as a file-local 'mode' on one
>> project, and I've had cause to suggest the same thing to other
>> users.
>
> Minor modes can also be used on the "mode:" thingy, tho I don't
> think that should make a big difference.

They can be, but, to the best of my understanding, they shouldn't be.
Using a minor mode as a file-local 'mode' is explicitly deprecated --
it's specifically stated that only major modes should be used as a
'mode' value, and minor modes should be enabled via 'eval'.

The comments in `set-auto-mode' (also in `so-long-check-header-modes',
which needed to duplicate that code) read:

;; Once we drop the deprecated feature where mode: is also allowed to
;; specify minor-modes (ie, there can be more than one "mode:"), we can
;; remove this section and just let (hack-local-variables t) handle it.

I also think the behaviour when there's a single 'mode' value but it
isn't a major mode can easily be confusing.  It seems like something
which might be working only by accident and might be brittle.

Or to put it another way, the notion of a non-major mode that gets
used as if it were a major mode is, to me, a more confusing prospect
than any confusion you're trying to avoid by proposing that approach.

(Tangentially, I've often thought that modes should have an easy way
of being inspected and identified as major or minor, buffer-local or
global, or globalized (controller or control-ee); and if that happened
then it could sensibly be used to enforce the restriction of 'mode' to
major modes, which would then be at odds with any hack to treat a
minor mode as a major mode.)



Regarding using a minor mode instead of `so-long'/`so-long-revert'...

> The point of a minor mode is to provide a standard UI to enable
> *and* disable something.

Having now established that I'm keeping the major mode, I think that
a minor mode which controls a major mode would also be a confusing
situation.

i.e. At that point we would have: global-so-long-mode calling a
buffer-local minor mode which is (potentially) calling our major mode,
so all three could be listed when the user typed C-h m.

The minor mode would then need to remain active after that particular
major mode change (but also *not* remain active if some other major
mode change occurred).

I know that's not what you had in mind (on account of your wanting to
eliminate the major mode); but as I feel strongly about retaining the
major mode, I also feel strongly that we don't want to create the
above situation -- especially not on the basis of an attempt to reduce
the potential for confusion.

The `so-long' command is really a dispatcher -- it invokes an action,
and that action can be anything, and it's simpler to keep it that way.



The part that conceptually seemed like more of a candidate for a minor
mode was the `overrides-only' action, which was essentially the same
as `so-long-mode' but without the major mode.  Originally I had
dismissed the notion of that being a minor mode (on the basis that it
could add confusion for no real benefit); but I've changed my mind and
implemented it: `so-long-minor-mode' now exists as the minor-mode
equivalent to `so-long-mode'.

This possibly achieves some middle ground with your suggestions, as
the user can, if they wish, invoke this minor mode directly (to much
the same effect as setting `so-long-action' to the minor mode value,
and calling `so-long').


-Phil

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Stefan Monnier]

>>     (advice-add 'hack-local-variables :around #'so-long--hlv)
>>     (defun so-long--hlv (orig-fun &optional handle-mode &rest args)
>>       ;; The first arg to `hack-local-variables' is HANDLE-MODE since
> Emacs 26.1,
>>       ;; and MODE-ONLY in earlier versions.  In either case we are
> interested in
>>       ;; whether it has the value `t'.
>>       (let ((retval (apply orig-fun handle-mode args)))
>>         (and (eq handle-mode t)
>>              retval ; A file-local mode was set.
>>              (so-long-handle-file-local-mode retval))))
>
> This is changing 'after' advice into 'around' advice, which means that
> your version changes the return value, so that's definitely not
> equivalent.

Oops, indeed, sorry.

> I've changed it to explicitly return retval.

Good, thanks.

> Can nadvice not do 'after' advice which knows the arguments?

The arguments, yes, but the just-computed return value, no.

> The "Files with a file-local 'mode'" section of the Commentary says
> much the same, but I'd not yet added that at the time you queried
> this.  I think that ought to provide the missing context for readers
> (as otherwise I think all the advice is documented pretty clearly?)

I must admit that I do a lot of hacking on packages I never use, so
I like it when the code explains what it does without requiring the
coder to have read the doc ;-)

>>> Such changes will need to work in Emacs 25 at least, and ideally
>>> back to 24.3
>> advice-add was added to Emacs-24.4 and it is available in GNU ELPA
>> for earlier Emacsen, so it's OK.
> So: Package-Requires: ((emacs "24.3") nadvice), yes?
> No -- experimentally that causes Emacs 26.1 to install nadvice from
> ELPA instead of deferring to the library it already has.

IIUC that's a bug in Emacs-26.1 (and 26.2) because 26.[12] is not aware
that it comes with nadvice-1.0.

It really should have been fixed before Emacs-26.2, but I somehow forgot
to push that commit and it lingered on a machine I only used again
very recently :-(


        Stefan

Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Phil Sainty]

On 2019-04-15 03:14, Stefan Monnier wrote:
>> So: Package-Requires: ((emacs "24.3") nadvice), yes?
>> No -- experimentally that causes Emacs 26.1 to install nadvice from
>> ELPA instead of deferring to the library it already has.
> 
> IIUC that's a bug in Emacs-26.1 (and 26.2) because 26.[12] is not aware
> that it comes with nadvice-1.0.
> 
> It really should have been fixed before Emacs-26.2, but I somehow 
> forgot
> to push that commit and it lingered on a machine I only used again
> very recently :-(

Ah, ok.  I'll just stick with the new minimum version of 24.4, and side-
step the ELPA issue.  A single point-release increase in three years 
isn't
so bad.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

The origin/scratch/so-long branch (adding so-long.el) is at a release
candidate stage, and I have rebased it over the current master branch.

Since the last discussions I've added a test suite, which is passing
on all the versions I've tested, being: 24.4 (the minimum supported
version), 24.5, 25.3, 26.1, 26.2, and current master, and I've dealt
to a variety of minor and backwards-compatibility issues in the process.

As previously discussed, the ELPA package (for Emacs 24,25,26) can be
built from this code.

https://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip
also has the same code, just with a different directory structure for
the test code).

There will be some continuing work to eliminate the use of advice in
Emacs 27 (e.g. via bug#35351), but I feel that the library is in a
good state, and could potentially be merged at this point.


-Phil

Re: Performance degradation from long lines

[Phil Sainty]

I'll merge scratch/so-long to master in another week or so if there's
no objections.

I'm keen to get additional input on some of the default config, but
it'll be that much easier to get people to test and provide additional
suggestions once it's been merged.


-Phil


On 28/06/19 1:46 AM, Phil Sainty wrote:
> The origin/scratch/so-long branch (adding so-long.el) is at a release
> candidate stage, and I have rebased it over the current master branch.
> 
> Since the last discussions I've added a test suite, which is passing
> on all the versions I've tested, being: 24.4 (the minimum supported
> version), 24.5, 25.3, 26.1, 26.2, and current master, and I've dealt

Now tested with 25.1 and 25.2 as well.

> to a variety of minor and backwards-compatibility issues in the process.
> 
> As previously discussed, the ELPA package (for Emacs 24,25,26) can be
> built from this code.
> 
> https://git.savannah.nongnu.org/cgit/so-long.git/plain/so-long.el?h=wip
> also has the same code, just with a different directory structure for
> the test code).
> 
> There will be some continuing work to eliminate the use of advice in
> Emacs 27 (e.g. via bug#35351), but I feel that the library is in a
> good state, and could potentially be merged at this point.
> 
> 
> -Phil
> 

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: Eli Zaretskii <eliz@gnu.org>, mithraeum <mithraeum@protonmail.com>,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 7 Jul 2019 02:18:17 +1200
> 
> I'll merge scratch/so-long to master in another week or so if there's
> no objections.
> 
> I'm keen to get additional input on some of the default config, but
> it'll be that much easier to get people to test and provide additional
> suggestions once it's been merged.

Thanks for merging this.

I made a few minor fixes to the docs.

One comment I have is that disabling bidi-display-reordering should
probably be removed from the defaults, because doing so puts the
display engine in a state that is not being tested, and can cause
inconsistencies and even bugs (because some portions of the code were
written under the assumption that this variable is never nil).

OTOH, I'd suggest setting bidi-paragraph-direction to 'left-to-right
by default when so-long-mode is turned on.

Also, I don't understand why the defaults disable
display-line-number-mode, it AFAIK does not slow down redisplay in any
significant ways.  Do you have any evidence it should be disabled in
buffers with long lines?

IME, truncate-lines sometimes makes display of long lines _faster_, so
I'm not sure we should disable that by default.

Re: Performance degradation from long lines

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> One comment I have is that disabling bidi-display-reordering should
> probably be removed from the defaults, because doing so puts the
> display engine in a state that is not being tested, and can cause
> inconsistencies and even bugs (because some portions of the code were
> written under the assumption that this variable is never nil).

I think some users are already setting bidi-display-reordering to nil.  I
stumbled into this the other day, for example:

    You can try and see if setting bidi-display-reordering to nil
improves the speed
    for you. This removes one significant contributor to line scans,
but sadly not
    the only one.

https://emacs.stackexchange.com/questions/598/how-do-i-prevent-extremely-long-lines-making-emacs-slow

Should Emacs support that?  Should it be added to its doc string that it could
be dangerous?

Thanks,
Stefan Kangas

Re: Performance degradation from long lines

[Phil Sainty]

On 13/07/19 8:07 PM, Eli Zaretskii wrote:
> One comment I have is that disabling bidi-display-reordering should
> probably be removed from the defaults, because doing so puts the
> display engine in a state that is not being tested, and can cause
> inconsistencies and even bugs (because some portions of the code
> were written under the assumption that this variable is never nil).
>
> OTOH, I'd suggest setting bidi-paragraph-direction to 'left-to-right
> by default when so-long-mode is turned on.

That sounds good to me.  I wasn't aware that nil was an invalid value
for bidi-display-reordering.  I would suggest that its docstring be
updated in this regard.  Currently the doc is just "Non-nil means
reorder bidirectional text for display in the visual order." which to
me implies that value of nil is ok.


> Also, I don't understand why the defaults disable
> display-line-number-mode, it AFAIK does not slow down redisplay in
> any significant ways.  Do you have any evidence it should be
> disabled in buffers with long lines?

No, I'd simply included it along with the older line-numbering minor
modes.  I believe I can see a *very* slight difference, depending on
the state of display-line-number-mode, when moving around the visual
lines in a ~1MB line; however it's not significant, so I don't object
to removing it from the list.

I'm not sure that there's a benefit to the end-user in having line-
numbering enabled in such a file, though (which was the other reason
I went ahead and added all of those modes).

Some of the default minor modes are in the list because I felt the
features they provided would be redundant in a long-lines situation
(or redundant with font-lock-mode disabled), rather than because I'd
done benchmarking and established that they were notable CPU hogs.

I'd concluded that any mode or feature which scans buffer contents in
order to highlight things would be reasonable to disable by default,
and my general approach has been to err on the side of being overly
conservative in order to provide maximum benefit in the really extreme
cases.

I'm certain that the default settings can be improved though, so I'm
very happy for them to be discussed, and I hope that people will do
some experimentation in their own configs and recommend changes and
additions to the defaults, so that 27.1 ships with better coverage in
general -- the current defaults really only reflect things which I've
used personally.


> IME, truncate-lines sometimes makes display of long lines _faster_,
> so I'm not sure we should disable that by default.

This should stay.  The combination of truncate-lines being disabled
and line-move-visual being enabled is a tremendous benefit when the
user tries to move vertically from an extremely long line to the next
line.  With truncated lines, Emacs will have to scan to the end of the
line (one of my test files is a 19M JSON file in a single line*),
whereas with the settings I've used the user can happily move up and
down in the early parts of the file with no problems at all.

(*) You can find that file in this Stack Overflow question:
https://emacs.stackexchange.com/q/598 (see the 'wget' command).


-Phil

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 13 Jul 2019 11:07:08 +0200
> Cc: Phil Sainty <psainty@orcon.net.nz>, mithraeum@protonmail.com, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > One comment I have is that disabling bidi-display-reordering should
> > probably be removed from the defaults, because doing so puts the
> > display engine in a state that is not being tested, and can cause
> > inconsistencies and even bugs (because some portions of the code were
> > written under the assumption that this variable is never nil).
> 
> I think some users are already setting bidi-display-reordering to nil.  I
> stumbled into this the other day, for example:

They do that at their own risk.

> Should Emacs support that?

If someone wants to work on that, they should feel free.  I won't.

> Should it be added to its doc string that it could be dangerous?

I'm okay with documenting that, thanks.  Although the variable was
supposed to be an internal on, but I guess that genie is out of the
bottle long ago.

Re: Performance degradation from long lines

[Eli Zaretskii]

> Cc: mithraeum@protonmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Sat, 13 Jul 2019 21:33:19 +1200
> 
> On 13/07/19 8:07 PM, Eli Zaretskii wrote:
> > One comment I have is that disabling bidi-display-reordering should
> > probably be removed from the defaults, because doing so puts the
> > display engine in a state that is not being tested, and can cause
> > inconsistencies and even bugs (because some portions of the code
> > were written under the assumption that this variable is never nil).
> >
> > OTOH, I'd suggest setting bidi-paragraph-direction to 'left-to-right
> > by default when so-long-mode is turned on.
> 
> That sounds good to me.  I wasn't aware that nil was an invalid value
> for bidi-display-reordering.

It isn't invalid.  It is useful for debugging the display code, but
using it in production session is dangerous, because it causes the
code to execute control-flow paths that aren't supported in
general-purpose usage.

> > Also, I don't understand why the defaults disable
> > display-line-number-mode, it AFAIK does not slow down redisplay in
> > any significant ways.  Do you have any evidence it should be
> > disabled in buffers with long lines?
> 
> No, I'd simply included it along with the older line-numbering minor
> modes.  I believe I can see a *very* slight difference, depending on
> the state of display-line-number-mode, when moving around the visual
> lines in a ~1MB line; however it's not significant, so I don't object
> to removing it from the list.

In my measurements, the slow down is about 10% or less.

> I'm not sure that there's a benefit to the end-user in having line-
> numbering enabled in such a file, though (which was the other reason
> I went ahead and added all of those modes).

Log files is one important use case where line numbers might be of
benefit.

> > IME, truncate-lines sometimes makes display of long lines _faster_,
> > so I'm not sure we should disable that by default.
> 
> This should stay.  The combination of truncate-lines being disabled
> and line-move-visual being enabled is a tremendous benefit when the
> user tries to move vertically from an extremely long line to the next
> line.

If it's the combination that matters, I suggest to mention that in the
documentation (doc string, perhaps?), as I don't think this will be
otherwise evident.

Thanks.

Re: Performance degradation from long lines

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:
> > Should it be added to its doc string that it could be dangerous?>
> I'm okay with documenting that, thanks.  Although the variable was
> supposed to be an internal on, but I guess that genie is out of the
> bottle long ago.

How about the attached patch?  It's based on what you've said in this
thread so far.

Thanks,
Stefan Kangas

[-- Attachment #2: 0001-Add-warning-to-bidi-display-reordering-doc-string.patch --]
[-- Type: text/x-patch, Size: 1621 bytes --]

From cc3b8e1b0ecf7dca67246878f9374780685f0f5d Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sat, 13 Jul 2019 12:11:19 +0200
Subject: [PATCH] Add warning to bidi-display-reordering doc string

This explanation was given by Eli Zaretskii on emacs-devel.
For discussion, see:
https://lists.gnu.org/archive/html/emacs-devel/2019-07/msg00294.html

* src/buffer.c (syms_of_buffer): Add warning to doc string of
bidi-display-reordering to explain it could be dangerous in a
production session.
---
 src/buffer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index 209e29f0f1..9b978c131e 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -5659,8 +5659,12 @@ syms_of_buffer (void)
 
   DEFVAR_PER_BUFFER ("bidi-display-reordering",
 		     &BVAR (current_buffer, bidi_display_reordering), Qnil,
-		     doc: /* Non-nil means reorder bidirectional text for display in the visual order.  */);
-
+		     doc: /* Non-nil means reorder bidirectional text for display in the visual order.
+WARNING: This variable is useful for debugging the display code, but
+using it in a production session is dangerous, because its puts the
+display engine in a state that is not being tested.  It can cause
+inconsistencies and even bugs (because some portions of the code were
+written under the assumption that this variable is never nil).  */);
   DEFVAR_PER_BUFFER ("bidi-paragraph-start-re",
 		     &BVAR (current_buffer, bidi_paragraph_start_re), Qnil,
 		     doc: /* If non-nil, a regexp matching a line that starts OR separates paragraphs.
-- 
2.11.0

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 13 Jul 2019 12:23:20 +0200
> Cc: Phil Sainty <psainty@orcon.net.nz>, mithraeum@protonmail.com, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> 
> > I'm okay with documenting that, thanks.  Although the variable was
> > supposed to be an internal on, but I guess that genie is out of the
> > bottle long ago.
> 
> How about the attached patch?  It's based on what you've said in this
> thread so far.

Well, I guess I said too much ;-)

It should be enough to say something shorter like

  Setting this to nil is intended for use in debugging the display
  code.  Don't set to nil in normal sessions, as that is not supported.

Thanks.

Re: Performance degradation from long lines

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:

> Well, I guess I said too much ;-)
>
> It should be enough to say something shorter like
>
>   Setting this to nil is intended for use in debugging the display
>   code.  Don't set to nil in normal sessions, as that is not supported.

OK, I've attached an updated patch with that text.

Thanks,
Stefan Kangas

[-- Attachment #2: 0001-Add-warning-to-bidi-display-reordering-doc-string.patch --]
[-- Type: text/x-patch, Size: 1408 bytes --]

From cbe0a0fa166ed1a0704a40c8566a72740c37de4c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sat, 13 Jul 2019 12:11:19 +0200
Subject: [PATCH] Add warning to bidi-display-reordering doc string

This explanation was given by Eli Zaretskii on emacs-devel.
For discussion, see:
https://lists.gnu.org/archive/html/emacs-devel/2019-07/msg00294.html

* src/buffer.c (syms_of_buffer): Add warning to doc string of
bidi-display-reordering to explain that it should only be used for
debugging.
---
 src/buffer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index 209e29f0f1..859c09f29f 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -5659,8 +5659,9 @@ syms_of_buffer (void)
 
   DEFVAR_PER_BUFFER ("bidi-display-reordering",
 		     &BVAR (current_buffer, bidi_display_reordering), Qnil,
-		     doc: /* Non-nil means reorder bidirectional text for display in the visual order.  */);
-
+		     doc: /* Non-nil means reorder bidirectional text for display in the visual order.
+Setting this to nil is intended for use in debugging the display code.
+Don't set to nil in normal sessions, as that is not supported.  */);
   DEFVAR_PER_BUFFER ("bidi-paragraph-start-re",
 		     &BVAR (current_buffer, bidi_paragraph_start_re), Qnil,
 		     doc: /* If non-nil, a regexp matching a line that starts OR separates paragraphs.
-- 
2.11.0

Re: Performance degradation from long lines

[Phil Sainty]

I think it would be good if that docstring also told users that
setting bidi-paragraph-direction to 'left-to-right is what they
should probably be doing instead?



On 13/07/19 10:38 PM, Stefan Kangas wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>> Well, I guess I said too much ;-)
>>
>> It should be enough to say something shorter like
>>
>>   Setting this to nil is intended for use in debugging the display
>>   code.  Don't set to nil in normal sessions, as that is not supported.
> 
> OK, I've attached an updated patch with that text.
> 
> Thanks,
> Stefan Kangas
> 

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 13 Jul 2019 12:38:44 +0200
> Cc: Phil Sainty <psainty@orcon.net.nz>, mithraeum@protonmail.com, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> 
> >   Setting this to nil is intended for use in debugging the display
> >   code.  Don't set to nil in normal sessions, as that is not supported.
> 
> OK, I've attached an updated patch with that text.

Thanks, pushed to the emacs-26 branch.

Re: Performance degradation from long lines

[Eli Zaretskii]

> Cc: mithraeum@protonmail.com, Stefan Monnier <monnier@iro.umontreal.ca>,
>  emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Sat, 13 Jul 2019 22:58:51 +1200
> 
> I think it would be good if that docstring also told users that
> setting bidi-paragraph-direction to 'left-to-right is what they
> should probably be doing instead?

Done, thanks.

Re: Performance degradation from long lines

[Stefan Monnier]

>> That sounds good to me.  I wasn't aware that nil was an invalid value
>> for bidi-display-reordering.
>
> It isn't invalid.  It is useful for debugging the display code, but
> using it in production session is dangerous, because it causes the
> code to execute control-flow paths that aren't supported in
> general-purpose usage.

How 'bout we rename it to clarify it. 
The new name could include "debug", "internal" and/or "--".


        Stefan

Re: Performance degradation from long lines

[Stefan Kangas]

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

> >> That sounds good to me.  I wasn't aware that nil was an invalid value
> >> for bidi-display-reordering.
> >
> > It isn't invalid.  It is useful for debugging the display code, but
> > using it in production session is dangerous, because it causes the
> > code to execute control-flow paths that aren't supported in
> > general-purpose usage.
>
> How 'bout we rename it to clarify it.
> The new name could include "debug", "internal" and/or "--".

It should probably be "-internal", according to
(info "(elisp)Tips for Defining")

‘...-internal’
     The variable is intended for internal use and is defined in C code.
     (Emacs code contributed before 2018 may follow other conventions,
     which are being phased out.)

Best regards,
Stefan Kangas

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 13 Jul 2019 09:31:02 -0400
> Cc: Phil Sainty <psainty@orcon.net.nz>, mithraeum@protonmail.com,
>  emacs-devel@gnu.org
> 
> >> That sounds good to me.  I wasn't aware that nil was an invalid value
> >> for bidi-display-reordering.
> >
> > It isn't invalid.  It is useful for debugging the display code, but
> > using it in production session is dangerous, because it causes the
> > code to execute control-flow paths that aren't supported in
> > general-purpose usage.
> 
> How 'bout we rename it to clarify it. 
> The new name could include "debug", "internal" and/or "--".

And then introduce an alias or it, and maintain those for years?
Doesn't sound worth the hassle.

Re: Performance degradation from long lines

[Stefan Monnier]

> And then introduce an alias for it,

I was thinking of *not* having an alias for it.


        Stefan

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 13 Jul 2019 10:17:18 -0400
> Cc: psainty@orcon.net.nz, mithraeum@protonmail.com, emacs-devel@gnu.org
> 
> > And then introduce an alias for it,
> 
> I was thinking of *not* having an alias for it.

I don't see how we could avoid that, this variable is quite old and is
known too widely to simply remove it.

Re: Performance degradation from long lines

[Stefan Monnier]

>> I was thinking of *not* having an alias for it.
> I don't see how we could avoid that,

Just like that: we rename it and announce the change in etc/NEWS.

> this variable is quite old and is known too widely to simply remove it.

I think the risk is very low.


        Stefan

Re: Performance degradation from long lines

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 13 Jul 2019 18:22:43 -0400
> Cc: psainty@orcon.net.nz, mithraeum@protonmail.com, emacs-devel@gnu.org
> 
> >> I was thinking of *not* having an alias for it.
> > I don't see how we could avoid that,
> 
> Just like that: we rename it and announce the change in etc/NEWS.

Sorry, that would be burying our head in the sand.

Re: Performance degradation from long lines

[Dmitry Gutov]

On 14.07.2019 8:39, Eli Zaretskii wrote:

>> Just like that: we rename it and announce the change in etc/NEWS.
> 
> Sorry, that would be burying our head in the sand.

If the variable is only used out there for performance purposes, or 
temporarily for debugging, I don't think it would lead to any 
significant breakage, even if the former name suddenly stops working.

Re: Performance degradation from long lines

[Eli Zaretskii]

> Cc: psainty@orcon.net.nz, mithraeum@protonmail.com, emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 15 Jul 2019 16:12:26 +0300
> 
> On 14.07.2019 8:39, Eli Zaretskii wrote:
> 
> >> Just like that: we rename it and announce the change in etc/NEWS.
> > 
> > Sorry, that would be burying our head in the sand.
> 
> If the variable is only used out there for performance purposes, or 
> temporarily for debugging, I don't think it would lead to any 
> significant breakage, even if the former name suddenly stops working.

You are missing the point, I think: this variable's name is known
widely enough to make it a de-facto public interface.  Changing its
name without an alias will cause breakage to many people's
configurations.  However I dislike use of this variable in people's
configurations, I cannot break them all just because I dislike that.

IOW, it's a question of being fair to our users, even if they do
something we don't endorse.

Re: Performance degradation from long lines

[Stefan Monnier]

> You are missing the point, I think: this variable's name is known
> widely enough to make it a de-facto public interface.  Changing its
> name without an alias will cause breakage to many people's
> configurations.  However I dislike use of this variable in people's
> configurations, I cannot break them all just because I dislike that.

I think we all understand that.  But some of us think the breakage will
be minimal.

My cursory search through github seemed to indicate that it's very
rarely usedin Elisp packages and the few times it's used it comes with
a comment along the lines of "disable bidi for a marginal performance
gain".

As for such settings in people's .emacs, my educated guess is that they
date back to Emacs-23's introduction to work around problems in specific
modes that have since solved those problems (typically by setting
bidi-paragraph-direction).

IOW, any noticeable breakage that might result likely points to
a *real* bug.


        Stefan

Re: Performance degradation from long lines

[Clément Pit-Claudel]

On 2019-07-18 10:48, Stefan Monnier wrote:
> As for such settings in people's .emacs, my educated guess is that they
> date back to Emacs-23's introduction to work around problems in specific
> modes that have since solved those problems (typically by setting
> bidi-paragraph-direction).

It's also suggested in the top-voted answer on how to make Emacs faster with long lines on emacs.stackexchange (https://emacs.stackexchange.com/questions/598/how-do-i-prevent-extremely-long-lines-making-emacs-slow), so I'd expect many people to have it in their config for that reason.

Re: Performance degradation from long lines

[Stefan Monnier]

>> As for such settings in people's .emacs, my educated guess is that they
>> date back to Emacs-23's introduction to work around problems in specific
>> modes that have since solved those problems (typically by setting
>> bidi-paragraph-direction).
>
> It's also suggested in the top-voted answer on how to make Emacs faster with
> long lines on emacs.stackexchange
> (https://emacs.stackexchange.com/questions/598/how-do-i-prevent-extremely-long-lines-making-emacs-slow),
> so I'd expect many people to have it in their config for that reason.

Yes, but making it ineffective won't break their config.
It will merely make long-lines a bit slower in some cases.
And we can point to so-long for a better solution.
Nothing problematic, IMO.


        Stefan

Re: Performance degradation from long lines

[Andy Moreton]

On Thu 18 Jul 2019, Stefan Monnier wrote:

>> You are missing the point, I think: this variable's name is known
>> widely enough to make it a de-facto public interface.  Changing its
>> name without an alias will cause breakage to many people's
>> configurations.  However I dislike use of this variable in people's
>> configurations, I cannot break them all just because I dislike that.
>
> I think we all understand that.  But some of us think the breakage will
> be minimal.

It is perfectly reasonable to deprecate the variable. However it is
better to show the user how to fix the problem in their code than just
break previously working code (for some definition of working).

How about renaming all occurences of `bidi-display-reordering' to
`debug--bidi-display-reordering' or similar, and then add something like:

  (make-obsolete-variable 'bidi-display-reordering
   "not used. To disable reordering, use `(setq bidi-paragraph-direction
   'left-to-right)' instead." "27.1")

That would less hostile to users than immediate removal.

    AndyM