GNU bug report logs - #64391
30.0.50; buffer narrowing slowdown regression in emacs 29

Previous Next

To reply to this bug, email your comments to 64391 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andrew Cohen <acohen <at> ust.hk>
To: bug-gnu-emacs <at> gnu.org
Cc: Gregory Heytings <gregory <at> heytings.org>
Subject: 30.0.50; buffer narrowing slowdown regression in emacs 29
Date: Sat, 01 Jul 2023 08:15:25 +0800

[Message part 1 (text/plain, inline)]

I have noticed a huge slowdown in parsing email headers in gnus. After
some debugging with help from Mattias Engdegård, the problem has been
traced to the narrowing code introduced in

commit ba9315b1641b483f2bf843c38dcdba0cd1643a55 (HEAD)
Merge: aef803d6c3d a3b654e069e
Author: Gregory Heytings <gregory <at> heytings.org>
Date:   Thu Nov 24 14:21:30 2022 +0100

    Merge master into feature/improved-locked-narrowing.

Gnus populates a buffer with headers from a set of email messages and
then parses them by: narrowing the buffer to the headers for an
individual message; parsing the headers; widening; and then repeating
for the next message.  As part of the parsing the headers are "unfolded"
so that each header doesn't include line breaks. I noticed that for a
long list of messages (10,000) this takes between one and two orders of
magnitude more time in Emacs 30 than in Emacs 28. Unfolding all the
headers in the full buffer before the parsing process removes most of
the slowdown. The slowdown seems to grow quadratically with the size of
the buffer.

The problem seems fairly general and Mattias has produced a simple test
case to demonstrate the issue (code at the end of this message):

1. Create a buffer with 100,000 lines each containing two characters  "ab".
2. Loop through the buffer narrowing to each line, and immediately widening
    back to the full buffer (so no change is made to the buffer
    contents).
3.  Loop through the buffer removing the first character of each line.

This takes a very long time compared with reversing the order of 2. and 3.

Emacs 28.2
Modify after narrowing:  0.173 s elapsed, 0 GCs, 0.000 s GC, 0.173 s non-GC
Modify before narrowing: 0.160 s elapsed, 0 GCs, 0.000 s GC, 0.160 s non-GC

Emacs 29.0.92
Modify after narrowing:  4.454 s elapsed, 9 GCs, 0.115 s GC, 4.339 s non-GC
Modify before narrowing: 0.291 s elapsed, 9 GCs, 0.114 s GC, 0.177 s non-GC

The problem may be the creation of markers in the narrowing process that
slows down further mutation. (Also note that the new code has introduced
some significant GC that was previously absent).

[abench2.el (application/emacs-lisp, attachment)]

[Message part 3 (text/plain, inline)]

-- 
Andrew Cohen

Message #8 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Cohen <acohen <at> ust.hk>
To: 64391 <at> debbugs.gnu.org
Subject: bug 64391 wrong git info
Date: Sat, 01 Jul 2023 10:45:35 +0800

Sorry, I didn't give the right git commit info.

The last GOOD commit was

Commit ba9315b1641b483f2bf843c38dcdba0cd1643a55
Emacs 29.0.50
Modify after narrowing:  0.080 s elapsed, 0 GCs, 0.000 s GC, 0.080 s non-GC
Modify before narrowing: 0.073 s elapsed, 0 GCs, 0.000 s GC, 0.073 s non-GC

The first BAD commit showed a slowdown of a significant factor
Commit 9dee6df39cd14be78ff96cb24169842f4772488a
Emacs 29.0.50
Modify after narrowing:  0.734 s elapsed, 9 GCs, 0.051 s GC, 0.684 s non-GC
Modify before narrowing: 0.153 s elapsed, 9 GCs, 0.052 s GC, 0.102 s non-GC

Then a further order of magnitude slowdown shows up around
 Commit 321d4e61551a0f6dfb1abfc0b54e6177735bde58
Modify after narrowing:  7.954 s elapsed, 3 GCs, 0.022 s GC, 7.932 s non-GC
Modify before narrowing: 0.122 s elapsed, 3 GCs, 0.023 s GC, 0.099 s non-GC


Hope this info is helpful.
-- 
Andrew Cohen

Message #11 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Cohen <acohen <at> ust.hk>
To: 64391 <at> debbugs.gnu.org
Cc: Mattias Engdegård <mattiase <at> acm.org>,
 Gregory Heytings <gregory <at> heytings.org>
Subject: more info
Date: Sat, 01 Jul 2023 12:59:20 +0800

Mattias had flagged this code at line 2948 in editfns.c as a likely culprit. 

/* Record the accessible range of the buffer when narrow-to-region
     is called, that is, before applying the narrowing.  That
     information is used only by internal--label-restriction.  */
  Fset (Qoutermost_restriction, list3 (Qoutermost_restriction,
				       Fpoint_min_marker (),
				       Fpoint_max_marker ()));

Just for fun I simply commented it out (since the test code doesn't use
labeled restrictions) and ran the test code, and the problem disappears:

Emacs 30.0.50
Modify after narrowing:  0.076 s elapsed, 0 GCs, 0.000 s GC, 0.076 s non-GC
Modify before narrowing: 0.070 s elapsed, 0 GCs, 0.000 s GC, 0.070 s non-GC
-- 
Andrew Cohen

Message #14 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrew Cohen <acohen <at> ust.hk>
Cc: 64391 <at> debbugs.gnu.org, gregory <at> heytings.org
Subject: Re: bug#64391: 30.0.50;
 buffer narrowing slowdown regression in emacs 29
Date: Sat, 01 Jul 2023 10:07:00 +0300

