Redisplay: NS port, high CPU load

Redisplay: NS port, high CPU load

[David Reitter]

I am and users are experiencing regular high CPU load with the NS port, which started some time around late 2015, but according to users co-incided with an OS upgrade, too.  I tried rolling back changes, with limited success: it does seem though that very recently, the problem has become much worse.

Usually, Emacs goes into 100% CPU load, or sometimes high load.  What invariably shows up in my process samples is NSMenu _sidebandUpdate*  (no idea what that does), within ns_upgrade_menubar.  The attachment (sorry, could not export to text) shows an example.

I’m getting the spinning beachball sometimes when entering a minibuffer for a command.

I would like to fix this problem, but have been unsuccessful in tracing it down.  As you can see, we’re spending a lot of time in redisplay.
Is redisplay slow, or is it called again and again?

Advice would be appreciated.  (Yes, this is with Aquamacs, and I can’t tell if the problem exists in GNU Emacs as it probably interacts with some configuration.)



Date/Time:       2016-06-08 15:32:29.059 +1000
Launch Time:     2016-06-08 13:51:42.364 +1000
OS Version:      Mac OS X 10.11.4 (15E65)
Report Version:  7
Analysis Tool:   /usr/bin/sample
----

Call graph:
    2484 Thread_453834: Main Thread   DispatchQueue_<multiple>
    + 2417 start  (in libdyld.dylib) + 1  [0x7fff892d35ad]
    + ! 2417 main  () + 5602  [0x1000c3602]  emacs.c:1606
    + !   2417 Frecursive_edit  () + 218  [0x1000c48ea]  keyboard.c:755
    + !     2417 recursive_edit_1  () + 101  [0x1000c46c5]  keyboard.c:684
    + !       2417 command_loop  () + 158  [0x1000c47ae]  keyboard.c:1078
    + !         2417 internal_catch  () + 54  [0x10013f6b6]  eval.c:1074
    + !           2417 command_loop_2  () + 48  [0x1000d45c0]  keyboard.c:1099
    + !             2417 internal_condition_case  () + 70  [0x10013fb46]  eval.c:1309
    + !               2417 command_loop_1  () + 1154  [0x1000c5552]  keyboard.c:1357
    + !                 2417 read_key_sequence  () + 1959  [0x1000c6e17]  keyboard.c:9101
    + !                   2417 read_char  () + 5783  [0x1000ca407]  keyboard.c:2706
    + !                     2417 sit_for  () + 261  [0x100008515]  dispnew.c:5762
    + !                       1694 wait_reading_process_output  () + 1218  [0x100183482]  process.c:4609
    + !                       : 1694 redisplay_preserve_echo_area  () + 44  [0x100028dfc]  xdisp.c:14286
    + !                       :   970 redisplay_internal  () + 2034  [0x100027282]  xdisp.c:11850
    + !                       :   | 962 update_menu_bar  () + 450  [0x10004c9b2]  xdisp.c:11960
    + !                       :   | + 869 ns_update_menubar  () + 479  [0x1001cba6f]  nsmenu.m:139
    + !                       :   | + ! 868 -[NSApplication setMainMenu:]  (in AppKit) + 1584  [0x7fff8e8dc2c0]
    + !                       :   | + ! : 868 -[NSMenu _addSidebandMenuUpdaterForRoles:token:priority:handler:]  (in AppKit) + 229  [0x7fff8e8dc912]
    + !                       :   | + ! :   868 -[NSMenu _sidebandUpdaterRoles]  (in AppKit) + 41,47,...  [0x7fff8e8dca4d,0x7fff8e8dca53,...]
    + !                       :   | + ! 1 -[NSApplication setMainMenu:]  (in AppKit) + 655  [0x7fff8e8dbf1f]
    + !                       :   | + !   1 objc_msgSend  (in libobjc.A.dylib) + 26  [0x7fff8dbc54da]
    + !                       :   | + 57 ns_update_menubar  () + 1577  [0x1001cbeb9]  nsmenu.m:454

Re: Redisplay: NS port, high CPU load

[Anders Lindgren]

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

Hi!

Unfortunately, I have no direct idea of what could be wrong, but you can
try the following:

Enable NSTRACE (this is done in nsterm.h) -- this might give you a bit more
information about what happens on the NS port side. You have to launch
Emacs from a terminal window, though.

Adding a pair of block_input() and unblock_input() has fixed similar
problems, it might be worth a try here as well.

