bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Kang Niu]

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

1. emacs -q
2. Load a elpa package, symbol-overlay, which has
symbol-overlay-post-command in post-command-hook
3. Open a file buffer, enable symbol-overlay-mode
4. Hold C-n, symbol-overlay-post-command will call thing-at-point
frequently and the cpu profiler-report is as follows

     Samples    %   Function
       29500  93% - symbol-overlay-post-command
       29497  93%  - if
       29480  93%   - string=
       29474  93%    - symbol-overlay-get-symbol
       29471  93%     - or
       29466  93%      - thing-at-point
       29447  93%       - save-restriction
       29129  92%          narrow-to-region
           309   0%        + let
             13   0%   + symbol-overlay-remove-temp
         1388   4% + command-execute
           269   0% + redisplay_internal (C function)
           216   0% + ...
           191   0% + timer-event-handler
             20   0% + internal-timer-start-idle
             15   0%   jit-lock--antiblink-post-command
             11   0% + undo-auto--add-boundary
               4   0% + clear-minibuffer-message

The cpu profiler-report of the same process with thingatpt.el before
commit 7db376e560448e61485ba054def8c82b21f33d6a is as follows:

    Samples    %   Function
        3859  42% + redisplay_internal (C function)
        2895  31% + command-execute
         892   9% + ...
         689   7% + timer-event-handler
         618   6% - symbol-overlay-post-command
         611   6%  - if
         576   6%   - string=
         563   6%    - symbol-overlay-get-symbol
         563   6%     - or
         557   6%      - thing-at-point
         557   6%       - let
         545   5%        - cond
         510   5%         - let
         493   5%          - bounds-of-thing-at-point
         493   5%           - if
         478   5%            - let
         471   5%             - condition-case
         471   5%              - progn
         451   4%               - save-excursion
         261   2%                + funcall
         182   2%                + let
             4   0%              get

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

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Eli Zaretskii]

> From: Kang Niu <isgniuk@gmail.com>
> Date: Sat, 18 Dec 2021 13:59:24 +0800
> 
> 1. emacs -q
> 2. Load a elpa package, symbol-overlay, which has  symbol-overlay-post-command in post-command-hook
> 3. Open a file buffer, enable symbol-overlay-mode
> 4. Hold C-n, symbol-overlay-post-command will call thing-at-point frequently and the cpu profiler-report is as
> follows
> 
>      Samples    %   Function
>        29500  93% - symbol-overlay-post-command
>        29497  93%  - if
>        29480  93%   - string=
>        29474  93%    - symbol-overlay-get-symbol
>        29471  93%     - or
>        29466  93%      - thing-at-point
>        29447  93%       - save-restriction
>        29129  92%          narrow-to-region
>            309   0%        + let
>              13   0%   + symbol-overlay-remove-temp
>          1388   4% + command-execute
>            269   0% + redisplay_internal (C function)
>            216   0% + ...
>            191   0% + timer-event-handler
>              20   0% + internal-timer-start-idle
>              15   0%   jit-lock--antiblink-post-command
>              11   0% + undo-auto--add-boundary
>                4   0% + clear-minibuffer-message
> 
> The cpu profiler-report of the same process with thingatpt.el before commit
> 7db376e560448e61485ba054def8c82b21f33d6a is as follows:
> 
>     Samples    %   Function       
>         3859  42% + redisplay_internal (C function)
>         2895  31% + command-execute
>          892   9% + ...
>          689   7% + timer-event-handler
>          618   6% - symbol-overlay-post-command
>          611   6%  - if
>          576   6%   - string=
>          563   6%    - symbol-overlay-get-symbol
>          563   6%     - or
>          557   6%      - thing-at-point
>          557   6%       - let
>          545   5%        - cond
>          510   5%         - let
>          493   5%          - bounds-of-thing-at-point
>          493   5%           - if
>          478   5%            - let
>          471   5%             - condition-case
>          471   5%              - progn
>          451   4%               - save-excursion
>          261   2%                + funcall
>          182   2%                + let
>              4   0%              get

Thanks.  The NEWS entry about the change you mention says the feature
of respecting fields in thing-at-point is only active when the buffer
uses fields.

The implementation uses

  (save-restriction
    (narrow-to-region (field-beginning) (field-end))

AFAICT, field-beginning returns BOB and field-end returns EOB, when
there are no fields in the buffer.  Looking at the code of
narrow-to-region, I see that it basically does very little when called
with BOB and EOB as its arguments.  So it is strange that
narrow-to-region takes such a large percent of CPU time in the
profile.  If you make narrow-to-region return immediately when called
with BOB and EOB, does the profile change in any way?

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Eli Zaretskii]