> Cc: Gregory Heytings <gregory <at> heytings.org>
> From: Andrew Cohen <acohen <at> ust.hk>
> Date: Sat, 01 Jul 2023 08:15:25 +0800
> 
> I have noticed a huge slowdown in parsing email headers in gnus. After
> some debugging with help from Mattias Engdegård, the problem has been
> traced to the narrowing code introduced in
> 
> commit ba9315b1641b483f2bf843c38dcdba0cd1643a55 (HEAD)
> Merge: aef803d6c3d a3b654e069e
> Author: Gregory Heytings <gregory <at> heytings.org>
> Date:   Thu Nov 24 14:21:30 2022 +0100
> 
>     Merge master into feature/improved-locked-narrowing.
> 
> Gnus populates a buffer with headers from a set of email messages and
> then parses them by: narrowing the buffer to the headers for an
> individual message; parsing the headers; widening; and then repeating
> for the next message.  As part of the parsing the headers are "unfolded"
> so that each header doesn't include line breaks. I noticed that for a
> long list of messages (10,000) this takes between one and two orders of
> magnitude more time in Emacs 30 than in Emacs 28. Unfolding all the
> headers in the full buffer before the parsing process removes most of
> the slowdown. The slowdown seems to grow quadratically with the size of
> the buffer.
> 
> The problem seems fairly general and Mattias has produced a simple test
> case to demonstrate the issue (code at the end of this message):
> 
> 1. Create a buffer with 100,000 lines each containing two characters  "ab".
> 2. Loop through the buffer narrowing to each line, and immediately widening
>     back to the full buffer (so no change is made to the buffer
>     contents).
> 3.  Loop through the buffer removing the first character of each line.
> 
> This takes a very long time compared with reversing the order of 2. and 3.

Does this problem go away if you set long-line-threshold to nil?

Can you show a profile of the processing that takes a long time?

Message #17 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Cohen <acohen <at> ust.hk>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64391 <at> debbugs.gnu.org, gregory <at> heytings.org
Subject: Re: bug#64391: 30.0.50; buffer narrowing slowdown regression in
 emacs 29
Date: Sat, 01 Jul 2023 15:31:53 +0800

>>>>> "EZ" == Eli Zaretskii <eliz <at> gnu.org> writes:

[...]

    EZ> Does this problem go away if you set long-line-threshold to nil?

No.

    EZ> Can you show a profile of the processing that takes a long time?

Not exactly sure how to answer this. The test case just goes through the
buffer narrowing and widening (without modifying the buffer).  This
takes very little time. Then the following takes a very  long time:

      (goto-char (point-min))
      (while (not (eobp))
        (delete-char 1)
        (forward-line))

-- 
Andrew Cohen

Message #20 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrew Cohen <acohen <at> ust.hk>
Cc: 64391 <at> debbugs.gnu.org, gregory <at> heytings.org
Subject: Re: bug#64391: 30.0.50; buffer narrowing slowdown regression in
 emacs 29
Date: Sat, 01 Jul 2023 11:09:01 +0300

> From: Andrew Cohen <acohen <at> ust.hk>
> Cc: 64391 <at> debbugs.gnu.org,  gregory <at> heytings.org
> Date: Sat, 01 Jul 2023 15:31:53 +0800
> 
> >>>>> "EZ" == Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>     EZ> Can you show a profile of the processing that takes a long time?
> 
> Not exactly sure how to answer this. The test case just goes through the
> buffer narrowing and widening (without modifying the buffer).  This
> takes very little time. Then the following takes a very  long time:
> 
>       (goto-char (point-min))
>       (while (not (eobp))
>         (delete-char 1)
>         (forward-line))

I asked for the profile produce by "M-x profiler-start RET RET" for
this very long processing.  is it feasible to produce such a profile?

Message #23 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Andrew Cohen <acohen <at> ust.hk>
Cc: 64391 <at> debbugs.gnu.org, Gregory Heytings <gregory <at> heytings.org>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: bug#64391: buffer narrowing slowdown regression in emacs 29 
Date: Sat, 1 Jul 2023 13:37:08 +0200

[Message part 1 (text/plain, inline)]

1 juli 2023 kl. 06.59 skrev Andrew Cohen <acohen <at> ust.hk>:

> Just for fun I simply commented it out (since the test code doesn't use
> labeled restrictions) and ran the test code, and the problem disappears:

The attached patch combines narrow-to-region and internal--label-restriction into a single function, internal--narrow-to-region. (We could also add the label as an optional argument to narrow-to-region.)

As a result, the up-front consing and marker allocation disappear entirely for normal (unlabelled) narrowing, as does the expensive buffer-local `outermost-restriction` variable.

Labelled narrowing is almost as expensive as before but is, we hope, less common. Maybe we can work separately on reducing its cost.

(And to those reading the patch: yes, some work on doc strings is needed. Suggestions welcome.)

[narrow-to-region-with-label.diff (application/octet-stream, attachment)]

Message #26 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, gregory <at> heytings.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 01 Jul 2023 15:08:36 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Sat, 1 Jul 2023 13:37:08 +0200
> Cc: 64391 <at> debbugs.gnu.org,
>  Gregory Heytings <gregory <at> heytings.org>,
>  Eli Zaretskii <eliz <at> gnu.org>
> 
> 1 juli 2023 kl. 06.59 skrev Andrew Cohen <acohen <at> ust.hk>:
> 
> > Just for fun I simply commented it out (since the test code doesn't use
> > labeled restrictions) and ran the test code, and the problem disappears:
> 
> The attached patch combines narrow-to-region and internal--label-restriction into a single function, internal--narrow-to-region. (We could also add the label as an optional argument to narrow-to-region.)

It does more than that, so I'd appreciate a more detailed description
of the changes and their rationale.

Thanks.

Message #29 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Andrew Cohen <acohen <at> ust.hk>, 64391 <at> debbugs.gnu.org,
 Gregory Heytings <gregory <at> heytings.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 1 Jul 2023 14:52:58 +0200

1 juli 2023 kl. 14.08 skrev Eli Zaretskii <eliz <at> gnu.org>:

>> The attached patch combines narrow-to-region and internal--label-restriction into a single function, internal--narrow-to-region. (We could also add the label as an optional argument to narrow-to-region.)
> 
> It does more than that, so I'd appreciate a more detailed description
> of the changes and their rationale.