If Emacs calls setMainMeny a lot, it might be an idea to try to reduce
this, e.g. by only calling it when the meny really has been changed (unless
this mechanism already is in place).

    -- Anders

On Wed, Jun 8, 2016 at 8:04 AM, David Reitter <david.reitter@gmail.com>
wrote:

> I am and users are experiencing regular high CPU load with the NS port,
> which started some time around late 2015, but according to users co-incided
> with an OS upgrade, too.  I tried rolling back changes, with limited
> success: it does seem though that very recently, the problem has become
> much worse.
>
> Usually, Emacs goes into 100% CPU load, or sometimes high load.  What
> invariably shows up in my process samples is NSMenu _sidebandUpdate*  (no
> idea what that does), within ns_upgrade_menubar.  The attachment (sorry,
> could not export to text) shows an example.
>
> I’m getting the spinning beachball sometimes when entering a minibuffer
> for a command.
>
> I would like to fix this problem, but have been unsuccessful in tracing it
> down.  As you can see, we’re spending a lot of time in redisplay.
> Is redisplay slow, or is it called again and again?
>
> Advice would be appreciated.  (Yes, this is with Aquamacs, and I can’t
> tell if the problem exists in GNU Emacs as it probably interacts with some
> configuration.)
>
>
>
> Date/Time:       2016-06-08 15:32:29.059 +1000
> Launch Time:     2016-06-08 13:51:42.364 +1000
> OS Version:      Mac OS X 10.11.4 (15E65)
> Report Version:  7
> Analysis Tool:   /usr/bin/sample
> ----
>
> Call graph:
>     2484 Thread_453834: Main Thread   DispatchQueue_<multiple>
>     + 2417 start  (in libdyld.dylib) + 1  [0x7fff892d35ad]
>     + ! 2417 main  () + 5602  [0x1000c3602]  emacs.c:1606
>     + !   2417 Frecursive_edit  () + 218  [0x1000c48ea]  keyboard.c:755
>     + !     2417 recursive_edit_1  () + 101  [0x1000c46c5]  keyboard.c:684
>     + !       2417 command_loop  () + 158  [0x1000c47ae]  keyboard.c:1078
>     + !         2417 internal_catch  () + 54  [0x10013f6b6]  eval.c:1074
>     + !           2417 command_loop_2  () + 48  [0x1000d45c0]
> keyboard.c:1099
>     + !             2417 internal_condition_case  () + 70  [0x10013fb46]
> eval.c:1309
>     + !               2417 command_loop_1  () + 1154  [0x1000c5552]
> keyboard.c:1357
>     + !                 2417 read_key_sequence  () + 1959  [0x1000c6e17]
> keyboard.c:9101
>     + !                   2417 read_char  () + 5783  [0x1000ca407]
> keyboard.c:2706
>     + !                     2417 sit_for  () + 261  [0x100008515]
> dispnew.c:5762
>     + !                       1694 wait_reading_process_output  () + 1218
> [0x100183482]  process.c:4609
>     + !                       : 1694 redisplay_preserve_echo_area  () +
> 44  [0x100028dfc]  xdisp.c:14286
>     + !                       :   970 redisplay_internal  () + 2034
> [0x100027282]  xdisp.c:11850
>     + !                       :   | 962 update_menu_bar  () + 450
> [0x10004c9b2]  xdisp.c:11960
>     + !                       :   | + 869 ns_update_menubar  () + 479
> [0x1001cba6f]  nsmenu.m:139
>     + !                       :   | + ! 868 -[NSApplication setMainMenu:]
> (in AppKit) + 1584  [0x7fff8e8dc2c0]
>     + !                       :   | + ! : 868 -[NSMenu
> _addSidebandMenuUpdaterForRoles:token:priority:handler:]  (in AppKit) +
> 229  [0x7fff8e8dc912]
>     + !                       :   | + ! :   868 -[NSMenu
> _sidebandUpdaterRoles]  (in AppKit) + 41,47,...
> [0x7fff8e8dca4d,0x7fff8e8dca53,...]
>     + !                       :   | + ! 1 -[NSApplication setMainMenu:]
> (in AppKit) + 655  [0x7fff8e8dbf1f]
>     + !                       :   | + !   1 objc_msgSend  (in
> libobjc.A.dylib) + 26  [0x7fff8dbc54da]
>     + !                       :   | + 57 ns_update_menubar  () + 1577
> [0x1001cbeb9]  nsmenu.m:454
>

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

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 8, 2016, at 5:50 PM, Anders Lindgren <andlind@gmail.com> wrote:

> Enable NSTRACE (this is done in nsterm.h) -- this might give you a bit more information about what happens on the NS port side. You have to launch Emacs from a terminal window, though.

It looks like some tool bar items are triggering this.  Below is what it cycles through.  Hiding the toolbar, switching the buffer, or turning off the major mode makes it go away.  Local variable, non-permanent?

nsmenu.m  : 1134: [396624]  update_frame_tool_bar
nsmenu.m  : 1458: [396625]  | [EmacsToolbar clearActive]
nsmenu.m  : 1551: [396626]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396627]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396628]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396629]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396630]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396631]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396632]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396633]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396634]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396635]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396636]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396637]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396638]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396639]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1551: [396640]  | [EmacsToolbar addDisplayItemWithImage: ...]
nsmenu.m  : 1509: [396641]  | [EmacsToolbar changed]
nsfns.m   :  529: [396642]  x_implicitly_set_name
nsfns.m   :  579: [396643]  | ns_set_name_as_filename
nsterm.m  : 4715: [396644]  ns_redeem_scroll_bar
nsterm.m  : 9207: [396645]  | [EmacsScroller reprieve]
nsterm.m  : 4530: [396646]  ns_set_vertical_scroll_bar
nsterm.m  : 2654: [396647]  | ns_clear_frame_area
nsterm.m  : 4715: [396648]  ns_redeem_scroll_bar
nsterm.m  : 4743: [396649]  ns_judge_scroll_bars
nsterm.m  : 9215: [396650]  | [EmacsScroller judge]
nsterm.m  : 1076: [396651]  ns_update_begin
nsterm.m  : 1007: [396652]  | ns_update_auto_hide_menu_bar
nsterm.m  : 8225: [396653]  | [EmacsView isFullscreen] ->> 0
nsterm.m  : 1156: [396654]  ns_update_window_begin
nsterm.m  : 1187: [396655]  ns_update_window_end
nsterm.m  : 3004: [396656]  | ns_draw_window_cursor
nsterm.m  : 1156: [396657]  ns_update_window_begin
nsterm.m  : 1187: [396658]  ns_update_window_end
nsterm.m  : 3004: [396659]  | ns_draw_window_cursor
nsterm.m  : 1237: [396660]  ns_update_end
nsterm.m  : 1222: [396661]  | ns_flush
nsterm.m  : 2499: [396662]  ns_frame_up_to_date
nsterm.m  : 1076: [396663]  | ns_update_begin
nsterm.m  : 1007: [396664]  | | ns_update_auto_hide_menu_bar
nsterm.m  : 8225: [396665]  | | [EmacsView isFullscreen] ->> 0
nsterm.m  : 1237: [396666]  | ns_update_end
nsterm.m  : 1222: [396667]  | | ns_flush
nsfns.m   :  529: [396668]  x_implicitly_set_name
nsfns.m   :  579: [396669]  | ns_set_name_as_filename
nsmenu.m  :  122: [396670]  ns_update_menubar
nsterm.m  : 4696: [396671]  ns_condemn_scroll_bars
nsterm.m  : 9199: [396672]  | [EmacsScroller condemn]
nsmenu.m  : 1134: [396673]  update_frame_tool_bar
nsmenu.m  : 1458: [396674]  | [EmacsToolbar clearActive]

Re: Redisplay: NS port, high CPU load

[Alan Third]

On Wed, Jun 08, 2016 at 08:44:49PM +1000, David Reitter wrote:
> It looks like some tool bar items are triggering this. Below is what
> it cycles through. Hiding the toolbar, switching the buffer, or
> turning off the major mode makes it go away. Local variable,
> non-permanent?

Am I right in thinking that the only common factor is that the toolbar
must be on? Otherwise it just happens randomly?

Does the Aquamacs toolbar use different images or something, or is it
identical to the GNU Emacs one?
-- 
Alan Third

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 9, 2016, at 5:55 AM, Alan Third <alan@idiocy.org> wrote:

> Does the Aquamacs toolbar use different images or something, or is it
> identical to the GNU Emacs one?

No it uses different images and provides a different set of commands.

The offending item seems to be revert-buffer.  Indeed, if in disabled state, updates go on and on.  As soon as you modify the buffer, it’s enabled, and updates stop.  Here’s the entry from tool-bar-map:

(revert-buffer menu-item "Revert Buffer" revert-buffer :enable
		(or
		 (not
		  (eq revert-buffer-function 'revert-buffer--default))
		 (not
		  (eq revert-buffer-insert-file-contents-function 'revert-buffer-insert-file-contents--default-function))
		 (and buffer-file-number
		      (or
		       (and buffer-file-name
			    (file-remote-p buffer-file-name))
		       (buffer-modified-p)
		       (not
			(verify-visited-file-modtime
			 (current-buffer))))))
		:help "Re-read current buffer from its file" :image
		(image :type png :file "/Users/dr/emacs/nextstep/Aquamacs.app/Contents/Resources/etc/images/update.tiff" :background "grey" :mask heuristic)
		:label "Revert")

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 9, 2016, at 6:12 AM, David Reitter <david.reitter@gmail.com> wrote:

> The offending item seems to be revert-buffer.  Indeed, if in disabled state, updates go on and on.  As soon as you modify the buffer, it’s enabled, and updates stop.  Here’s the entry from tool-bar-map:

Further to that, it appears that (verify-visited-file-modtime (current-buffer))  is what causes the redisplay of the toolbar (and more).

I can manually cause a call to update_frame_tool_bar by evaluating this expression.  I don’t have time to look further right now, but the call to Ffind_file_name_handler is a suspect.  See also below.


file-name-handler-alist is a variable defined in ‘fileio.c’.
Its value is (("\\.gpg\\(~\\|\\.~[0-9]+~\\)?\\'" . epa-file-handler)
 ("\\(?:\\.dz\\|\\.txz\\|\\.xz\\|\\.lzma\\|\\.lz\\|\\.g?z\\|\\.\\(?:tgz\\|svgz\\|sifz\\)\\|\\.tbz2?\\|\\.bz2\\|\\.Z\\)\\(?:~\\|\\.~[-[:alnum:]:#@^._]+\\(?:~[[:digit:]]+\\)?~\\)?\\'" . jka-compr-handler)
 ("\\`/[^/]*\\'" . tramp-completion-file-name-handler)
 ("\\`/[^/|:][^/|]*:" . tramp-autoload-file-name-handler)
 ("\\`/:" . file-name-non-special))

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 9, 2016, at 11:03 AM, David Reitter <david.reitter@gmail.com> wrote:

> I can manually cause a call to update_frame_tool_bar by evaluating this expression.  I don’t have time to look further right now, but the call to Ffind_file_name_handler is a suspect.  See also below.

So far, I have traced it to the use of ENCODE_FILE  (encode_file_name) in verify-visited-file-modtime:

 filename = ENCODE_FILE (BVAR (b, filename));

Now, file-name-coding-system is utf-8-hfs, and that setting is associated with the error.  Setting it to ‘utf-8 makes the bug go away.

What can trigger a redisplay or menu/toolbar update in encode_file_name() for utf-8-hfs?

Re: Redisplay: NS port, high CPU load

[Anders Lindgren]

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

Hi!

The encoding utf-8-hfs is defined in lisp, in
international/ucs-normalize.el. However, by glancing at the code, I can't
see anything obvious that would trigger a redisplay (the code is relatively
complex, though).

One thing that is unique about the utf-8-hfs encoding is that is sets the
`decomposed-characters' coding system property. (See my and Eli:s commits
around 2015-12-23). This ensures that editing and completion of composed
characters like "åäö" work properly. However, looking at the code in
src/dired.c doesn't reveal anything obvious either. If you remove the code
that sets this property (in ucs-normalize.el) you can check if this causes
the redisplays.

    -- Anders

On Thu, Jun 9, 2016 at 10:22 AM, David Reitter <david.reitter@gmail.com>
wrote:

> On Jun 9, 2016, at 11:03 AM, David Reitter <david.reitter@gmail.com>
> wrote:
>
> > I can manually cause a call to update_frame_tool_bar by evaluating this
> expression.  I don’t have time to look further right now, but the call to
> Ffind_file_name_handler is a suspect.  See also below.
>
> So far, I have traced it to the use of ENCODE_FILE  (encode_file_name) in
> verify-visited-file-modtime:
>
>  filename = ENCODE_FILE (BVAR (b, filename));
>
> Now, file-name-coding-system is utf-8-hfs, and that setting is associated
> with the error.  Setting it to ‘utf-8 makes the bug go away.
>
> What can trigger a redisplay or menu/toolbar update in encode_file_name()
> for utf-8-hfs?
>
>
>

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

Re: Redisplay: NS port, high CPU load

[David Reitter]

Thanks, that’s informative.  I too was looking at ucs-normalize, but couldn’t find anything obvious.

On Jun 9, 2016, at 7:25 PM, Anders Lindgren <andlind@gmail.com> wrote:

> If you remove the code that sets this property (in ucs-normalize.el) you can check if this causes the redisplays.

No dice.  Removing or undoing this...

(coding-system-put 'utf-8-hfs 'decomposed-characters ’t)

… has no effect.

I also tried reverting the two changes from Dec 25 (with some conflicts, though) - this didn’t seem to change anything.

In any case, we need to understand what triggers the expensive toolbar update.

Re: Redisplay: NS port, high CPU load

[Anders Lindgren]

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

>
> In any case, we need to understand what triggers the expensive toolbar
> update.


Try to clear out one of :post-read-conversion and :pre-write-conversion to
see which of the two is the culprit. (While you're at it, try to clear out
both first just to make sure we're barking up the right tree.)

I just saw one suspicious thing:
`ucs-normalize-hfs-nfd-pre-write-conversion' change current buffer but
doesn't restore it, whereas the post-read function use save-excursion.

    -- Anders

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

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 10, 2016, at 12:11 AM, Anders Lindgren <andlind@gmail.com> wrote:

> Try to clear out one of :post-read-conversion and :pre-write-conversion to see which of the two is the culprit. (While you're at it, try to clear out both first just to make sure we're barking up the right tree.)

Bingo.  I had a look at the function and it’s the `insert’ into the temporary buffer that is causing this.

;; Pre-write conversion for `utf-8-hfs'.
(defun ucs-normalize-hfs-nfd-pre-write-conversion (from to)
  (let ((old-buf (current-buffer)))
    (set-buffer (generate-new-buffer " *temp*"))
    (if (stringp from)
        (insert from)
      (insert-buffer-substring old-buf from to))
    (ucs-normalize-HFS-NFD-region (point-min) (point-max))
    nil))


old-buf here is *code-conversion-work*.

I don’t understand this pre-write-conversion function.  According to the documentation of   `define-coding-system’, why would it make a new buffer and switch to it?

> VALUE must be a function to call after all functions in
> ‘write-region-annotate-functions’ and ‘buffer-file-format’ are
> called, and before the text is encoded by the coding system
> itself.  This function should convert the whole text in the
> current buffer.  For backward compatibility, this function is
> passed two arguments which can be ignored.

Re: Redisplay: NS port, high CPU load

[Anders Lindgren]

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

>
> > Try to clear out one of :post-read-conversion and :pre-write-conversion
> to see which of the two is the culprit. (While you're at it, try to clear
> out both first just to make sure we're barking up the right tree.)
>
> Bingo.  I had a look at the function and it’s the `insert’ into the
> temporary buffer that is causing this.
>

Great news!


> ;; Pre-write conversion for `utf-8-hfs'.
> (defun ucs-normalize-hfs-nfd-pre-write-conversion (from to)
>   (let ((old-buf (current-buffer)))
>     (set-buffer (generate-new-buffer " *temp*"))
>     (if (stringp from)
>         (insert from)
>       (insert-buffer-substring old-buf from to))
>     (ucs-normalize-HFS-NFD-region (point-min) (point-max))
>     nil))
>
>
> old-buf here is *code-conversion-work*.
>
> I don’t understand this pre-write-conversion function.  According to the
> documentation of   `define-coding-system’, why would it make a new buffer
> and switch to it?
>
> > VALUE must be a function to call after all functions in
> > ‘write-region-annotate-functions’ and ‘buffer-file-format’ are
> > called, and before the text is encoded by the coding system
> > itself.  This function should convert the whole text in the
> > current buffer.  For backward compatibility, this function is
> > passed two arguments which can be ignored.
>
>
It looks like this function needs to be modernized. (In my december rewrite
I simply started using it, assuming that it was fully functional.) Given
the documentation, it *should* be enough to implement it like this (with a
big reservation -- this is all new to me):

(defun ucs-normalize-hfs-nfd-pre-write-conversion (from to)
  (ucs-normalize-HFS-NFD-region (point-min) (point-max))
  nil)

If I remember correctly, the rewrite tried to correct two problems:

* When deleting a character like "ä" (which in the HFS file system is
decomposed into "a" and "¨") the full character should be deleted. Earlier,
when pressing backspace, "ä" was converted to an "a".

* Completion: If a directory contains files like "åäö.txt" and "aao.txt",
Emacs used to say that "a" was the common start to both sequences.

See the discussion in bug 22169 for more information.

    -- Anders

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

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 10, 2016, at 4:52 AM, Anders Lindgren <andlind@gmail.com> wrote:

> (defun ucs-normalize-hfs-nfd-pre-write-conversion (from to)
>   (ucs-normalize-HFS-NFD-region (point-min) (point-max))
>   nil)

Yes, that removes the high CPU load, at least for the reproducible case at hand.

> If I remember correctly, the rewrite tried to correct two problems:
> 
> * When deleting a character like "ä" (which in the HFS file system is decomposed into "a" and "¨") the full character should be deleted. Earlier, when pressing backspace, "ä" was converted to an "a”.

I don’t know how to test that, as I don’t know what deletion of a character means in the context of a filename.

> * Completion: If a directory contains files like "åäö.txt" and "aao.txt", Emacs used to say that "a" was the common start to both sequences.

I verified that your new version works correctly with respect to file name completion.

It looks like this particular issue (30% CPU load) may be addressed.  I don’t know yet whether the 100% CPU load issue is the same, as I can't reproduce it.

Do you want to check in the function above?

Re: Redisplay: NS port, high CPU load

[Anders Lindgren]

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

Hi!


> > * When deleting a character like "ä" (which in the HFS file system is
> decomposed into "a" and "¨") the full character should be deleted. Earlier,
> when pressing backspace, "ä" was converted to an "a”.
>
> I don’t know how to test that, as I don’t know what deletion of a
> character means in the context of a filename.
>

If you have a file named "åäö.txt" and you have auto-completed it, deleting
a character simply means pressing backspace. Earlier, when deleting, say,
the "ä", it was converted to an "a", so you had to press backspace twice to
delete a single character.



> It looks like this particular issue (30% CPU load) may be addressed.  I
> don’t know yet whether the 100% CPU load issue is the same, as I can't
> reproduce it.
>
> Do you want to check in the function above?
>
>
I will be away for the weekend, so if you have write access, feel free to
commit it yourself. Otherwise, I can do it at the beginning of next week.
Personally, I think this should go into the emacs-25 branch but we will
have to get that approved from John or Eli.

    -- Anders

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

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 10, 2016, at 4:02 PM, Anders Lindgren <andlind@gmail.com> wrote:

> I will be away for the weekend, so if you have write access, feel free to commit it yourself. Otherwise, I can do it at the beginning of next week. Personally, I think this should go into the emacs-25 branch but we will have to get that approved from John or Eli.

Good, I have checked in the change.

Eli, this addresses a high-CPU-load condition on OS X when a revert-buffer icon is present in the toolbar and the current buffer was unmodified.  This may or may not be the somewhat difficult-to-trace 100% CPU load bug a lot of us have been seeing on OSX.   The functionality of the utf-8-hfs coding system for the filesystem is preserved is tests.

I would suggest to merge this into emacs-25.

Also, I would suggest to check whether code like this should indeed trigger an update_frame_tool_bar.

  (with-temp-buffer
   (insert "XXX"))

Re: Redisplay: NS port, high CPU load

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Fri, 10 Jun 2016 18:16:39 +1000
> Cc: Alan Third <alan@idiocy.org>,
>  Emacs-Devel devel <emacs-devel@gnu.org>
> 
> On Jun 10, 2016, at 4:02 PM, Anders Lindgren <andlind@gmail.com> wrote:
> 
> > I will be away for the weekend, so if you have write access, feel free to commit it yourself. Otherwise, I can do it at the beginning of next week. Personally, I think this should go into the emacs-25 branch but we will have to get that approved from John or Eli.
> 
> Good, I have checked in the change.
> 
> Eli, this addresses a high-CPU-load condition on OS X when a revert-buffer icon is present in the toolbar and the current buffer was unmodified.  This may or may not be the somewhat difficult-to-trace 100% CPU load bug a lot of us have been seeing on OSX.   The functionality of the utf-8-hfs coding system for the filesystem is preserved is tests.
> 
> I would suggest to merge this into emacs-25.

If you want it on the release branch, why did you push it to master?
This just makes things more complicated than they should be.

> Also, I would suggest to check whether code like this should indeed trigger an update_frame_tool_bar.
> 
>   (with-temp-buffer
>    (insert "XXX"))

I don't think so.  It doesn't here.  How did you check that it does on
your system?

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 10, 2016, at 7:34 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> If you want it on the release branch, why did you push it to master?
> This just makes things more complicated than they should be.

I guess because I’m stupid.  I would have cherry-picked it in the release branch then.


> 
>> Also, I would suggest to check whether code like this should indeed trigger an update_frame_tool_bar.
>> 
>>  (with-temp-buffer
>>   (insert "XXX"))
> 
> I don't think so.  It doesn't here.  How did you check that it does on
> your system?

Enable NSTRACE in nsterm.h, specifically for updates.

Run an observe output.  Observe that the above causes an update to the toolbar, unlike evaluating other expressions.

Re: Redisplay: NS port, high CPU load

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Fri, 10 Jun 2016 19:46:01 +1000
> Cc: andlind@gmail.com,
>  alan@idiocy.org,
>  emacs-devel@gnu.org
> 
> >> Also, I would suggest to check whether code like this should indeed trigger an update_frame_tool_bar.
> >> 
> >>  (with-temp-buffer
> >>   (insert "XXX"))
> > 
> > I don't think so.  It doesn't here.  How did you check that it does on
> > your system?
> 
> Enable NSTRACE in nsterm.h, specifically for updates.
> 
> Run an observe output.  Observe that the above causes an update to the toolbar, unlike evaluating other expressions.

If by "evaluating expression" you mean something like C-j or M-:, then
they might trigger redisplay regardless of the expression they
evaluate.

To be sure this triggers redisplay of the tool bar, you need:

  . emacs -Q
  . disable blink-cursor-mode
  . disable global-eldoc-mode
  . define an interactive function that does the above insertion, and
    then bind that function to a key (I used F8)
  . press F8 and see if update_frame_tool_bar is called

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 10, 2016, at 8:22 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> To be sure this triggers redisplay of the tool bar, you need:
> 
>  . emacs -Q
>  . disable blink-cursor-mode
>  . disable global-eldoc-mode
>  . define an interactive function that does the above insertion, and
>    then bind that function to a key (I used F8)
>  . press F8 and see if update_frame_tool_bar is called


Well, turns out this happens with -q, but not with -Q.

If I clear tool-bar-map, the update goes away, but running populating it with a standard toolbar as per (tool-bar-setup), the updates come back.
So, I’m not sure what exactly makes this happen - I’m obviously loading a whole lot of settings with -q.

I’m assuming I’ll have to give you something more reproducible then, so please stand by.

Re: Redisplay: NS port, high CPU load

[Anders Lindgren]

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

Hi!

Unfortunately, I can't reproduce this either, but I have a theory on what
is going on.

`with-temp-buffer' calls `generate-new-buffer'. This, in turn calls
`get-buffer-create', which run `buffer-list-update-hook'. In theory, if a
function on this hook would trigger a tool-bar redisplay,
`(with-temp-buffer (insert "XXX"))' will too.

If this is the cause of the problem David is seeing, one solution would be
to temporarily bind `buffer-list-update-hook' to nil in `with-temp-buffer'.
(This might be a good idea anyway, since we don't want `with-temp-buffer'
to be slowed down by slow hooks, unless is it more important to ensure that
the buffer list is up to date?)

When it comes to the HFS coding rewrite, I still think it is valid. The
documentation of `define-coding-system' indicated that it expects the
pre-write function to convert the current buffer and that the two
parameters are only there for backward compatibility. The function, before
the rewrite, was clearly written for an older version of
`define-coding-system'.

    -- Anders


On Fri, Jun 10, 2016 at 12:36 PM, David Reitter <david.reitter@gmail.com>
wrote:

> On Jun 10, 2016, at 8:22 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > To be sure this triggers redisplay of the tool bar, you need:
> >
> >  . emacs -Q
> >  . disable blink-cursor-mode
> >  . disable global-eldoc-mode
> >  . define an interactive function that does the above insertion, and
> >    then bind that function to a key (I used F8)
> >  . press F8 and see if update_frame_tool_bar is called
>
>
> Well, turns out this happens with -q, but not with -Q.
>
> If I clear tool-bar-map, the update goes away, but running populating it
> with a standard toolbar as per (tool-bar-setup), the updates come back.
> So, I’m not sure what exactly makes this happen - I’m obviously loading a
> whole lot of settings with -q.
>
> I’m assuming I’ll have to give you something more reproducible then, so
> please stand by.
>
>
>

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

Re: Redisplay: NS port, high CPU load

[Eli Zaretskii]

> From: Anders Lindgren <andlind@gmail.com>
> Date: Mon, 13 Jun 2016 20:44:15 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, Alan Third <alan@idiocy.org>, emacs-devel <emacs-devel@gnu.org>
> 
> Unfortunately, I can't reproduce this either, but I have a theory on what is going on.

David said that it didn't happen in "emacs -Q", but did happen in his
normal setup, and AFAIU intended to come up with a recipe.

> When it comes to the HFS coding rewrite, I still think it is valid. The documentation of `define-coding-system'
> indicated that it expects the pre-write function to convert the current buffer and that the two parameters are
> only there for backward compatibility. The function, before the rewrite, was clearly written for an older version
> of `define-coding-system'.

That's not the issue.  The issue is what change(s) to install on the
release branch.  We've lived with the current form of the utf-8-hfs's
pre-write conversion for a long time, so if it is not the root cause
behind the CPU load, I'd like to leave it alone on the release branch,
and fix the real problem instead.

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 14, 2016, at 2:44 AM, Anders Lindgren <andlind@gmail.com> wrote:

> `with-temp-buffer' calls `generate-new-buffer'. This, in turn calls `get-buffer-create', which run `buffer-list-update-hook'. In theory, if a function on this hook would trigger a tool-bar redisplay, `(with-temp-buffer (insert "XXX"))' will too.

buffer-list-update-hook is nil in my setup.

- D

Re: Redisplay: NS port, high CPU load

[David Reitter]

On Jun 14, 2016, at 3:16 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> David said that it didn't happen in "emacs -Q", but did happen in his
> normal setup, and AFAIU intended to come up with a recipe.

OK, I looked into this.  I don’t have time right now for a recipe, but what’s happening is that I’m loading (my version of) tabbar.el, which does this:

(add-hook 'first-change-hook 'tabbar-window-update-tabsets-when-idle)

Only when first-change-hook is set are those extraneous toolbar refreshes made.  The tabbar-window-update function at some point calls  (force-window-update (window-buffer)), which may be what triggers the toolbar refresh.

So, I think that we should think about disabling hooks such as buffer-list-update-hook, first-change-hook, kill-buffer-hook, for temporary buffers.  In my view it just does not make sense to run the hooks for temporary buffers in any sensible scenario.

I also think that this change might affect some modes, if only by triggering bugs.  Therefore I would argue that the change to ucs-normalize is the minimal change to fix my bug, and a good housekeeping change in general.

Re: Redisplay: NS port, high CPU load

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Tue, 14 Jun 2016 20:07:01 +0800
> Cc: Anders Lindgren <andlind@gmail.com>,
>  Alan Third <alan@idiocy.org>,
>  emacs-devel@gnu.org
> 
> OK, I looked into this.  I don’t have time right now for a recipe, but what’s happening is that I’m loading (my version of) tabbar.el, which does this:
> 
> (add-hook 'first-change-hook 'tabbar-window-update-tabsets-when-idle)
> 
> Only when first-change-hook is set are those extraneous toolbar refreshes made.  The tabbar-window-update function at some point calls  (force-window-update (window-buffer)), which may be what triggers the toolbar refresh.

OK, thanks, this explains everything.  Indeed, calling
force-window-update will cause the tool bar to be redisplayed.

> So, I think that we should think about disabling hooks such as buffer-list-update-hook, first-change-hook, kill-buffer-hook, for temporary buffers.  In my view it just does not make sense to run the hooks for temporary buffers in any sensible scenario.

I'm not sure I agree.  Forcing redisplay for such buffers might not
make sense in most situations (though again, I'm not sure it makes no
sense at all), but a hook could do anything at all, and I see no
reason to summarily disable it in such buffers.  I think it's up to
the hook function to be selective if needed.

> I also think that this change might affect some modes, if only by triggering bugs.  Therefore I would argue that the change to ucs-normalize is the minimal change to fix my bug, and a good housekeeping change in general.

I'm okay with the change in ucs-normalize, but I don't think we should
do that on the release branch.  The situation you describe is quite
specialized, and the solution that avoids excess redisplay is to make
the hook function more selective about the buffer in which it runs.

So I think we are okay with the change you committed to master, and
this bug could otherwise be closed.

Thanks for looking into this.

Re: Redisplay: NS port, high CPU load

[Stefan Monnier]

> Only when first-change-hook is set are those extraneous toolbar refreshes
> made.  The tabbar-window-update function at some point calls
> (force-window-update (window-buffer)), which may be what triggers the
> toolbar refresh.

Oh, yes.  Looks like a bug in tabbar.el.  A "first change" in an
undisplayed buffer should't cause a force-window-update in the window
that happens to be currently selected.


        Stefan