[Forwarding to the bug tracker.  Please use Reply All to keep the bug
tracker on the CC list.]

> From: Kang Niu <isgniuk@gmail.com>
> Date: Sun, 19 Dec 2021 21:07:24 +0800
> 
> Thanks for your reply. I'v added two lines of code at the beginning of narrow-to-region and made a test.
> 
> 2681   EMACS_INT s = fix_position (start), e = fix_position (end);
> 2682   if (BEGV == s && ZV == e)
> 2683     return Qnil;
> 
> The overhead was still there with little improvement.

That is very strange, since the code is almost a no-op.

Does field-beginning and field-end indeed return BOB and EOB in that
case?  Maybe field-beginning and field-end take this time?

> The overhead disappears only if I comment out (narrow-to-region (field-beginning) (field-end)) in
> thing-at-point.

Perhaps thing-at-point should acquire a new optional argument saying
"don't pay attention to fields"?

> It is unnoticed but with some modes that call thing-at-point in post-command-hook, it's unbearable.
> 
> OS:
> ArchLinux with gcc-11.1.0
> &
> Ubuntu16.04 with gcc-9
> 
> configuration:
> ./configuration 'CFLAGS=-O2 -pipe' --prefix=/usr --libdir=/usr/lib --sysconfdir=/etc --libexecdir=/usr/lib
> --localstatedir=/var --without-x --with-x-toolkit=no --without-xwidgets --without-toolkit\
> -scroll-bars --without-dbus --without-gsettings --without-gconf --without-xim --without-xpm --without-jpeg
> --without-tiff --without-gif --without-png --without-rsvg --without-imagem\
> agick --without-lcms2 --without-libsystemd --without-selinux --without-xaw3d --without-gpm --with-sound=no
> --without-xft --without-libotf --without-cairo --without-harfbuzz --withou\
> t-m17n-flt --with-modules --without-compress-install

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> The overhead disappears only if I comment out (narrow-to-region
>> (field-beginning) (field-end)) in
>> thing-at-point.

Are there fields in the buffer you're using this in?  I.e., does
(field-beginning) (field-end) return anything other than
point-min/point-max?

> Perhaps thing-at-point should acquire a new optional argument saying
> "don't pay attention to fields"?

If the buffer has fields, then I think thing-at-point should pay
attention to them -- it's for stuff like this Emacs has fields in the
first place.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Kang Niu]

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

There are no fields in the buffer. (field-beginning) (field-end) return the
same value as (point-min) (point-max).

Lars Ingebrigtsen <larsi@gnus.org> 于2021年12月19日周日 21:35写道：

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> The overhead disappears only if I comment out (narrow-to-region
> >> (field-beginning) (field-end)) in
> >> thing-at-point.
>
> Are there fields in the buffer you're using this in?  I.e., does
> (field-beginning) (field-end) return anything other than
> point-min/point-max?
>
> > Perhaps thing-at-point should acquire a new optional argument saying
> > "don't pay attention to fields"?
>
> If the buffer has fields, then I think thing-at-point should pay
> attention to them -- it's for stuff like this Emacs has fields in the
> first place.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

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

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Kang Niu <isgniuk@gmail.com> writes:

> There are no fields in the buffer. (field-beginning) (field-end) return the same value
> as (point-min) (point-max).