Actually that's just what it does. Here is a tentative commit message:

    Fix severe narrowing performance bug (regression from emacs 28)
    
    In Emacs 29, `narrow-to-region` conses and creates markers
    unconditionally on the offchance that a call to
    `internal--label-restriction` would need it, which is only rarely the
    case (in `with-restriction` with a :label argument).
    
    As a remedy we fuse the two functions to one,
    `internal--narrow-to-region`, and only perform the costly consing and
    marker creation for labelled narrowing. (Bug#64391)
    
    * lisp/subr.el (internal--with-restriction):
    * src/editfns.c (labeled_narrow_to_region):
    Call `internal--narrow-to-region` instead of `narrow-to-region`
    followed by `internal--label-restriction`.
    (Fwiden): Remove assignment to eliminated `outermost-restriction`.
    (Fnarrow_to_region): Reduce to a stub that calls the original
    function, now named...
    (Finternal__narrow_to_region): ...this, which takes an added
    `label` argument and includes at the end the optimised body of...
    (Finternal__label_restriction): ...this function, now removed
    since it has been entirely absorbed.
    (syms_of_editfns): Remove the `outermost-restriction` buffer-local
    variable as it was only used to convey data from `narrow-to-region`
    to `internal--label-restriction` called immediately afterwards.

And again, if anyone would prefer an optional `label` argument to `narrow-to-region` then that would be fine too. It depends a little on whether we want to expose that functionality to the user in that function, or as now keep it in `with-restriction` only.

Message #32 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Andrew Cohen <acohen <at> ust.hk>, 
 Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 02 Jul 2023 07:35:13 +0000

Thanks Andrew for the bug report, and Mattias for the patch.

I'll have a look in a few hours.

Message #35 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Andrew Cohen <acohen <at> ust.hk>, 64391 <at> debbugs.gnu.org,
 Gregory Heytings <gregory <at> heytings.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29 
Date: Sun, 2 Jul 2023 11:37:53 +0200

[Message part 1 (text/plain, inline)]

How expensive markers can be when a buffer is modified is often unappreciated. Here are the results of a benchmark to illustrate the cost (code attached).
A buffer contains "abababab...", we optionally add a marker to every other position, and then time the deletion of all the "a"s. Results:

|   size | without markers | with markers |
|--------+-----------------+--------------|
|    500 |           0.000 |        0.001 |
|   1000 |           0.001 |        0.004 |
|   5000 |           0.002 |        0.089 |
|  10000 |           0.005 |        0.363 |
|  50000 |           0.024 |        9.527 |
| 100000 |           0.049 |       58.048 |

Now there's something for the curve-fitters among you.

[abench4.el (application/octet-stream, attachment)]

Message #38 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Wed, 05 Jul 2023 14:57:52 +0300

> Date: Sun, 02 Jul 2023 07:35:13 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: Eli Zaretskii <eliz <at> gnu.org>, 64391 <at> debbugs.gnu.org, 
>     Stefan Monnier <monnier <at> iro.umontreal.ca>
> 
> 
> Thanks Andrew for the bug report, and Mattias for the patch.
> 
> I'll have a look in a few hours.

Any progress there?

Message #41 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Thu, 06 Jul 2023 14:41:13 +0000

>> Thanks Andrew for the bug report, and Mattias for the patch.
>>
>> I'll have a look in a few hours.
>
> Any progress there?
>

Sorry for the delay, I'm looking at this right now.

Message #44 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Thu, 06 Jul 2023 17:33:13 +0000

I pushed a proposed change to the scratch/bug64391 branch.

It contains three commits:

- b741dc7fcd which is the minimal possible commit to fix this bug

- 01fb898420 which moves a few statements from the callee 
(internal--label-restriction) to its only caller 
(internal--labeled-narrow-to-region), and simplifies the code accordingly

- c52ade305e which is optional, and makes the symmetrical change for the 
widening case

Eli and Mattias, could you perhaps have a look?

Thanks.

Message #47 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Thu, 06 Jul 2023 21:22:07 +0300

> Date: Thu, 06 Jul 2023 17:33:13 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     monnier <at> iro.umontreal.ca
> 
> 
> I pushed a proposed change to the scratch/bug64391 branch.
> 
> It contains three commits:
> 
> - b741dc7fcd which is the minimal possible commit to fix this bug
> 
> - 01fb898420 which moves a few statements from the callee 
> (internal--label-restriction) to its only caller 
> (internal--labeled-narrow-to-region), and simplifies the code accordingly
> 
> - c52ade305e which is optional, and makes the symmetrical change for the 
> widening case
> 
> Eli and Mattias, could you perhaps have a look?

How are those different from what Mattias proposed?  Is this a
completely different set of changes, or is it what Mattias suggested,
just in several separate parts?

If these are different, did you time the results in the use case(s)
where the slow-down was detected and reported, and compared the two
proposals performance-wise?

Message #50 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Thu, 06 Jul 2023 18:45:50 +0000

>> I pushed a proposed change to the scratch/bug64391 branch.
>>
>> It contains three commits:
>>
>> - b741dc7fcd which is the minimal possible commit to fix this bug
>>
>> - 01fb898420 which moves a few statements from the callee 
>> (internal--label-restriction) to its only caller 
>> (internal--labeled-narrow-to-region), and simplifies the code 
>> accordingly
>>
>> - c52ade305e which is optional, and makes the symmetrical change for 
>> the widening case
>>
>> Eli and Mattias, could you perhaps have a look?
>
> How are those different from what Mattias proposed?  Is this a 
> completely different set of changes, or is it what Mattias suggested, 
> just in several separate parts?
>

It resembles what Mattias proposed of course (remove the negative effect 
of setting a buffer-local variable in all calls to narrow-to-region, and 
limit it to case when labeled narrowing is used), but it's nonetheless 
different.  The essential change is isolated in the first commit, and in 
the second commit, instead of moving the body of Fnarrow_to_region to 
another function with an additional (third) argument, I use another 
separate function which calls Fnarrow_to_region.  I believe the result is 
clearer.

>
> If these are different, did you time the results in the use case(s) 
> where the slow-down was detected and reported, and compared the two 
> proposals performance-wise?
>

Yes, there is no difference, performance-wise:

- with Mattias' patch:

Modify after narrowing:  0.114 s elapsed, 0 GCs, 0.000 s GC, 0.114 s non-GC
Modify before narrowing: 0.103 s elapsed, 0 GCs, 0.000 s GC, 0.103 s non-GC

- with the code in scratch/bug64391:

Modify after narrowing:  0.109 s elapsed, 0 GCs, 0.000 s GC, 0.109 s non-GC
Modify before narrowing: 0.102 s elapsed, 0 GCs, 0.000 s GC, 0.102 s non-GC

(The small differences are not meaningful, other runs show different 
values in the same order of magnitude.)

Message #53 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29 
Date: Fri, 7 Jul 2023 11:30:11 +0200

6 juli 2023 kl. 20.45 skrev Gregory Heytings <gregory <at> heytings.org>:

>> How are those different from what Mattias proposed?  Is this a completely different set of changes, or is it what Mattias suggested, just in several separate parts?
>> 
> 
> It resembles what Mattias proposed of course (remove the negative effect of setting a buffer-local variable in all calls to narrow-to-region, and limit it to case when labeled narrowing is used), but it's nonetheless different.  The essential change is isolated in the first commit, and in the second commit, instead of moving the body of Fnarrow_to_region to another function with an additional (third) argument, I use another separate function which calls Fnarrow_to_region.  I believe the result is clearer.

Gregory's first two patches and mine are indeed equivalent up to refactoring; either would do. The moving parts are the same, so performance shouldn't differ noticeably. Neither appears to tie our hands for future changes as the interfaces are all internal.

The third of Gregory's patches appears mainly cosmetic. Maybe that one is less important for emacs-29.

I have one question about the new `with-restriction` macro: if it is intended as a convenience macro for unlabelled narrowing as well, shouldn't it then expand to the obvious

  (save-restriction
    (narrow-to-region BEG END)
    BODY)

instead the much more expensive call to a helper function?

The documentation should also make it clear that `nil` is not a valid label.

Message #56 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 10:00:48 +0000

[Message part 1 (text/plain, inline)]

Thanks for your feedback!

>
> I have one question about the new `with-restriction` macro: if it is 
> intended as a convenience macro for unlabelled narrowing as well, 
> shouldn't it then expand to the obvious
>
>  (save-restriction
>    (narrow-to-region BEG END)
>    BODY)
>
> instead the much more expensive call to a helper function?
>

Yes.  In fact now that we have a separate function for the labeled case, 
we can remove the funcall in both cases.  Patch attached.

>
> The documentation should also make it clear that `nil` is not a valid 
> label.
>

Is that not already clear in the docstring, which says "in which LABEL is 
a symbol"?

[Simplify-with-restriction-and-without-restriction.patch (text/x-diff, attachment)]

Message #59 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 13:23:11 +0300

> Date: Fri, 07 Jul 2023 10:00:48 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, 
>     monnier <at> iro.umontreal.ca, bugs <at> gnus.org
> 
> > I have one question about the new `with-restriction` macro: if it is 
> > intended as a convenience macro for unlabelled narrowing as well, 
> > shouldn't it then expand to the obvious
> >
> >  (save-restriction
> >    (narrow-to-region BEG END)
> >    BODY)
> >
> > instead the much more expensive call to a helper function?
> 
> Yes.  In fact now that we have a separate function for the labeled case, 
> we can remove the funcall in both cases.  Patch attached.

You are killing me.

Message #62 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, gregory <at> heytings.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 13:27:59 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Fri, 7 Jul 2023 11:30:11 +0200
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>  acohen <at> ust.hk,
>  64391 <at> debbugs.gnu.org,
>  monnier <at> iro.umontreal.ca
> 
> Gregory's first two patches and mine are indeed equivalent up to refactoring; either would do. The moving parts are the same, so performance shouldn't differ noticeably. Neither appears to tie our hands for future changes as the interfaces are all internal.

I was considering applying only the first of Gregory's patches,
without the second.  WDYT?

Message #65 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 10:31:01 +0000

>>> I have one question about the new `with-restriction` macro: if it is 
>>> intended as a convenience macro for unlabelled narrowing as well, 
>>> shouldn't it then expand to the obvious
>>>
>>>  (save-restriction
>>>    (narrow-to-region BEG END)
>>>    BODY)
>>>
>>> instead the much more expensive call to a helper function?
>>
>> Yes.  In fact now that we have a separate function for the labeled 
>> case, we can remove the funcall in both cases.  Patch attached.
>
> You are killing me.
>

???  That wasn't meant for the release branch.

Message #68 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, gregory <at> heytings.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 7 Jul 2023 13:27:12 +0200

7 juli 2023 kl. 12.27 skrev Eli Zaretskii <eliz <at> gnu.org>:

> I was considering applying only the first of Gregory's patches,
> without the second.  WDYT?

I don't think that makes much sense -- the division between the two is pretty much artificial.

The third patch can safely be left out in emacs-29.

Message #71 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 7 Jul 2023 13:33:32 +0200

7 juli 2023 kl. 12.00 skrev Gregory Heytings <gregory <at> heytings.org>:

> Yes.  In fact now that we have a separate function for the labeled case, we can remove the funcall in both cases.  Patch attached.

Thank you, but are we sure that save-restriction would be used for labelled restrictions in the future? Maybe we should just start with the unlabelled case.

>> The documentation should also make it clear that `nil` is not a valid label.
> 
> Is that not already clear in the docstring, which says "in which LABEL is a symbol"?

Well, nil is a symbol.

Message #74 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 15:41:35 +0300

> Date: Fri, 07 Jul 2023 10:31:01 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     monnier <at> iro.umontreal.ca, bugs <at> gnus.org
> 
> 
> >>> I have one question about the new `with-restriction` macro: if it is 
> >>> intended as a convenience macro for unlabelled narrowing as well, 
> >>> shouldn't it then expand to the obvious
> >>>
> >>>  (save-restriction
> >>>    (narrow-to-region BEG END)
> >>>    BODY)
> >>>
> >>> instead the much more expensive call to a helper function?
> >>
> >> Yes.  In fact now that we have a separate function for the labeled 
> >> case, we can remove the funcall in both cases.  Patch attached.
> >
> > You are killing me.
> >
> 
> ???  That wasn't meant for the release branch.

Then what is?

Message #77 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 12:42:28 +0000

>> Yes.  In fact now that we have a separate function for the labeled 
>> case, we can remove the funcall in both cases.  Patch attached.
>
> Thank you, but are we sure that save-restriction would be used for 
> labelled restrictions in the future? Maybe we should just start with the 
> unlabelled case.
>

I'm sorry, I don't understand your question.  The only way (except by 
using internal functions) to enter a labeled restriction is to use the 
with-restriction special form with its optional label argument.  That form 
has a save-restriction since day one.

>>> The documentation should also make it clear that `nil` is not a valid 
>>> label.
>>
>> Is that not already clear in the docstring, which says "in which LABEL 
>> is a symbol"?
>
> Well, nil is a symbol.
>

Indeed, but I'd say it's clear enough from the context that "symbol" means 
a quoted symbol here.

Message #80 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 12:46:24 +0000

>> ???  That wasn't meant for the release branch.
>
> Then what is?
>

I was replying to Mattias who said, rightly, that using a funcall is not 
optimal here, and that we could do better.  I suggested a patch, but that 
optimization is independent of this bug.

Message #83 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, gregory <at> heytings.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 15:50:42 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Fri, 7 Jul 2023 13:27:12 +0200
> Cc: gregory <at> heytings.org,
>  acohen <at> ust.hk,
>  64391 <at> debbugs.gnu.org,
>  monnier <at> iro.umontreal.ca
> 
> 7 juli 2023 kl. 12.27 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> > I was considering applying only the first of Gregory's patches,
> > without the second.  WDYT?
> 
> I don't think that makes much sense -- the division between the two is pretty much artificial.

It makes sense to me, because it minimizes changes in the code, which
at this stage in the pretest is something very important, IMO.

Message #86 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org,
 Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 11:49:19 -0400

> I'm sorry, I don't understand your question.  The only way (except by using
> internal functions) to enter a labeled restriction is to use the
> with-restriction special form with its optional label argument.

Nitpick: it's not a special form, it's a macro.  There's a *big*
difference because adding a special form requires changing
`macroexpand-all`, the compiler, yadayada, and it introduces backward
incompatibilities with packages doing their own code-walks.

> Indeed, but I'd say it's clear enough from the context that "symbol" means
> a quoted symbol here.

Other nitpick: nil can also be quoted :-)
BTW, "LABEL is a symbol" sends the wrong message (a quoted
symbol evaluates to a symbol but it's not itself a symbol).
IOW the docstring should clarify that LABEL is an expression that's
evaluated at runtime (and should return a symbol).
While I'm here, is it important that LABEL evaluates to a symbol?
Or is it like `catch/throw` where we expect most uses to use a symbol
but where any other (non-nil) value works as well?


        Stefan


diff --git a/lisp/subr.el b/lisp/subr.el
index 85adef5b689..f8a34796584 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3964,11 +3964,11 @@ with-restriction
 
 The current restrictions, if any, are restored upon return.
 
-When the optional :label LABEL argument is present, in which
-LABEL is a symbol, inside BODY, `narrow-to-region' and `widen'
-can be used only within the START and END limits.  To gain access
-to other portions of the buffer, use `without-restriction' with the
-same LABEL argument.
+When the optional :label LABEL argument is present (in which
+LABEL evaluates to a non-nil symbol) inside BODY, `narrow-to-region'
+and `widen' can be used only within the START and END limits.
+To gain access to other portions of the buffer, use `without-restriction'
+with the same LABEL argument.
 
 \(fn START END [:label LABEL] BODY)"
   (declare (indent 2) (debug t))

Message #89 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, gregory <at> heytings.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29 
Date: Fri, 7 Jul 2023 18:49:56 +0200

7 juli 2023 kl. 14.50 skrev Eli Zaretskii <eliz <at> gnu.org>:

>>> I was considering applying only the first of Gregory's patches,
>>> without the second.  WDYT?
>> 
>> I don't think that makes much sense -- the division between the two is pretty much artificial.
> 
> It makes sense to me, because it minimizes changes in the code, which
> at this stage in the pretest is something very important, IMO.

Oh, I certainly appreciate the importance of conservatism in changes on the release branch, but sensibly so -- it's not a game of code golf. Only applying half of the intended change leaves the code in an ugly state, and does in fact not minimise risk at all, quite the opposite.

Message #92 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, gregory <at> heytings.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Fri, 07 Jul 2023 21:22:01 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Fri, 7 Jul 2023 18:49:56 +0200
> Cc: gregory <at> heytings.org,
>  acohen <at> ust.hk,
>  64391 <at> debbugs.gnu.org,
>  monnier <at> iro.umontreal.ca
> 
> 7 juli 2023 kl. 14.50 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> >>> I was considering applying only the first of Gregory's patches,
> >>> without the second.  WDYT?
> >> 
> >> I don't think that makes much sense -- the division between the two is pretty much artificial.
> > 
> > It makes sense to me, because it minimizes changes in the code, which
> > at this stage in the pretest is something very important, IMO.
> 
> Oh, I certainly appreciate the importance of conservatism in changes
> on the release branch, but sensibly so -- it's not a game of code
> golf. Only applying half of the intended change leaves the code in
> an ugly state,

Ugly is in the eyes of the beholder.  The release of Emacs 29 was
already delayed twice by this feature, so please forgive me if my
sense of aesthetics in this regard is beginning to fade.

> and does in fact not minimise risk at all, quite the opposite.

How so?  If the patch solves the problem, the problem is solved,
period.  Where's the risk in not applying unnecessary changes?

Message #95 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 08 Jul 2023 11:01:49 +0300

> Date: Thu, 06 Jul 2023 17:33:13 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     monnier <at> iro.umontreal.ca
> 
> 
> I pushed a proposed change to the scratch/bug64391 branch.
> 
> It contains three commits:
> 
> - b741dc7fcd which is the minimal possible commit to fix this bug
> 
> - 01fb898420 which moves a few statements from the callee 
> (internal--label-restriction) to its only caller 
> (internal--labeled-narrow-to-region), and simplifies the code accordingly
> 
> - c52ade305e which is optional, and makes the symmetrical change for the 
> widening case

OK, please merge the first two commits to the release branch, and
thanks.

Message #98 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 08 Jul 2023 19:41:03 +0000

>> I pushed a proposed change to the scratch/bug64391 branch.
>>
>> It contains three commits:
>>
>> - b741dc7fcd which is the minimal possible commit to fix this bug
>>
>> - 01fb898420 which moves a few statements from the callee 
>> (internal--label-restriction) to its only caller 
>> (internal--labeled-narrow-to-region), and simplifies the code 
>> accordingly
>>
>> - c52ade305e which is optional, and makes the symmetrical change for 
>> the widening case
>
> OK, please merge the first two commits to the release branch, and 
> thanks.
>

Thanks, now done.

I'll merge the third commit, and the other patch suggested by Mattias, to 
master.

Message #101 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 eliz <at> gnu.org, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 08 Jul 2023 20:58:44 +0000

>> I'm sorry, I don't understand your question.  The only way (except by 
>> using internal functions) to enter a labeled restriction is to use the 
>> with-restriction special form with its optional label argument.
>
> Nitpick: it's not a special form, it's a macro.  There's a *big* 
> difference because adding a special form requires changing 
> `macroexpand-all`, the compiler, yadayada, and it introduces backward 
> incompatibilities with packages doing their own code-walks.
>

TIL.  I thought that "special form" and "macro" were more or less 
synonyms.  The manual describes lambda, prog2, setq-default, dlet, letrec, 
named-let, with-suppressed-warnings, with-no-warnings, with-restriction 
and without-restriction as "special forms", although they are in fact 
macros.  That being said, from a Elisp programmer viewpoint, special forms 
and macros are similar, and AFAIU one could say that a special form is a 
macro written in C, and a macro is a special form written in Elisp.

Should the above occurrences of "special form" be corrected in the 
manuals?

>> Indeed, but I'd say it's clear enough from the context that "symbol" 
>> means a quoted symbol here.
>
> Other nitpick: nil can also be quoted :-)
>

I knew someone would say that ;-)

>
> BTW, "LABEL is a symbol" sends the wrong message (a quoted symbol 
> evaluates to a symbol but it's not itself a symbol). IOW the docstring 
> should clarify that LABEL is an expression that's evaluated at runtime 
> (and should return a symbol). While I'm here, is it important that LABEL 
> evaluates to a symbol? Or is it like `catch/throw` where we expect most 
> uses to use a symbol but where any other (non-nil) value works as well?
>

The latter: it's like catch/throw, it's intended to use with a symbol but 
it could be any non-nil value.  So one could write something similar to 
what is found in the docstring of catch: "LABEL is evalled to get the 
label to use, it must not be nil".

Message #104 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 eliz <at> gnu.org, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 08 Jul 2023 17:42:19 -0400

>>> I'm sorry, I don't understand your question.  The only way (except by
>>> using internal functions) to enter a labeled restriction is to use the
>>> with-restriction special form with its optional label argument.
>> Nitpick: it's not a special form, it's a macro.  There's a *big*
>> difference because adding a special form requires changing
>> `macroexpand-all`, the compiler, yadayada, and it introduces backward
>> incompatibilities with packages doing their own code-walks.
> TIL.  I thought that "special form" and "macro" were more or less synonyms.
> The manual describes lambda, prog2, setq-default, dlet, letrec, named-let,
> with-suppressed-warnings, with-no-warnings, with-restriction and
> without-restriction as "special forms", although they are in fact macros.

You're right: from a programmer's stand point the distinction doesn't
really matter.  It matters only from the point of view of the language
implementer.  For some reason it tripped me, here (I went looking at the
code fearing that we were using an actual special form).

Sorry 'bout that, move along, nothing to see :-)

>> BTW, "LABEL is a symbol" sends the wrong message (a quoted symbol
>> evaluates to a symbol but it's not itself a symbol). IOW the docstring
>> should clarify that LABEL is an expression that's evaluated at runtime
>> (and should return a symbol). While I'm here, is it important that LABEL
>> evaluates to a symbol? Or is it like `catch/throw` where we expect most
>> uses to use a symbol but where any other (non-nil) value works as well?
>
> The latter: it's like catch/throw, it's intended to use with a symbol but it
> could be any non-nil value.  So one could write something similar to what is
> found in the docstring of catch: "LABEL is evalled to get the label to use,
> it must not be nil".

+1 from me,


        Stefan

Message #107 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 eliz <at> gnu.org, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 08 Jul 2023 22:21:20 +0000

>> I thought that "special form" and "macro" were more or less synonyms. 
>> The manual describes lambda, prog2, setq-default, dlet, letrec, 
>> named-let, with-suppressed-warnings, with-no-warnings, with-restriction 
>> and without-restriction as "special forms", although they are in fact 
>> macros.
>
> You're right: from a programmer's stand point the distinction doesn't 
> really matter.  It matters only from the point of view of the language 
> implementer.  For some reason it tripped me, here (I went looking at the 
> code fearing that we were using an actual special form).
>
> Sorry 'bout that, move along, nothing to see :-)
>

Well, there's at least something that could be fixed in the manuals.  I 
admit I had never read the "Special Forms" section, and if the manual had 
been consistent about the special form vs. macro distiction, perhaps I 
wouldn't have confused these two similar, but subtly different, notions.

>> The latter: it's like catch/throw, it's intended to use with a symbol 
>> but it could be any non-nil value.  So one could write something 
>> similar to what is found in the docstring of catch: "LABEL is evalled 
>> to get the label to use, it must not be nil".
>
> +1 from me,
>

Eli, I guess that minor documentation fix is okay for emacs-29?

diff --git a/doc/lispref/positions.texi b/doc/lispref/positions.texi
index e74a165b9ed..af5e648eb9d 100644
--- a/doc/lispref/positions.texi
+++ b/doc/lispref/positions.texi
@@ -1169,9 +1169,9 @@ Narrowing

 @cindex labeled narrowing
 @cindex labeled restriction
-When the optional argument @var{label}, a symbol, is present, the
-narrowing is @dfn{labeled}.  A labeled narrowing differs from a
-non-labeled one in several ways:
+When the optional argument @var{label}, which may be any Lisp object
+except @code{nil}, is present, the narrowing is @dfn{labeled}.  A
+labeled narrowing differs from a non-labeled one in several ways:

 @itemize @bullet
 @item
diff --git a/lisp/subr.el b/lisp/subr.el
index 0b397b7bebf..b73d0e5d989 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3964,11 +3964,11 @@ with-restriction

 The current restrictions, if any, are restored upon return.

-When the optional :label LABEL argument is present, in which
-LABEL is a symbol, inside BODY, `narrow-to-region' and `widen'
-can be used only within the START and END limits.  To gain access
-to other portions of the buffer, use `without-restriction' with the
-same LABEL argument.
+When the optional :label LABEL argument, which is evalled to get
+the label to use and must not be nil, is present, inside BODY,
+`narrow-to-region' and `widen' can be used only within the START
+and END limits.  To gain access to other portions of the buffer,
+use `without-restriction' with the same LABEL argument.

 \(fn START END [:label LABEL] BODY)"
   (declare (indent 2) (debug t))

Message #110 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 eliz <at> gnu.org, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sat, 08 Jul 2023 19:22:41 -0400

> Well, there's at least something that could be fixed in the manuals.
> I admit I had never read the "Special Forms" section, and if the manual had
> been consistent about the special form vs. macro distiction, perhaps
> I wouldn't have confused these two similar, but subtly different, notions.

At the same time, for the ELisp programmer, this distinction is just an
implementation detail (except for rare corner cases where the programmer
needs to look at the output of `macroexpand`).  What is a macro and what
is a special form has changed in the past and will likely change again
in the future (e.g. `defun`, `defmacro`, and `prog2` are now macros but
used to be special forms).


        Stefan

Message #113 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 09:03:17 +0300

> Date: Sat, 08 Jul 2023 22:21:20 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     eliz <at> gnu.org, bugs <at> gnus.org
> 
> 
> Eli, I guess that minor documentation fix is okay for emacs-29?

Yes, but:

  . please use TAG, not LABEL for the :label's argument, since that's
    what it is;
  . please say "evaluated", not "evalled"

I also question the wisdom of telling only in the doc string that the
tag is evaluated.  I could understand if you did that the other way
around, but if the doc string says that, the manual should also say
it.

Message #116 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 08:35:29 +0000

>> Eli, I guess that minor documentation fix is okay for emacs-29?
>
> Yes, but:
>
> . please use TAG, not LABEL for the :label's argument, since that's what 
> it is;
>
> . please say "evaluated", not "evalled"
>

Okay.

>
> I also question the wisdom of telling only in the doc string that the 
> tag is evaluated.  I could understand if you did that the other way 
> around, but if the doc string says that, the manual should also say it.
>

But the patch I sent changes both the manual and the docstring?  Am I 
missing something?

Message #119 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 11:52:39 +0300

> Date: Sun, 09 Jul 2023 08:35:29 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     monnier <at> iro.umontreal.ca, bugs <at> gnus.org
> 
> > I also question the wisdom of telling only in the doc string that the 
> > tag is evaluated.  I could understand if you did that the other way 
> > around, but if the doc string says that, the manual should also say it.
> >
> 
> But the patch I sent changes both the manual and the docstring?

It does, but only the latter says that LABEL is evaluated.

Message #122 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 eliz <at> gnu.org, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 16:03:52 +0000

>> Well, there's at least something that could be fixed in the manuals. I 
>> admit I had never read the "Special Forms" section, and if the manual 
>> had been consistent about the special form vs. macro distiction, 
>> perhaps I wouldn't have confused these two similar, but subtly 
>> different, notions.
>
> At the same time, for the ELisp programmer, this distinction is just an 
> implementation detail (except for rare corner cases where the programmer 
> needs to look at the output of `macroexpand`).  What is a macro and what 
> is a special form has changed in the past and will likely change again 
> in the future (e.g. `defun`, `defmacro`, and `prog2` are now macros but 
> used to be special forms).
>

Indeed, but... what would you suggest?  Leave the manual, in which that 
distinction is not always clear, as it is?  Fix the manual to make that 
distinction clearer?  Remove that distinction, which is indeed an 
implementation detail, from the manual (and perhaps mention that some 
macros are not defined in Elisp but in C, in which case they can also be 
called "special forms")?

Message #125 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 16:04:09 +0000

>>> I also question the wisdom of telling only in the doc string that the 
>>> tag is evaluated.  I could understand if you did that the other way 
>>> around, but if the doc string says that, the manual should also say 
>>> it.
>>
>> But the patch I sent changes both the manual and the docstring?
>
> It does, but only the latter says that LABEL is evaluated.
>

Oh, indeed, now I see what you mean.  Here's the updated patch.

On a second thought, I believe its better to not replace LABEL with TAG, 
because that would mean changing that word in many places, including 
places in which such a change would make the text less understandable, 
e.g. the docstring of narrow-to-region:

However, when restrictions have been set by `with-restriction' with a 
label, `narrow-to-region' can be used only within the limits of these 
restrictions.  If the START or END arguments are outside these limits, the 
corresponding limit set by `with-restriction' is used instead of the 
argument.  To gain access to other portions of the buffer, use 
`without-restriction' with the same label.

diff --git a/doc/lispref/positions.texi b/doc/lispref/positions.texi
index e74a165b9ed..183d0e39aee 100644
--- a/doc/lispref/positions.texi
+++ b/doc/lispref/positions.texi
@@ -1169,9 +1169,10 @@ Narrowing

 @cindex labeled narrowing
 @cindex labeled restriction
-When the optional argument @var{label}, a symbol, is present, the
-narrowing is @dfn{labeled}.  A labeled narrowing differs from a
-non-labeled one in several ways:
+When the optional argument @var{label}, which is evaluated to get the
+label to use and must not be @code{nil}, is present, the narrowing is
+@dfn{labeled}.  A labeled narrowing differs from a non-labeled one in
+several ways:

 @itemize @bullet
 @item
diff --git a/lisp/subr.el b/lisp/subr.el
index 0b397b7bebf..2f0144e0f11 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3964,11 +3964,11 @@ with-restriction

 The current restrictions, if any, are restored upon return.

-When the optional :label LABEL argument is present, in which
-LABEL is a symbol, inside BODY, `narrow-to-region' and `widen'
-can be used only within the START and END limits.  To gain access
-to other portions of the buffer, use `without-restriction' with the
-same LABEL argument.
+When the optional LABEL argument, which is evaluated to get the
+label to use and must not be nil, is present, inside BODY,
+`narrow-to-region' and `widen' can be used only within the START
+and END limits.  To gain access to other portions of the buffer,
+use `without-restriction' with the same LABEL argument.

 \(fn START END [:label LABEL] BODY)"
   (declare (indent 2) (debug t))
@@ -3990,9 +3990,8 @@ without-restriction

 The current restrictions, if any, are restored upon return.

-When the optional :label LABEL argument is present, the
-restrictions set by `with-restriction' with the same LABEL argument
-are lifted.
+When the optional LABEL argument is present, the restrictions set
+by `with-restriction' with the same LABEL argument are lifted.

 \(fn [:label LABEL] BODY)"
   (declare (indent 0) (debug t))

Message #128 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 19:39:46 +0300

> Date: Sun, 09 Jul 2023 16:04:09 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     monnier <at> iro.umontreal.ca, bugs <at> gnus.org
> 
> On a second thought, I believe its better to not replace LABEL with TAG, 
> because that would mean changing that word in many places, including 
> places in which such a change would make the text less understandable, 
> e.g. the docstring of narrow-to-region:

LABEL implies a string or a symbol, whereas :label can accept "any
object that is not nil".  The description of 'catch' uses TAG.

> --- a/doc/lispref/positions.texi
> +++ b/doc/lispref/positions.texi
> @@ -1169,9 +1169,10 @@ Narrowing
> 
>   @cindex labeled narrowing
>   @cindex labeled restriction
> -When the optional argument @var{label}, a symbol, is present, the
> -narrowing is @dfn{labeled}.  A labeled narrowing differs from a
> -non-labeled one in several ways:
> +When the optional argument @var{label}, which is evaluated to get the
> +label to use and must not be @code{nil},

What "must not be nil": the label or the result of its evaluation?

Message #131 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 18:03:02 +0000

>> On a second thought, I believe its better to not replace LABEL with 
>> TAG, because that would mean changing that word in many places, 
>> including places in which such a change would make the text less 
>> understandable, e.g. the docstring of narrow-to-region:
>
> LABEL implies a string or a symbol, whereas :label can accept "any 
> object that is not nil".  The description of 'catch' uses TAG.
>

I know, and I agree that in principle TAG would be better.  However all 
the code and documentation was written with the word "label", and although 
using a non-symbol is possible, LABEL is intended to be a symbol, not an 
arbitrary Lisp object.  So perhaps the best thing to do would be this:

diff --git a/lisp/subr.el b/lisp/subr.el
index 0b397b7bebf..c2110cb4bb2 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3972,7 +3972,8 @@ with-restriction

 \(fn START END [:label LABEL] BODY)"
   (declare (indent 2) (debug t))
-  (if (eq (car rest) :label)
+  (if (and (eq (car rest) :label)
+           (symbolp (cadr rest)))
       `(internal--with-restriction ,start ,end (lambda () ,@(cddr rest))
                                  ,(cadr rest))
     `(internal--with-restriction ,start ,end (lambda () ,@rest))))

>> --- a/doc/lispref/positions.texi
>> +++ b/doc/lispref/positions.texi
>> @@ -1169,9 +1169,10 @@ Narrowing
>>
>>   @cindex labeled narrowing
>>   @cindex labeled restriction
>> -When the optional argument @var{label}, a symbol, is present, the
>> -narrowing is @dfn{labeled}.  A labeled narrowing differs from a
>> -non-labeled one in several ways:
>> +When the optional argument @var{label}, which is evaluated to get the
>> +label to use and must not be @code{nil},
>
> What "must not be nil": the label or the result of its evaluation?
>

The result of the evaluation of the label argument.  I don't know how to 
make this clearer.

Message #134 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 mattias.engdegard <at> gmail.com, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 14:52:52 -0400

>  \(fn START END [:label LABEL] BODY)"
>    (declare (indent 2) (debug t))
> -  (if (eq (car rest) :label)
> +  (if (and (eq (car rest) :label)
> +           (symbolp (cadr rest)))
>        `(internal--with-restriction ,start ,end (lambda () ,@(cddr rest))
>                                   ,(cadr rest))
>      `(internal--with-restriction ,start ,end (lambda () ,@rest))))

Doesn't look right: (cadr rest) should be an *expression* that evaluates
to a symbol, so in general it won't itself be a symbol.

>>> -When the optional argument @var{label}, a symbol, is present, the
>>> -narrowing is @dfn{labeled}.  A labeled narrowing differs from a
>>> -non-labeled one in several ways:
>>> +When the optional argument @var{label}, which is evaluated to get the
>>> +label to use and must not be @code{nil},
>> What "must not be nil": the label or the result of its evaluation?
> The result of the evaluation of the label argument.  I don't know how to
> make this clearer.

Maybe:

    When the optional argument @var{label}, which should evaluate to
    a non-nil value,


-- Stefan

Message #137 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 eliz <at> gnu.org, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 14:57:49 -0400

> Indeed, but... what would you suggest?  Leave the manual, in which that
> distinction is not always clear, as it is?

Yup.


        Stefan

Message #140 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 22:03:41 +0300

> Date: Sun, 09 Jul 2023 18:03:02 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com, 
>     monnier <at> iro.umontreal.ca, bugs <at> gnus.org
> 
> >> +When the optional argument @var{label}, which is evaluated to get the
> >> +label to use and must not be @code{nil},
> >
> > What "must not be nil": the label or the result of its evaluation?
> >
> 
> The result of the evaluation of the label argument.  I don't know how to 
> make this clearer.

Instead of "must not be nil", say "must not yield nil".

Message #143 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: gregory <at> heytings.org
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 22:06:06 +0300

> Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
>  monnier <at> iro.umontreal.ca
> Date: Sun, 09 Jul 2023 22:03:41 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> Instead of "must not be nil", say "must not yield nil".

Or rather "must yield a non-nil value".

Message #146 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, eliz <at> gnu.org,
 mattias.engdegard <at> gmail.com, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 19:19:43 +0000

>>  \(fn START END [:label LABEL] BODY)"
>>    (declare (indent 2) (debug t))
>> -  (if (eq (car rest) :label)
>> +  (if (and (eq (car rest) :label)
>> +           (symbolp (cadr rest)))
>>        `(internal--with-restriction ,start ,end (lambda () ,@(cddr rest))
>>                                   ,(cadr rest))
>>      `(internal--with-restriction ,start ,end (lambda () ,@rest))))
>
> Doesn't look right: (cadr rest) should be an *expression* that evaluates 
> to a symbol, so in general it won't itself be a symbol.
>

Whoops, indeed.  (symbolp (eval (cadr rest))) would work, but somehow I 
think it's not TRT and there's a more idiomatic way to do that.

Message #149 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, mattias.engdegard <at> gmail.com,
 monnier <at> iro.umontreal.ca, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 19:29:42 +0000

>> Instead of "must not be nil", say "must not yield nil".
>
> Or rather "must yield a non-nil value".
>

Thanks, done (419b4d4491).

Message #152 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, eliz <at> gnu.org,
 mattias.engdegard <at> gmail.com, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 15:42:04 -0400

> Whoops, indeed.  (symbolp (eval (cadr rest))) would work,

No, it wouldn't work: we can't (in general) decide at macroexpansion
time whether the expression will return a symbol at runtime.

> but somehow I think it's not TRT and there's a more idiomatic way to
> do that.

Yes: don't do it, or do it via a runtime check rather than
a compile-time check.  But here I don't see any benefit to imposing that
LABEL evaluates to a symbol.


        Stefan

Message #155 received at 64391 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acohen <at> ust.hk, 64391 <at> debbugs.gnu.org, eliz <at> gnu.org,
 mattias.engdegard <at> gmail.com, bugs <at> gnus.org
Subject: Re: bug#64391: buffer narrowing slowdown regression in emacs 29
Date: Sun, 09 Jul 2023 19:44:08 +0000

>> but somehow I think it's not TRT and there's a more idiomatic way to do 
>> that.
>
> Yes: don't do it, or do it via a runtime check rather than a 
> compile-time check.  But here I don't see any benefit to imposing that 
> LABEL evaluates to a symbol.
>

Agreed.

I guess this bug can be closed for good now?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.