Odd.  Does the following fix the performance problem?  I'm wondering
whether it's the field functions that are slow or whether the
narrow-to-region is the thing that somehow triggers the slowness.

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 2d1bf2013e..e746d94080 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -178,7 +178,11 @@ thing-at-point
 See the file `thingatpt.el' for documentation on how to define
 a symbol as a valid THING."
   (save-restriction
-    (narrow-to-region (field-beginning) (field-end))
+    (let ((beg (field-beginning))
+          (end (field-end)))
+      (when (or (not (= beg (point-min)))
+                (not (= end (point-max))))
+        (narrow-to-region (field-beginning) (field-end))))
     (let ((text
            (cond
             ((cl-loop for (pthing . function) in thing-at-point-provider-alist


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Kang Niu]

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

Seems now the let clause undertake the overhead:
     Samples    %   Function


        7230  77% - symbol-overlay-post-command
        7227  77%  - if
        7183  77%   - string=
        7180  77%    - symbol-overlay-get-symbol
        7180  77%     - or
        7176  77%      - thing-at-point
        7163  77%       - save-restriction
        7138  76%        - let
         221   2%         - cond
         206   2%          + let
           2   0%            let*
           7   0%         + if
          40   0%   + symbol-overlay-remove-temp
        1234  13% - command-execute
        1215  13%  - call-interactively
        1205  12%   - funcall-interactively
         783   8%    + next-line
         415   4%    + previous-line
           7   0%    + execute-extended-command
           3   0%   + byte-code
         358   3% + ...
         255   2% + redisplay_internal (C function)
         156   1% + timer-event-handler
          25   0% + undo-auto--add-boundary
          18   0% + jit-lock--antiblink-post-command
          17   0% + internal-timer-start-idle
           8   0% + clear-minibuffer-message

Lars Ingebrigtsen <larsi@gnus.org> 于2021年12月19日周日 22:44写道：

> Kang Niu <isgniuk@gmail.com> writes:
>
> > There are no fields in the buffer. (field-beginning) (field-end) return
> the same value
> > as (point-min) (point-max).
>
> Odd.  Does the following fix the performance problem?  I'm wondering
> whether it's the field functions that are slow or whether the
> narrow-to-region is the thing that somehow triggers the slowness.
>
> diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
> index 2d1bf2013e..e746d94080 100644
> --- a/lisp/thingatpt.el
> +++ b/lisp/thingatpt.el
> @@ -178,7 +178,11 @@ thing-at-point
>  See the file `thingatpt.el' for documentation on how to define
>  a symbol as a valid THING."
>    (save-restriction
> -    (narrow-to-region (field-beginning) (field-end))
> +    (let ((beg (field-beginning))
> +          (end (field-end)))
> +      (when (or (not (= beg (point-min)))
> +                (not (= end (point-max))))
> +        (narrow-to-region (field-beginning) (field-end))))
>      (let ((text
>             (cond
>              ((cl-loop for (pthing . function) in
> thing-at-point-provider-alist
>
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

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

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Kang Niu]

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

Maybe the field functions are slow.
I replaced the field functions with point-min and point-max
(narrow-to-region (point-min) (point-max))
and the overhead disappeared.

Kang Niu <isgniuk@gmail.com> 于2021年12月19日周日 23:19写道：

> Seems now the let clause undertake the overhead:
>      Samples    %   Function
>
>
>         7230  77% - symbol-overlay-post-command
>         7227  77%  - if
>         7183  77%   - string=
>         7180  77%    - symbol-overlay-get-symbol
>         7180  77%     - or
>         7176  77%      - thing-at-point
>         7163  77%       - save-restriction
>         7138  76%        - let
>          221   2%         - cond
>          206   2%          + let
>            2   0%            let*
>            7   0%         + if
>           40   0%   + symbol-overlay-remove-temp
>         1234  13% - command-execute
>         1215  13%  - call-interactively
>         1205  12%   - funcall-interactively
>          783   8%    + next-line
>          415   4%    + previous-line
>            7   0%    + execute-extended-command
>            3   0%   + byte-code
>          358   3% + ...
>          255   2% + redisplay_internal (C function)
>          156   1% + timer-event-handler
>           25   0% + undo-auto--add-boundary
>           18   0% + jit-lock--antiblink-post-command
>           17   0% + internal-timer-start-idle
>            8   0% + clear-minibuffer-message
>
> Lars Ingebrigtsen <larsi@gnus.org> 于2021年12月19日周日 22:44写道：
>
>> Kang Niu <isgniuk@gmail.com> writes:
>>
>> > There are no fields in the buffer. (field-beginning) (field-end) return
>> the same value
>> > as (point-min) (point-max).
>>
>> Odd.  Does the following fix the performance problem?  I'm wondering
>> whether it's the field functions that are slow or whether the
>> narrow-to-region is the thing that somehow triggers the slowness.
>>
>> diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
>> index 2d1bf2013e..e746d94080 100644
>> --- a/lisp/thingatpt.el
>> +++ b/lisp/thingatpt.el
>> @@ -178,7 +178,11 @@ thing-at-point
>>  See the file `thingatpt.el' for documentation on how to define
>>  a symbol as a valid THING."
>>    (save-restriction
>> -    (narrow-to-region (field-beginning) (field-end))
>> +    (let ((beg (field-beginning))
>> +          (end (field-end)))
>> +      (when (or (not (= beg (point-min)))
>> +                (not (= end (point-max))))
>> +        (narrow-to-region (field-beginning) (field-end))))
>>      (let ((text
>>             (cond
>>              ((cl-loop for (pthing . function) in
>> thing-at-point-provider-alist
>>
>>
>> --
>> (domestic pets only, the antidote for overdose, milk.)
>>    bloggy blog: http://lars.ingebrigtsen.no
>>
>

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

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Kang Niu <isgniuk@gmail.com> writes:

> Maybe the field functions are slow.
> I replaced the field functions with point-min and point-max
> (narrow-to-region (point-min) (point-max))
> and the overhead disappeared.

Reading the field functions, it seems like they don't check for the
common case -- that is, being called outside a field, and do a whole
lot of work that turns out to be pointless.

Does the following (untested) patch fix the performance problems? 

diff --git a/src/editfns.c b/src/editfns.c
index 5c9c34dc35..3e716a4544 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -437,6 +437,17 @@ find_field (Lisp_Object pos, Lisp_Object merge_at_boundary,
 
   after_field
     = get_char_property_and_overlay (pos, Qfield, Qnil, NULL);
+
+  /* We're not in a field, so just return `point-min'/`point-max'.  */
+  if (NILP (after_field))
+    {
+      if (beg)
+	*beg = BEGV;
+      if (end)
+	*end = ZV;
+      return;
+    }
+
   before_field
     = (XFIXNAT (pos) > BEGV
        ? get_char_property_and_overlay (make_fixnum (XFIXNUM (pos) - 1),


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Reading the field functions, it seems like they don't check for the
> common case -- that is, being called outside a field, and do a whole
> lot of work that turns out to be pointless.

Never mind, that's not correct -- outside a field, the `field-end'
should return the position before the beginning of the next field, so I
that's wrong.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Never mind, that's not correct -- outside a field, the `field-end'
> should return the position before the beginning of the next field, so I
> that's wrong.

This one should be more correct, but it's just 3x faster than what's on
the trunk...

diff --git a/src/editfns.c b/src/editfns.c
index 5c9c34dc35..355a7a3e29 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -437,6 +437,27 @@ find_field (Lisp_Object pos, Lisp_Object merge_at_boundary,
 
   after_field
     = get_char_property_and_overlay (pos, Qfield, Qnil, NULL);
+
+  /* We're not in a field, so find the prev/next area with a field
+     property.  */
+  if (NILP (after_field))
+    {
+      if (beg)
+	{
+	  Lisp_Object p = Fprevious_single_char_property_change (pos, Qfield,
+								 Qnil,
+								 beg_limit);
+	  *beg = NILP (p) ? BEGV : XFIXNAT (p);
+	}
+      if (end)
+	{
+	  Lisp_Object p = Fnext_single_char_property_change (pos, Qfield, Qnil,
+							     end_limit);
+	  *end = NILP (p) ? ZV : XFIXNAT (p);
+	}
+      return;
+    }
+
   before_field
     = (XFIXNAT (pos) > BEGV
        ? get_char_property_and_overlay (make_fixnum (XFIXNUM (pos) - 1),


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Kang Niu <isgniuk@gmail.com> writes:

> With this patch, the overhead is still there but has some improvement.

Right.  I think I'll push that patch anyway -- it seems like a general
improvement, even if it doesn't really help this specific case much.

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Perhaps thing-at-point should acquire a new optional argument saying
> "don't pay attention to fields"?

We could indeed do that...  But perhaps we could just feed in some LIMIT
parameters to the field functions:

(field-beginning &optional POS ESCAPE-FROM-EDGE LIMIT)

The problem is that they will search to the beginning/end of the buffer
for the overlays/text properties when there's none under point, and that
will be slow.

So limiting this here might do the trick, but what to use as the limit?
Line end/beg positions?  Is there any THING that spans several lines?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So limiting this here might do the trick, but what to use as the limit?
> Line end/beg positions?  Is there any THING that spans several lines?

To answer my question: Yes.  For instance, `list' may span several lines.
So...  Eli's suggestion of adding a no-field parameter here may be
solution, unless we somehow rethink the thing-at-point algorithm.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> To answer my question: Yes.  For instance, `list' may span several lines.
> So...  Eli's suggestion of adding a no-field parameter here may be
> solution, unless we somehow rethink the thing-at-point algorithm.

Another idea: We could change thing-at-point to only respond to
"positive" fields.  That is, only consider a thing to be a field if
there's a `field' property under point.

(insert (propertize "foo" 'field 1) "bar" (propertize "zot" 'field 2))

This would mean that (symbol-at-point) in the "foo" bit would return
`foo', but in the "bar" part, it would return `foobarzot'.  (Before
adding field support, it would return the latter in the first case,
too.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 52593@debbugs.gnu.org,  Kang Niu <isgniuk@gmail.com>
> Date: Sun, 19 Dec 2021 18:33:37 +0100
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > To answer my question: Yes.  For instance, `list' may span several lines.
> > So...  Eli's suggestion of adding a no-field parameter here may be
> > solution, unless we somehow rethink the thing-at-point algorithm.
> 
> Another idea: We could change thing-at-point to only respond to
> "positive" fields.  That is, only consider a thing to be a field if
> there's a `field' property under point.
> 
> (insert (propertize "foo" 'field 1) "bar" (propertize "zot" 'field 2))
> 
> This would mean that (symbol-at-point) in the "foo" bit would return
> `foo', but in the "bar" part, it would return `foobarzot'.  (Before
> adding field support, it would return the latter in the first case,
> too.)

I admit that I lack the context here: why was thing-at-point changed
to honor fields? what was that supposed to achieve?

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I admit that I lack the context here: why was thing-at-point changed
> to honor fields? what was that supposed to achieve?

I think respecting fields in general is a good idea -- the more things
that do, the more useful the field concept becomes.  And thing-at-point
seems like an obvious thing that can be helped by fields -- the fields
clarify what the "thing" is.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 52593@debbugs.gnu.org,  isgniuk@gmail.com
> Date: Mon, 20 Dec 2021 10:23:36 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I admit that I lack the context here: why was thing-at-point changed
> > to honor fields? what was that supposed to achieve?
> 
> I think respecting fields in general is a good idea -- the more things
> that do, the more useful the field concept becomes.  And thing-at-point
> seems like an obvious thing that can be helped by fields -- the fields
> clarify what the "thing" is.

So basically, the "thing" should have the field property on its
characters?  And testing for that property at point, and refraining
from narrowing if there's no such property, still leaves the code too
slow?

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> So basically, the "thing" should have the field property on its
> characters?  And testing for that property at point, and refraining
> from narrowing if there's no such property, still leaves the code too
> slow?

No, if we do that, it should be plenty fast.  But it's slightly
different from what `(goto-char field-beginning)' (for instance) does --
if we're not in a field, it'll take us to the end of the previous field.
(I.e., it considers a section of text that has no `field' parameter to
be a field in itself.)

But I think we can just document that small quirk in `thing-at-point'
and still get the intended `field' action in most things...  perhaps.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Kang Niu]

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

Maybe an optional arg for thing-at-point is necessary to respect fields?
As I understand, it should be determined by the user of thing-at-point to
get what kind of thing at point.
If the user knows or expects fields in the text, he passes the optional arg.
If the user just wants the "normal" thing at the point whether there are
fields or not, he calls it without the opt arg as the old way.

Eli Zaretskii <eliz@gnu.org> 于2021年12月19日周日 21:26写道：

> [Forwarding to the bug tracker.  Please use Reply All to keep the bug
> tracker on the CC list.]
>
> > From: Kang Niu <isgniuk@gmail.com>
> > Date: Sun, 19 Dec 2021 21:07:24 +0800
> >
> > Thanks for your reply. I'v added two lines of code at the beginning of
> narrow-to-region and made a test.
> >
> > 2681   EMACS_INT s = fix_position (start), e = fix_position (end);
> > 2682   if (BEGV == s && ZV == e)
> > 2683     return Qnil;
> >
> > The overhead was still there with little improvement.
>
> That is very strange, since the code is almost a no-op.
>
> Does field-beginning and field-end indeed return BOB and EOB in that
> case?  Maybe field-beginning and field-end take this time?
>
> > The overhead disappears only if I comment out (narrow-to-region
> (field-beginning) (field-end)) in
> > thing-at-point.
>
> Perhaps thing-at-point should acquire a new optional argument saying
>> "don't pay attention to fields"?
>
>
> > It is unnoticed but with some modes that call thing-at-point in
> post-command-hook, it's unbearable.
> >
> > OS:
> > ArchLinux with gcc-11.1.0
> > &
> > Ubuntu16.04 with gcc-9
> >
> > configuration:
> > ./configuration 'CFLAGS=-O2 -pipe' --prefix=/usr --libdir=/usr/lib
> --sysconfdir=/etc --libexecdir=/usr/lib
> > --localstatedir=/var --without-x --with-x-toolkit=no --without-xwidgets
> --without-toolkit\
> > -scroll-bars --without-dbus --without-gsettings --without-gconf
> --without-xim --without-xpm --without-jpeg
> > --without-tiff --without-gif --without-png --without-rsvg
> --without-imagem\
> > agick --without-lcms2 --without-libsystemd --without-selinux
> --without-xaw3d --without-gpm --with-sound=no
> > --without-xft --without-libotf --without-cairo --without-harfbuzz
> --withou\
> > t-m17n-flt --with-modules --without-compress-install
>

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

bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Lars Ingebrigtsen]

Kang Niu <isgniuk@gmail.com> writes:

> Maybe an optional arg for thing-at-point is necessary to respect fields?
> As I understand, it should be determined by the user of thing-at-point to get what
> kind of thing at point.
> If the user knows or expects fields in the text, he passes the optional arg.
> If the user just wants the "normal" thing at the point whether there are fields or
> not, he calls it without the opt arg as the old way.

The problem is interoperability between packages.  If a package uses
fields to make the buffer more understandable, then other packages like
symbol-overlay should use those fields automatically without having to
be altered.  And if symbol-overlay says "do use fields", then we're back
to the same performance problems we're already seeing.

Ideally the way to make this work would be to change all the thingatp
functions to do their normal thing, but then see whether there's any
field separators in that area, and if so, recalculate the "thing".

But that would require a big rewrite.

I think at this point, the way forward is to revert this change on
emacs-28 to fix the performance regression, and then open a new bug
report for this.  So I'll do both now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52593: [External] : bug#52593: 28.0.90; (thing-at-point thing) has so much overhead since commit 7db376e560448e61485ba054def8c82b21f33d6a

[Drew Adams]

> > Maybe an optional arg for thing-at-point is necessary to respect fields?
> > As I understand, it should be determined by the user of thing-at-point to
> > get what kind of thing at point. If the user knows or expects fields in
> > the text, he passes the optional arg. If the user just wants the "normal"
> > thing at the point whether there are fields or not, he calls it without
> > the opt arg as the old way.

That's right - or almost so.  It's definitely right
that thing-at-point functions should not, on their
own and systematically, limit their check/search to
a field that might be present.  That would be awful.

But I think it should not just be an argument to
thing-at-point functions that controls/decides this.

Users, not just Lisp code, should be able to control
whether thing-at-point functions limit checking to a
field at point (when a field is present).

That means either a user option or a defvar that
users and code can bind, to get the behavior they
want.  But this question of how/where to tell
a thing-at-point function what to do about fields
is a secondary question.
___

I disagree strongly with Lars that from now on all
code that uses thing-at-point functions should
have its behavior changed to always respect a field
at point.  That would not only break existing code;
it would be inflexible for no reason.

IIUC, this is essentially about wanting to add a
NEW feature, which is that thing-at-point functions
can BE ABLE to return a thing (of the given kind),
while limiting the check/search to a field at point.

There's nothing wrong with adding that possibility,
as an optional feature.  It would be terribly wrong
to just impose such (backward incompatible) behavior
going forward.

Again: IIUC.

> If a package uses fields to make the buffer more
> understandable, then other packages like symbol-overlay
> should use those fields automatically without having to
> be altered.

Other packages "should"?  No.  Other packages should
perhaps _be able_ to.  Let's not be draconian, please.

Just because one package uses fields to "make the
buffer more understandable", it does not follow that
all (or even any) other packages want or need to
take that particular understandability into account
when checking for a particular kind of thing at
point.  That doesn't follow at all.

You have no way of knowing what some arbitrary other
package might want to do.  Supposing that all packages
must want to respect fields that are provided by some
package (for all of their behavior no less!) is, well,
misguided.

> Ideally the way to make this work would be to change all the thingatp
> functions to do their normal thing, but then see whether there's any
> field separators in that area, and if so, recalculate the "thing".

No, definitely not.  Thing-at-point functions can be
made _able_ to "recalculate the thing".  They should
not do so systematically or generally.

> I think at this point, the way forward is to revert this change on
> emacs-28 to fix the performance regression, and then open a new bug
> report for this.  So I'll do both now.

The bug is not just a performance one (IIUC).  The
bug is in the misguided design that limitation to a
field is what thing-at-point functions should always,
or even generally or by default, do.

Adding the ability to respect a field at point makes
sense.  The fact that a buffer has been given fields
should not, by itself, control what thing-at-point
functions do.  It can reasonably _inform_ what they do.