unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* NS port:  How to debug excessive garbage collection?
@ 2019-04-11  3:27 Keith David Bershatsky
  2019-04-11 14:14 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-11  3:27 UTC (permalink / raw)
  To: Emacs Devel

I am working on feature requests 22873 (multiple fake cursors) and 17684 (crosshairs that track the cursor position).

On 04/08/2019, I submitted a proof concept patch to the following bug tracker link that applies to a current version of the master branch as of that date/commit.  It works for GUI versions of Emacs on w32, X11 and NS ports:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22873#125

The bulk of my daily workflow is performed on a modified earlier version of the Emacs master branch from 03/28/2016, which uses about 98% of a current version of the display engine (xdisp.c) as of 04/08/2019.  The proof concept patches for each version of Emacs (03/28/2016 versus 04/08/2019) are virtually identical.  The only major difference is that nsterm.m has undergone significant changes over the past few months to accommodate Mojave users (MacOS).

Here is a Youtube link to a screen recording of the 03/28/2016 version of Emacs NS port -- there are virtually no visual pauses whatsoever while I hold down the right arrow key:

https://youtu.be/4vTX1YwrfC4

Here is a Youtube link to a screen recording of the 04/08/2019 version of Emacs NS port -- there is a painfully noticeable pause every few characters while I hold down the right arrow key:

https://youtu.be/RzkB4HKHR6o

The painful garbage collection issues do _not_ plague either the w32 port or the X11 port of Emacs master branch as of 04/08/2019.

Q:  How should I proceed to debug the excessive garbage collection issue on the NS port?

Thanks,

Keith



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
  2019-04-11  3:27 Keith David Bershatsky
@ 2019-04-11 14:14 ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2019-04-11 14:14 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date: Wed, 10 Apr 2019 20:27:49 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> Q:  How should I proceed to debug the excessive garbage collection issue on the NS port?

Does that happen with the unmodified master branch, or only with your
changes?

In any case, I think you should begin by finding out which type of
Lisp data contributes the most to the garbage, e.g. by looking at the
output of 'garbage-collect' (the function).



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
@ 2019-04-11 23:04 Keith David Bershatsky
  2019-04-12  9:30 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-11 23:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Thank you, Eli, for helping me to get started down the road towards diagnosing the excessive garbage collection issue that I am observing on my end.

The garbage collection issue is most noticeable to the naked eye when the fake cursors feature is turned on (e.g., crosshairs) -- using a modified master branch from 04/08/2019.  My naked eye does not perceive any obvious problem when holding down the right arrow-key in the stock / unmodified version of Emacs.

I performed three (3) tests wherein I held the right arrow-key (right-char) depressed for a few seconds in the *GNU Emacs* welcome buffer:

    (progn
      (setq timer-list nil
            timer-idle-list nil)
      (add-hook 'post-command-hook (lambda () (message "%s" (garbage-collect)))))

1.  Stock / Unmodified master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).

2.  Crosshairs turned _off_ using a modified master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).

3.  Crosshairs turned _on_ using a modified master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).

All tests were performed on the NS platform running OSX 10.6.8 with the following build options:

CFLAGS='-Wall -O0 -g3' ./configure \
--with-ns \
--enable-checking='yes,glyphs' \
--enable-check-lisp-object-type \
--without-compress-install \
--without-makeinfo \
--with-gnutls=no \
--with-mailutils \
--without-makeinfo

============================

;;; begin STOCK / UNMODIFIED

((conses 16 12248 37702)
 (symbols 48 1746 1)
 (strings 32 4063 432)
 (string-bytes 1 118324)
 (vectors 16 3486)
 (vector-slots 8 52564 9150)
 (floats 8 9 88)
 (intervals 56 55 25)
 (buffers 992 9))

((conses 16 12249 37701)
 (symbols 48 1746 1)
 (strings 32 4063 432)
 (string-bytes 1 118324)
 (vectors 16 3486)
 (vector-slots 8 52564 9150)
 (floats 8 9 88)
 (intervals 56 55 25)
 (buffers 992 9))

;;; end STOCK UNMODIFIED

============================

;;; begin MODIFIED -- CROSSHAIRS "OFF"

((conses 16 4650 7834)
 (symbols 48 1312 1)
 (strings 32 595 1792)
 (string-bytes 1 36726)
 (vectors 16 3654)
 (vector-slots 8 52893 12656)
 (floats 8 32 261)
 (intervals 56 54 26)
 (buffers 1072 9))

((conses 16 4651 7833)
 (symbols 48 1312 1)
 (strings 32 595 1792)
 (string-bytes 1 36726)
 (vectors 16 3654)
 (vector-slots 8 52893 12656)
 (floats 8 32 261)
 (intervals 56 54 26)
 (buffers 1072 9))

;;; end MODIFIED -- CROSSHAIRS "OFF"

============================

;;; begin MODIFIED -- CROSSHAIRS "ON"

((conses 16 5625 6858)
 (symbols 48 1314 0)
 (strings 32 597 1790)
 (string-bytes 1 36822)
 (vectors 16 3722)
 (vector-slots 8 53307 12242)
 (floats 8 233 421)
 (intervals 56 54 26)
 (buffers 1072 9))

((conses 16 5626 6993)
 (symbols 48 1314 0)
 (strings 32 597 1790)
 (string-bytes 1 36822)
 (vectors 16 3722)
 (vector-slots 8 53307 12242)
 (floats 8 233 547)
 (intervals 56 54 26)
 (buffers 1072 9))

;;; end MODIFIED -- CROSSHAIRS "ON"

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-11-2019 07:14:53] <11 Apr 2019 17:14:53 +0300>
> From: Eli Zaretskii <eliz@gnu.org>
> To: Keith David Bershatsky <esq@lawlist.com>
> CC: emacs-devel@gnu.org
> Subject: Re: NS port:  How to debug excessive garbage collection?
> 
> > Date: Wed, 10 Apr 2019 20:27:49 -0700
> > From: Keith David Bershatsky <esq@lawlist.com>
> >
> > Q:  How should I proceed to debug the excessive garbage collection issue on the NS port?
> 
> Does that happen with the unmodified master branch, or only with your
> changes?
> 
> In any case, I think you should begin by finding out which type of
> Lisp data contributes the most to the garbage, e.g. by looking at the
> output of 'garbage-collect' (the function).



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
  2019-04-11 23:04 Keith David Bershatsky
@ 2019-04-12  9:30 ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2019-04-12  9:30 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Thu, 11 Apr 2019 16:04:47 -0700
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> I performed three (3) tests wherein I held the right arrow-key (right-char) depressed for a few seconds in the *GNU Emacs* welcome buffer:
> 
>     (progn
>       (setq timer-list nil
>             timer-idle-list nil)
>       (add-hook 'post-command-hook (lambda () (message "%s" (garbage-collect)))))

I don't think this is a good idea.  You need to see the results after
performing the same commands in each case, not after every command.

Or maybe just put a breakpoint inside garbage_collect_1, and see which
of the *_consed variables goes up more in the modified version as
compared to the unchanged master.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
@ 2019-04-13  5:55 Keith David Bershatsky
  2019-04-13  6:48 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-13  5:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Thank you, Eli, for letting me know that my previous test was not the correct way to do it.  I added a little bit of code from garbage-collect to garbage_collect_1 so that messages to stderr are automatically generated when garbage collection occurs.

In all three (3) of the new tests, I used the following Lisp code at the outset and then held down the right arrow key (right-char) in the *GNU Emacs* welcome buffer:

(progn
  (blink-cursor-mode -1)
  (global-eldoc-mode -1)
  (setq timer-list nil
        timer-idle-list nil))

============================

;;; begin STOCK / UNMODIFIED Emacs

((conses 16 12347 39900)
 (symbols 48 1746 1)
 (strings 32 4057 438)
 (string-bytes 1 117832)
 (vectors 16 3477)
 (vector-slots 8 52314 9400)
 (floats 8 7 17)
 (intervals 56 54 26)
 (buffers 992 9))

;;; end STOCK UNMODIFIED Emacs

============================

;;; begin MODIFIED Emacs -- CROSSHAIRS "OFF"

((conses 16 4624 17450)
 (symbols 48 1312 0)
 (strings 32 580 1807)
 (string-bytes 1 34827)
 (vectors 16 3638)
 (vector-slots 8 52667 12372)
 (floats 8 17 425)
 (intervals 56 51 29)
 (buffers 1072 8))

;;; end MODIFIED Emacs -- CROSSHAIRS "OFF"

============================

;;; begin MODIFIED Emacs -- CROSSHAIRS "ON"

((conses 16 6032 17006)
 (symbols 48 1314 1)
 (strings 32 595 1854)
 (string-bytes 1 36097)
 (vectors 16 3729)
 (vector-slots 8 53337 12722)
 (floats 8 264 805)
 (intervals 56 53 27)
 (buffers 1072 9))

;;; end MODIFIED Emacs -- CROSSHAIRS "ON"

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Here is the diff I used to automatically generate messages to stderr during garbage collection:

diff --git a/src/alloc.c b/src/alloc.c
index dd78386..b503b74 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6206,6 +6206,53 @@ garbage_collect_1 (struct gcstat *gcst)
 
   *gcst = gcstat;
 
+
+/* *************************************************************************** */
+/* begin DEBUGGING */
+
+  Lisp_Object total[] = {
+    list4 (Qconses, make_fixnum (sizeof (struct Lisp_Cons)),
+           make_int (gcstat.total_conses),
+           make_int (gcstat.total_free_conses)),
+    list4 (Qsymbols, make_fixnum (sizeof (struct Lisp_Symbol)),
+           make_int (gcstat.total_symbols),
+           make_int (gcstat.total_free_symbols)),
+    list4 (Qstrings, make_fixnum (sizeof (struct Lisp_String)),
+           make_int (gcstat.total_strings),
+           make_int (gcstat.total_free_strings)),
+    list3 (Qstring_bytes, make_fixnum (1),
+           make_int (gcstat.total_string_bytes)),
+    list3 (Qvectors,
+           make_fixnum (header_size + sizeof (Lisp_Object)),
+           make_int (gcstat.total_vectors)),
+    list4 (Qvector_slots, make_fixnum (word_size),
+           make_int (gcstat.total_vector_slots),
+           make_int (gcstat.total_free_vector_slots)),
+    list4 (Qfloats, make_fixnum (sizeof (struct Lisp_Float)),
+           make_int (gcstat.total_floats),
+           make_int (gcstat.total_free_floats)),
+    list4 (Qintervals, make_fixnum (sizeof (struct interval)),
+           make_int (gcstat.total_intervals),
+           make_int (gcstat.total_free_intervals)),
+    list3 (Qbuffers, make_fixnum (sizeof (struct buffer)),
+           make_int (gcstat.total_buffers)),
+
+#ifdef DOUG_LEA_MALLOC
+    list4 (Qheap, make_fixnum (1024),
+           make_int ((mallinfo ().uordblks + 1023) >> 10),
+           make_int ((mallinfo ().fordblks + 1023) >> 10)),
+#endif
+  };
+
+  Lisp_Object val = CALLMANY (Flist, total);
+  Lisp_Object string = Fprin1_to_string (val, Qnil);
+  char *char_string = SSDATA (string);
+  fprintf (stderr, "%s\n", char_string);
+
+/* end DEBUGGING */
+/* *************************************************************************** */
+
+
   /* GC is complete: now we can run our finalizer callbacks.  */
   run_finalizers (&doomed_finalizers);



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
  2019-04-13  5:55 Keith David Bershatsky
@ 2019-04-13  6:48 ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2019-04-13  6:48 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Fri, 12 Apr 2019 22:55:42 -0700
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> Thank you, Eli, for letting me know that my previous test was not the correct way to do it.  I added a little bit of code from garbage-collect to garbage_collect_1 so that messages to stderr are automatically generated when garbage collection occurs.

That's not what I meant.  I meant to look at the values of the
*_consed variables, here's the full list:

  intervals_consed
  strings_consed
  string_chars_consed
  floats_consed
  cons_cells_consed
  vector_cells_consed
  symbols_consed

You can use the function memory-use-counts, if that's more
convenient.  The question is: which of the above counters changes the
most between two GCs, and how does the answer to that depend on the
version of Emacs?



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
@ 2019-04-13 16:31 Keith David Bershatsky
  2019-04-13 17:02 ` Alex Gramiak
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-13 16:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Thank you, Eli, for directing me to the function memory-use-counts.  I inserted the innards of that function into garbage_collect_1 and ran new tests, selecting the first 5 stderr printouts for each test using the following Lisp command (at the outset of each test) and then holding down the right arrow key (right-char):

(progn
  (find-library "simple")
  (fundamental-mode)
  (blink-cursor-mode -1)
  (global-eldoc-mode -1)
  (setq timer-list nil
        timer-idle-list nil
        crosshairs t))

============================

;;; begin STOCK / UNMODIFIED

((cons_cells_consed 2285210)
 (symbols_consed 16084)
 (strings_consed 95613)
 (string_chars_consed 2091140)
 (vector_cells_consed 6877366)
 (floats_consed 568)
 (intervals_consed 112))

((cons_cells_consed 2327648)
 (symbols_consed 16084)
 (strings_consed 97813)
 (string_chars_consed 2106916)
 (vector_cells_consed 6878987)
 (floats_consed 569)
 (intervals_consed 112))

((cons_cells_consed 2370021)
 (symbols_consed 16084)
 (strings_consed 100103)
 (string_chars_consed 2123400)
 (vector_cells_consed 6880233)
 (floats_consed 570)
 (intervals_consed 112))

((cons_cells_consed 2412389)
 (symbols_consed 16084)
 (strings_consed 102389)
 (string_chars_consed 2139936)
 (vector_cells_consed 6881507)
 (floats_consed 571)
 (intervals_consed 112))

((cons_cells_consed 2455285)
 (symbols_consed 16084)
 (strings_consed 104220)
 (string_chars_consed 2152636)
 (vector_cells_consed 6884284)
 (floats_consed 572)
 (intervals_consed 112))


;;; end STOCK UNMODIFIED

============================

;;; begin MODIFIED -- CROSSHAIRS "OFF"

((cons_cells_consed 2282540)
 (symbols_consed 16120)
 (strings_consed 95918)
 (string_chars_consed 2094233)
 (vector_cells_consed 6880135)
 (floats_consed 1656)
 (intervals_consed 112))

((cons_cells_consed 2313916)
 (symbols_consed 16120)
 (strings_consed 99976)
 (string_chars_consed 2122818)
 (vector_cells_consed 6885827)
 (floats_consed 7045)
 (intervals_consed 112))

((cons_cells_consed 2345230)
 (symbols_consed 16120)
 (strings_consed 104045)
 (string_chars_consed 2151571)
 (vector_cells_consed 6891513)
 (floats_consed 12446)
 (intervals_consed 112))

((cons_cells_consed 2376738)
 (symbols_consed 16120)
 (strings_consed 108070)
 (string_chars_consed 2179909)
 (vector_cells_consed 6897231)
 (floats_consed 17787)
 (intervals_consed 112))

((cons_cells_consed 2408250)
 (symbols_consed 16120)
 (strings_consed 112081)
 (string_chars_consed 2208228)
 (vector_cells_consed 6903029)
 (floats_consed 23104)
 (intervals_consed 112))

;;; end MODIFIED -- CROSSHAIRS "OFF"

============================

;;; begin MODIFIED -- CROSSHAIRS "ON"

((cons_cells_consed 2323418)
 (symbols_consed 16122)
 (strings_consed 98901)
 (string_chars_consed 2116217)
 (vector_cells_consed 6883922)
 (floats_consed 5413)
 (intervals_consed 112))

((cons_cells_consed 2360412)
 (symbols_consed 16122)
 (strings_consed 102038)
 (string_chars_consed 2139422)
 (vector_cells_consed 6887522)
 (floats_consed 9014)
 (intervals_consed 112))

((cons_cells_consed 2397407)
 (symbols_consed 16122)
 (strings_consed 105191)
 (string_chars_consed 2162776)
 (vector_cells_consed 6891122)
 (floats_consed 12615)
 (intervals_consed 112))

((cons_cells_consed 2434401)
 (symbols_consed 16122)
 (strings_consed 108329)
 (string_chars_consed 2185981)
 (vector_cells_consed 6894722)
 (floats_consed 16216)
 (intervals_consed 112))

((cons_cells_consed 2471395)
 (symbols_consed 16122)
 (strings_consed 111473)
 (string_chars_consed 2209234)
 (vector_cells_consed 6898322)
 (floats_consed 19817)
 (intervals_consed 112))

;;; end MODIFIED -- CROSSHAIRS "ON"

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

diff --git a/src/alloc.c b/src/alloc.c
index dd78386..0bd62c7 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6206,6 +6206,53 @@ garbage_collect_1 (struct gcstat *gcst)
 
   *gcst = gcstat;
 
+
+/* *************************************************************************** */
+/* begin DEBUGGING */
+
+  Lisp_Object total[] = {
+    list2 (intern ("cons_cells_consed"), make_int (cons_cells_consed)),
+    list2 (intern ("symbols_consed"), make_int (symbols_consed)),
+    list2 (intern ("strings_consed"), make_int (strings_consed)),
+    list2 (intern ("string_chars_consed"), make_int (string_chars_consed)),
+    list2 (intern ("vector_cells_consed"), make_int (vector_cells_consed)),
+    list2 (intern ("floats_consed"), make_int (floats_consed)),
+    list2 (intern ("intervals_consed"), make_int (intervals_consed)),
+/*
+    list4 (Qconses, make_fixnum (sizeof (struct Lisp_Cons)),
+           make_int (gcstat.total_conses), make_int (gcstat.total_free_conses)),
+    list4 (Qsymbols, make_fixnum (sizeof (struct Lisp_Symbol)),
+           make_int (gcstat.total_symbols), make_int (gcstat.total_free_symbols)),
+    list4 (Qstrings, make_fixnum (sizeof (struct Lisp_String)),
+           make_int (gcstat.total_strings), make_int (gcstat.total_free_strings)),
+    list3 (Qstring_bytes, make_fixnum (1), make_int (gcstat.total_string_bytes)),
+    list3 (Qvectors, make_fixnum (header_size + sizeof (Lisp_Object)),
+           make_int (gcstat.total_vectors)),
+    list4 (Qvector_slots, make_fixnum (word_size), make_int (gcstat.total_vector_slots),
+           make_int (gcstat.total_free_vector_slots)),
+    list4 (Qfloats, make_fixnum (sizeof (struct Lisp_Float)),
+           make_int (gcstat.total_floats), make_int (gcstat.total_free_floats)),
+    list4 (Qintervals, make_fixnum (sizeof (struct interval)),
+           make_int (gcstat.total_intervals), make_int (gcstat.total_free_intervals)),
+    list3 (Qbuffers, make_fixnum (sizeof (struct buffer)),
+           make_int (gcstat.total_buffers)),
+#ifdef DOUG_LEA_MALLOC
+    list4 (Qheap, make_fixnum (1024),
+           make_int ((mallinfo ().uordblks + 1023) >> 10),
+           make_int ((mallinfo ().fordblks + 1023) >> 10)),
+#endif
+*/
+  };
+
+  Lisp_Object val = CALLMANY (Flist, total);
+  Lisp_Object string = Fprin1_to_string (val, Qnil);
+  char *char_string = SSDATA (string);
+  fprintf (stderr, "%s\n", char_string);
+
+/* end DEBUGGING */
+/* *************************************************************************** */
+
+
   /* GC is complete: now we can run our finalizer callbacks.  */
   run_finalizers (&doomed_finalizers);



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
  2019-04-13 16:31 Keith David Bershatsky
@ 2019-04-13 17:02 ` Alex Gramiak
  0 siblings, 0 replies; 20+ messages in thread
From: Alex Gramiak @ 2019-04-13 17:02 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: Eli Zaretskii, emacs-devel


Keith David Bershatsky <esq@lawlist.com> writes:

> ;;; begin STOCK / UNMODIFIED
> [...]
> ((cons_cells_consed 2455285)
>  (symbols_consed 16084)
>  (strings_consed 104220)
>  (string_chars_consed 2152636)
>  (vector_cells_consed 6884284)
>  (floats_consed 572)
>  (intervals_consed 112))
>
>
> ;;; end STOCK UNMODIFIED
>
> ============================
>
> ;;; begin MODIFIED -- CROSSHAIRS "OFF"

> [...]
> ((cons_cells_consed 2408250)
>  (symbols_consed 16120)
>  (strings_consed 112081)
>  (string_chars_consed 2208228)
>  (vector_cells_consed 6903029)
>  (floats_consed 23104)
>  (intervals_consed 112))
>
> ;;; end MODIFIED -- CROSSHAIRS "OFF"
>
> ============================
>
> ;;; begin MODIFIED -- CROSSHAIRS "ON"
>
> [...]
> ((cons_cells_consed 2471395)
>  (symbols_consed 16122)
>  (strings_consed 111473)
>  (string_chars_consed 2209234)
>  (vector_cells_consed 6898322)
>  (floats_consed 19817)
>  (intervals_consed 112))

Your floats look out of control. I checked your latest diff, and found
this:

+  int int_red = XFIXNUM (Ftruncate (make_float (term_red), Qnil));
+  int int_green = XFIXNUM (Ftruncate (make_float (term_green), Qnil));;
+  int int_blue = XFIXNUM (Ftruncate (make_float (term_blue), Qnil));;

Why do you make a Lisp float just to truncate it? Can you run your test
again using one of the following?

https://en.cppreference.com/w/c/numeric/math/round



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
@ 2019-04-13 18:07 Keith David Bershatsky
  2019-04-13 21:41 ` Alex Gramiak
  2019-04-14  3:47 ` Daniel Colascione
  0 siblings, 2 replies; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-13 18:07 UTC (permalink / raw)
  To: Alex Gramiak; +Cc: Eli Zaretskii, emacs-devel

Thank you, Alex, for helping to diagnose the issue of excessive garbage collection.  I observe the issue when building Emacs on an OSX machine using the --with-ns flag.  The issue may also occur on the w32 and X11 builds of Emacs, but it is not noticeable to my naked eye.  The NS build, on the other hand, permits me to see an appreciable pause for garbage collection every few keystrokes.  The garbage collection tests with *_consed that I performed today were on the NS port of Emacs.

The usage of Ftruncate in the 04/08/2019 proof concept patch of fake cursors occurs only in w32term.c and xterm.c.

My general understanding is as follows:

1.  Emacs uses nsterm.m/h when building on an OSX machine and the --with-ns flag.

2.  Emacs uses xterm.c when building on an OSX machine that has X11 installed and the following flags are used:  --with-x (yes) and --with-ns (no).

3.  Emacs uses w32term.c when building on a Windows machine.

I would be pleased to remove Ftruncate and replace it with roundf.  Thank you for the suggestion.  The development of the fake cursors feature has been a work in progress for a little over 3 years now ... learning some C coding along the way.  I have seen roundf in a few internet examples over the years, and have a vague recollection of trying to use it somewhere -- however, I cannot remember in what context.

Perhaps there is a similar mistake I have made that would affect the NS build of Emacs, or perhaps would affect all of the builds (NS/w32/X11).  It is interesting that excessive garbage collection occurs even with the fake cursors feature turned _off_, and it gets much worse when that feature is turned _on_.

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-13-2019 10:02:07] <13 Apr 2019 11:02:07 -0600>
> From: Alex Gramiak <agrambot@gmail.com>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> Subject: Re: NS port:  How to debug excessive garbage collection?
> 
> Keith David Bershatsky <esq@lawlist.com> writes:
> 
> > ;;; begin STOCK / UNMODIFIED
> > [...]
> > ((cons_cells_consed 2455285)
> >  (symbols_consed 16084)
> >  (strings_consed 104220)
> >  (string_chars_consed 2152636)
> >  (vector_cells_consed 6884284)
> >  (floats_consed 572)
> >  (intervals_consed 112))
> >
> >
> > ;;; end STOCK UNMODIFIED
> >
> > ============================
> >
> > ;;; begin MODIFIED -- CROSSHAIRS "OFF"
> 
> > [...]
> > ((cons_cells_consed 2408250)
> >  (symbols_consed 16120)
> >  (strings_consed 112081)
> >  (string_chars_consed 2208228)
> >  (vector_cells_consed 6903029)
> >  (floats_consed 23104)
> >  (intervals_consed 112))
> >
> > ;;; end MODIFIED -- CROSSHAIRS "OFF"
> >
> > ============================
> >
> > ;;; begin MODIFIED -- CROSSHAIRS "ON"
> >
> > [...]
> > ((cons_cells_consed 2471395)
> >  (symbols_consed 16122)
> >  (strings_consed 111473)
> >  (string_chars_consed 2209234)
> >  (vector_cells_consed 6898322)
> >  (floats_consed 19817)
> >  (intervals_consed 112))
> 
> Your floats look out of control. I checked your latest diff, and found
> this:
> 
> +  int int_red = XFIXNUM (Ftruncate (make_float (term_red), Qnil));
> +  int int_green = XFIXNUM (Ftruncate (make_float (term_green), Qnil));;
> +  int int_blue = XFIXNUM (Ftruncate (make_float (term_blue), Qnil));;
> 
> Why do you make a Lisp float just to truncate it? Can you run your test
> again using one of the following?
> 
> https://en.cppreference.com/w/c/numeric/math/round



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port:  How to debug excessive garbage collection?
  2019-04-13 18:07 Keith David Bershatsky
@ 2019-04-13 21:41 ` Alex Gramiak
  2019-04-14  3:47 ` Daniel Colascione
  1 sibling, 0 replies; 20+ messages in thread
From: Alex Gramiak @ 2019-04-13 21:41 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: Eli Zaretskii, emacs-devel

Keith David Bershatsky <esq@lawlist.com> writes:

> Thank you, Alex, for helping to diagnose the issue of excessive garbage
> collection. I observe the issue when building Emacs on an OSX machine using the
> --with-ns flag. The issue may also occur on the w32 and X11 builds of Emacs, but
> it is not noticeable to my naked eye. The NS build, on the other hand, permits
> me to see an appreciable pause for garbage collection every few keystrokes. The
> garbage collection tests with *_consed that I performed today were on the NS
> port of Emacs.

If you haven't already, IMO it would do well to perform these tests on
w32 or X11 to see if there's any difference even if it's not noticeable.

> The usage of Ftruncate in the 04/08/2019 proof concept patch of fake cursors occurs only in w32term.c and xterm.c.

Right, I didn't notice that the nsterm.c implementation was missing it.

> Perhaps there is a similar mistake I have made that would affect the NS build of
> Emacs, or perhaps would affect all of the builds (NS/w32/X11).

Just to make sure, how about putting the debug_fg and the associated
ASET/make_float calls inside an if (debug_p) in mc_update_text_area?
It's probably not it, but it doesn't hurt to cross it off.

This might not help much, but I noticed this section:

+  enum face_id special_char_face_id =
+    lookup_named_face (w, f, intern ("+-special-character-face"), true);
+  struct face *special_char_face = FACE_FROM_ID (f, special_char_face_id);
+  Lisp_Object special_char_fg_color = special_char_face->lface[LFACE_FOREGROUND_INDEX];
+  Lisp_Object special_char_fg_vec = mc_color_vector_calc (w, special_char_fg_color);
+  if (glyph != NULL
+      && glyph->u.ch == 95
+      && cursor_type == HBAR_CURSOR)
+    foreground = special_char_fg_vec;

It would be better to place everything within the conditional here,
since mc_color_vector_calc calls make_float:

+  if (glyph != NULL
+      && glyph->u.ch == 95
+      && cursor_type == HBAR_CURSOR)
+    {
+      enum face_id special_char_face_id =
+        lookup_named_face (w, f, intern ("+-special-character-face"), true);
+      struct face *special_char_face = FACE_FROM_ID (f, special_char_face_id);
+      Lisp_Object special_char_fg_color = special_char_face->lface[LFACE_FOREGROUND_INDEX];
+      Lisp_Object special_char_fg_vec = mc_color_vector_calc (w, special_char_fg_color);     
+      foreground = special_char_fg_vec;
+    }

P.S.

I also noticed SSDATA (build_string ("string")) that I believe should
just be "string" (with the return type being const char *).

Also, the CHARACTERP (make_fixnum (arg)) could be:

  0 <= arg && arg <= MAX_CHAR

which is how other code in Emacs appears to check if a C int is a
character.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
  2019-04-13 18:07 Keith David Bershatsky
  2019-04-13 21:41 ` Alex Gramiak
@ 2019-04-14  3:47 ` Daniel Colascione
  1 sibling, 0 replies; 20+ messages in thread
From: Daniel Colascione @ 2019-04-14  3:47 UTC (permalink / raw)
  To: Keith David Bershatsky, Alex Gramiak; +Cc: Eli Zaretskii, emacs-devel

On 4/13/19 11:07 AM, Keith David Bershatsky wrote:
> Thank you, Alex, for helping to diagnose the issue of excessive garbage collection.  I observe the issue when building Emacs on an OSX machine using the --with-ns flag.  The issue may also occur on the w32 and X11 builds of Emacs, but it is not noticeable to my naked eye.  The NS build, on the other hand, permits me to see an appreciable pause for garbage collection every few keystrokes.  The garbage collection tests with *_consed that I performed today were on the NS port of Emacs.
> 
> The usage of Ftruncate in the 04/08/2019 proof concept patch of fake cursors occurs only in w32term.c and xterm.c.
> 
> My general understanding is as follows:
> 
> 1.  Emacs uses nsterm.m/h when building on an OSX machine and the --with-ns flag.
> 
> 2.  Emacs uses xterm.c when building on an OSX machine that has X11 installed and the following flags are used:  --with-x (yes) and --with-ns (no).
> 
> 3.  Emacs uses w32term.c when building on a Windows machine.
> 
> I would be pleased to remove Ftruncate and replace it with roundf.  Thank you for the suggestion.  The development of the fake cursors feature has been a work in progress for a little over 3 years now ... learning some C coding along the way.  I have seen roundf in a few internet examples over the years, and have a vague recollection of trying to use it somewhere -- however, I cannot remember in what context.
> 
> Perhaps there is a similar mistake I have made that would affect the NS build of Emacs, or perhaps would affect all of the builds (NS/w32/X11).  It is interesting that excessive garbage collection occurs even with the fake cursors feature turned _off_, and it gets much worse when that feature is turned _on_.
> 
> Keith
> 
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> 
>> Date: [04-13-2019 10:02:07] <13 Apr 2019 11:02:07 -0600>
>> From: Alex Gramiak <agrambot@gmail.com>
>> To: Keith David Bershatsky <esq@lawlist.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
>> Subject: Re: NS port:  How to debug excessive garbage collection?
>>
>> Keith David Bershatsky <esq@lawlist.com> writes:
>>
>>> ;;; begin STOCK / UNMODIFIED
>>> [...]
>>> ((cons_cells_consed 2455285)
>>>   (symbols_consed 16084)
>>>   (strings_consed 104220)
>>>   (string_chars_consed 2152636)
>>>   (vector_cells_consed 6884284)
>>>   (floats_consed 572)
>>>   (intervals_consed 112))
>>>
>>>
>>> ;;; end STOCK UNMODIFIED
>>>
>>> ============================
>>>
>>> ;;; begin MODIFIED -- CROSSHAIRS "OFF"
>>
>>> [...]
>>> ((cons_cells_consed 2408250)
>>>   (symbols_consed 16120)
>>>   (strings_consed 112081)
>>>   (string_chars_consed 2208228)
>>>   (vector_cells_consed 6903029)
>>>   (floats_consed 23104)
>>>   (intervals_consed 112))
>>>
>>> ;;; end MODIFIED -- CROSSHAIRS "OFF"
>>>
>>> ============================
>>>
>>> ;;; begin MODIFIED -- CROSSHAIRS "ON"
>>>
>>> [...]
>>> ((cons_cells_consed 2471395)
>>>   (symbols_consed 16122)
>>>   (strings_consed 111473)
>>>   (string_chars_consed 2209234)
>>>   (vector_cells_consed 6898322)
>>>   (floats_consed 19817)
>>>   (intervals_consed 112))
>>
>> Your floats look out of control. I checked your latest diff, and found
>> this:
>>
>> +  int int_red = XFIXNUM (Ftruncate (make_float (term_red), Qnil));
>> +  int int_green = XFIXNUM (Ftruncate (make_float (term_green), Qnil));;
>> +  int int_blue = XFIXNUM (Ftruncate (make_float (term_blue), Qnil));;
>>
>> Why do you make a Lisp float just to truncate it? Can you run your test
>> again using one of the following?

I really wish we had a generational collector. :-(



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
@ 2019-04-14  7:41 Keith David Bershatsky
  0 siblings, 0 replies; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-14  7:41 UTC (permalink / raw)
  To: Alex Gramiak; +Cc: Eli Zaretskii, Daniel Colascione, emacs-devel

Thank you, Alex, for the advice regarding testing for garbage collection issues on the other two platforms (i.e., X11 and w32), and also regarding make certain edits/improvements to the existing code of multiple fake cursors.  I will work on these in the next day or so.

Using an unsophisticated method of commenting out various modifications that I have made over the past few years, I am working towards coming up with a minimal (or at least significantly reduced) working example to demonstrate the issue that I am observing.  I will need another 1 to 2 hours with a cup of coffee after a good night sleep ....

At this point, I am about 98 percent certain that the main issue traces to the function mc_helper and the cache for fake cursors that have already been laid.  In a nutshell, Emacs gets bogged down by a list of 100 elements of sub-lists -- and this is true even when the sub-list is reduced to just one element (instead of fourteen); e.g.,:

  '((make_fixnum (1))(make_fixnum (2))(make_fixnum (3)) ... (make_fixnum (99))

Eliminating other calls to the creation of floats appears to help reduce the overhead a little, but the main problem occurs with the master cache created by mc_helper.

I'll continue chiseling away tomorrow ...

Keith



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
@ 2019-04-14 19:46 Keith David Bershatsky
  2019-04-14 23:31 ` Alex Gramiak
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-14 19:46 UTC (permalink / raw)
  To: agrambot, eliz, dancol; +Cc: emacs-devel

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

The attched patch (debug_mwe_001.diff) is a minimal working example that demonstrates the issue on all three platforms (X11, W32, NS) using the GUI version of Emacs built from the master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).

The snippet that I am pasting and evaluating in the minibuffer is as follows:

(progn
  (find-library "simple")
  (fundamental-mode)
  (blink-cursor-mode -1)
  (global-eldoc-mode -1)
  (setq timer-list nil
        timer-idle-list nil
        crosshairs t))

Here are Youtube links to screen recordings of the minimal working example on all three platforms:

NS (screen recording):  https://youtu.be/4IzXfP2j2GY

X11 (screen recording):  https://youtu.be/zrRH72qdmx0

W32 (screen recording):  https://youtu.be/cfIG4fbkesY

MINIMAL WORKING EXAMPLE:

1.  Imaginary / Pretend:  We imagine that all fake cursors are erased at the outset of update_window while w->current_matrix is still valid; i.e., before scrolling_window does its thing.

2.  The master cache of fake cursors (w->ch_cache) is set to Qnil.

3.  Imaginary / Pretend:  We imagine .... As to the rows in the w->desired_matrix that must be updated with update_text_area, we draw fake cursors immediately after draw_glyphs finishes updating the row.  As we are laying fake cursors, we use a temporary cache (w->mc_temp_cache) to store the relevant data so that we can redraw any fake cursors that get erased because they are left/right_overwritten as determined by draw_glyphs.  Once the row has been updated with fake cursors, we set the termporary cache (w->mc_temp_cache) to Qnil and we set the master cache with the new data -- appending new data if the cache is non-nil.

4.  As to all remaining rows that are not updated with update_text_area (which uses w->desired_matrix), we use the w->current_matrix and draw/cache the fake cursors using the same approach as mentioned in the preceding step; i.e., a temporary cache (w->mc_temp_cache) so that we can fix any fake cursors that got left/right_overwritten and then that cache is set to Qnil and the master cache (w->mc_cache) is updated.

In the above minimal working example, the window Lisp_Object caches are rather simple:

'((make_fixnum (1))
  (make_fixnum (2))
  (make_fixnum (3))
  ...
  (make_fixnum (99)))



[-- Attachment #2: debug_mwe_001.diff --]
[-- Type: application/diff, Size: 9565 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
  2019-04-14 19:46 NS port: How to debug excessive garbage collection? Keith David Bershatsky
@ 2019-04-14 23:31 ` Alex Gramiak
  0 siblings, 0 replies; 20+ messages in thread
From: Alex Gramiak @ 2019-04-14 23:31 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: eliz, dancol, emacs-devel

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

Keith David Bershatsky <esq@lawlist.com> writes:

> The attched patch (debug_mwe_001.diff) is a minimal working example that demonstrates the issue on all three platforms (X11, W32, NS) using the GUI version of Emacs built from the master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).
>
> The snippet that I am pasting and evaluating in the minibuffer is as follows:
>
> (progn
>   (find-library "simple")
>   (fundamental-mode)
>   (blink-cursor-mode -1)
>   (global-eldoc-mode -1)
>   (setq timer-list nil
>         timer-idle-list nil
>         crosshairs t))
>
> Here are Youtube links to screen recordings of the minimal working example on all three platforms:
>
> NS (screen recording):  https://youtu.be/4IzXfP2j2GY
>
> X11 (screen recording):  https://youtu.be/zrRH72qdmx0
>
> W32 (screen recording):  https://youtu.be/cfIG4fbkesY
>
> MINIMAL WORKING EXAMPLE:
>
> 1.  Imaginary / Pretend:  We imagine that all fake cursors are erased at the outset of update_window while w->current_matrix is still valid; i.e., before scrolling_window does its thing.
>
> 2.  The master cache of fake cursors (w->ch_cache) is set to Qnil.
>
> 3.  Imaginary / Pretend:  We imagine .... As to the rows in the w->desired_matrix that must be updated with update_text_area, we draw fake cursors immediately after draw_glyphs finishes updating the row.  As we are laying fake cursors, we use a temporary cache (w->mc_temp_cache) to store the relevant data so that we can redraw any fake cursors that get erased because they are left/right_overwritten as determined by draw_glyphs.  Once the row has been updated with fake cursors, we set the termporary cache (w->mc_temp_cache) to Qnil and we set the master cache with the new data -- appending new data if the cache is non-nil.
>
> 4.  As to all remaining rows that are not updated with update_text_area (which uses w->desired_matrix), we use the w->current_matrix and draw/cache the fake cursors using the same approach as mentioned in the preceding step; i.e., a temporary cache (w->mc_temp_cache) so that we can fix any fake cursors that got left/right_overwritten and then that cache is set to Qnil and the master cache (w->mc_cache) is updated.
>
> In the above minimal working example, the window Lisp_Object caches are rather simple:
>
> '((make_fixnum (1))
>   (make_fixnum (2))
>   (make_fixnum (3))
>   ...
>   (make_fixnum (99)))

If I'm looking at this right, the float issue is now resolved, but
there's still a cons cell issue. A way to fix this in the MWE is to
reuse the cons cells rather than making new cells each time. I've
included a diff below (that just includes the parts I touched on your
diff) that should solve, or at least mitigate, your problem. Hopefully
it's useful.

Looking at your actual implementation though, I don't see a reason for
your cache to be a Lisp_Object at all; am I missing something? It seems
that you could just use a C array of length 14. Not making any cons
cells at all is the best approach if you can do so.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mc --]
[-- Type: text/x-patch, Size: 2141 bytes --]

diff --git a/src/dispnew.c b/src/dispnew.c
index ccb08ec1b9..068296212f 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -3497,6 +3497,40 @@ update_window (struct window *w, bool force_p)
 
     set_cursor:
 
+
+/* *************************************************************************** */
+/* MULTIPLE-CURSORS */
+
+   for (struct glyph_row *desired_row = MATRIX_ROW (desired_matrix, 0);
+        desired_row < end
+        && (force_p || !input_pending)
+        && BUFFERP (w->contents)
+        && (!NILP (BVAR (XBUFFER (w->contents), crosshairs)));
+        ++desired_row)
+    {
+      if (desired_row->enabled_p)
+        continue;
+      wset_mc_temp_cache (w, Qnil);
+      int vpos = MATRIX_ROW_VPOS (desired_row, desired_matrix);
+      struct glyph_matrix *current_matrix = w->current_matrix;
+      struct glyph_row *current_row = MATRIX_ROW (current_matrix, vpos);
+      if (current_row->enabled_p)
+        {
+          Lisp_Object head = w->mc_temp_cache;
+          // Replace each element with the Lisp integer position
+          for (int i = 0; CONSP (head); ++i, head = XCDR (head))
+            XSETCAR (head, make_fixnum (i));
+        }
+    }
+
+   Lisp_Object head = w->ch_cache;
+   // Replace each element with the Lisp integer position
+   for (int i = 0; CONSP (head); ++i, head = XCDR (head))
+     XSETCAR (head, make_fixnum (i));
+
+/* *************************************************************************** */
+
+
       /* Update the header line after scrolling because a new header
 	 line would otherwise overwrite lines at the top of the window
 	 that can be scrolled.  */
diff --git a/src/window.c b/src/window.c
index ef2ed63850..6ee1f65d9f 100644
--- a/src/window.c
+++ b/src/window.c
@@ -4252,6 +4252,9 @@ make_window (void)
   w->scroll_bar_width = -1;
   w->scroll_bar_height = -1;
   w->column_number_displayed = -1;
+  /* Your MC caches -- note there's only 2 lists ever created for them */
+  w->ch_cache = Fmake_list (make_fixnum (100), Qnil);
+  w->mc_temp_cache = Fmake_list (make_fixnum (100), Qnil);
   /* Reset window_list.  */
   Vwindow_list = Qnil;
   /* Return window.  */

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
@ 2019-04-15  2:55 Keith David Bershatsky
  2019-04-15  3:44 ` Alex Gramiak
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-15  2:55 UTC (permalink / raw)
  To: Alex Gramiak; +Cc: eliz, dancol, emacs-devel

Thank you, Alex ... I like the idea of not using a Lisp_Object to store the various window caches of fake cursors, as this would entirely eliminate the garbage collection issues.  Inasmuch as this would be my first usage of an array in the world of programming, some guidance regarding the best way to initialize the array (window cache) would be greatly appreciated.

The foreground and background colors of each fake cursor are sets of three doubles (red/green/blue).  They do not necessarily need to be stored as a set of three, and could be broken up into separate columns within the array if that is more prudent.  active_p is a boolean type, but could be expressed as an int instead.

I saw a few online examples that initialize an array as int _or_ double, but not both simultaneously.  So, I thought to myself that they could all be doubles if need be ....

I saw three types of arrays:  an array matching an exact size known in advance; an array that is defined with a specified MAX_ROW / MAX_COL (some rows/columns may never be used); and, an array using dynamic allocation with malloc, realloc and free.

Each window cache should be able to handle up to 250 fake cursors.  The present design uses up to four (4) different window caches for each window where fake cursors are active.  Each fake cursor would need entries in the cache of either 14 elements or 18 elements, depending upon whether the foreground/background colors are grouped or broken up.

How do you recommend that the array for each window cache be initialized?

1.  int x
2.  int fx
3.  int y
4.  int fy
5.  int hpos
6.  int vpos
7.  int wd
8.  int h
9.  int cursor_type
10. int cursor_width
11. double foreground_red
12. double foreground_green
13. double foreground_blue
14. double background_red
15. double background_green
16. double background_blue
17. bool active_p
18. int glyph_flavor

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-14-2019 16:31:16] <14 Apr 2019 17:31:16 -0600>
> From: Alex Gramiak <agrambot@gmail.com>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: eliz@gnu.org, dancol@dancol.org, emacs-devel@gnu.org
> Subject: Re: NS port: How to debug excessive garbage collection?
> 
> * * *
> 
> Looking at your actual implementation though, I don't see a reason for your cache to be a Lisp_Object at all; am I missing something?  It seems that you could just use a C array of length 14.  Not making any cons cells at all is the best approach if you can do so.
> 
> * * *



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
  2019-04-15  2:55 Keith David Bershatsky
@ 2019-04-15  3:44 ` Alex Gramiak
  0 siblings, 0 replies; 20+ messages in thread
From: Alex Gramiak @ 2019-04-15  3:44 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: eliz, dancol, emacs-devel

Keith David Bershatsky <esq@lawlist.com> writes:

> Thank you, Alex ... I like the idea of not using a Lisp_Object to store the
> various window caches of fake cursors, as this would entirely eliminate the
> garbage collection issues. Inasmuch as this would be my first usage of an array
> in the world of programming, some guidance regarding the best way to initialize
> the array (window cache) would be greatly appreciated.

Sorry, I shouldn't have said to use a C array in this case. A struct
would be better in this case due to the different types involved.

> I saw three types of arrays: an array matching an exact size known in advance;
> an array that is defined with a specified MAX_ROW / MAX_COL (some rows/columns
> may never be used); and, an array using dynamic allocation with malloc, realloc
> and free.

Right.

> Each window cache should be able to handle up to 250 fake cursors.

Is 250 an arbitrary limit?

> How do you recommend that the array for each window cache be initialized?
>
> 1.  int x
> 2.  int fx
> 3.  int y
> 4.  int fy
> 5.  int hpos
> 6.  int vpos
> 7.  int wd
> 8.  int h
> 9.  int cursor_type
> 10. int cursor_width
> 11. double foreground_red
> 12. double foreground_green
> 13. double foreground_blue
> 14. double background_red
> 15. double background_green
> 16. double background_blue
> 17. bool active_p
> 18. int glyph_flavor

To change it to a struct (with a helper struct):

  struct RGB
  {
    double red;
    double green;
    double blue;
  };

  struct multiple_cursor_cache
  {
    int x;
    int fx;
    int y;
    int fy;
    int hpos;
    int vpos;
    int wd;
    int h;
    int cursor_type;
    int cursor_width;
    struct RGB foreground;
    struct RGB background;
    bool active_p;
    int glyph_flavor;
  };

Then you have:

  struct multiple_cursor_cache mc_cache;

and to set elements:

  w->mc_cache.x = x;
  w->mc_cache.foreground.red = red;

To set all elements to zero, one does:

  memset (&variable, 0, sizeof (variable));




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
@ 2019-04-15  5:19 Keith David Bershatsky
  0 siblings, 0 replies; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-15  5:19 UTC (permalink / raw)
  To: Alex Gramiak; +Cc: eliz, dancol, emacs-devel

Thank you, Alex, for the example structs and also for the zeroing out example -- greatly appreciated!

Yes, 250 fake cursors is an arbitrary upper limit based upon my own imagination of how many a user might reasonably use in any given window.  The crosshairs feature uses approximately that amount per window.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-14-2019 20:44:41] <14 Apr 2019 21:44:41 -0600>
> From: Alex Gramiak <agrambot@gmail.com>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: eliz@gnu.org, dancol@dancol.org, emacs-devel@gnu.org
> Subject: Re: NS port: How to debug excessive garbage collection?
> 
> * * *
> 
> > Each window cache should be able to handle up to 250 fake cursors.
> 
> Is 250 an arbitrary limit?
> 
> > How do you recommend that the array for each window cache be initialized?
> >
> > 1.  int x
> > 2.  int fx
> > 3.  int y
> > 4.  int fy
> > 5.  int hpos
> > 6.  int vpos
> > 7.  int wd
> > 8.  int h
> > 9.  int cursor_type
> > 10. int cursor_width
> > 11. double foreground_red
> > 12. double foreground_green
> > 13. double foreground_blue
> > 14. double background_red
> > 15. double background_green
> > 16. double background_blue
> > 17. bool active_p
> > 18. int glyph_flavor
> 
> To change it to a struct (with a helper struct):
> 
>   struct RGB
>   {
>     double red;
>     double green;
>     double blue;
>   };
> 
>   struct multiple_cursor_cache
>   {
>     int x;
>     int fx;
>     int y;
>     int fy;
>     int hpos;
>     int vpos;
>     int wd;
>     int h;
>     int cursor_type;
>     int cursor_width;
>     struct RGB foreground;
>     struct RGB background;
>     bool active_p;
>     int glyph_flavor;
>   };
> 
> Then you have:
> 
>   struct multiple_cursor_cache mc_cache;
> 
> and to set elements:
> 
>   w->mc_cache.x = x;
>   w->mc_cache.foreground.red = red;
> 
> To set all elements to zero, one does:
> 
>   memset (&variable, 0, sizeof (variable));



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
@ 2019-04-16  2:57 Keith David Bershatsky
  2019-04-16  5:26 ` Alex Gramiak
  0 siblings, 1 reply; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-16  2:57 UTC (permalink / raw)
  To: Alex Gramiak; +Cc: eliz, dancol, emacs-devel

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

I am working on implementing the suggestion of Alex to use structures to store the data for each fake cursor on a per window basis, instead of the prior method that created a Lisp_Object of cons cells (causing the excessive garbage collection issue).

It seemed to me that the implementation of creating the vector to store glyph rows looked somewhat similar to what we want to do here.  I attempted to borrow a portion of that vector glyph row implementation in the attached working example that applies to the master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).

I had hoped to keep the cache (w->mc_elts) alive for the duration that the fake cursors feature is active in a visible window.  However, the cache (w->mc_elts) disappears and becomes NULL.  In the attached example, whenever it is time to increase the size of the dynamic vector, I set the value of w->mc_elts[99].x to 99.  The value of -1 is used in the stderr printout whenever w->mc_elts is NULL.

How can I make the cache (w->mc_elts) persistent without automatically becoming NULL?

In the attached example, I launch Emacs after applying the patch and evaluate:  (setq crosshairs t)

w->mc_nelts (100) | w->mc_elts_allocated (100) | w->mc_elts[99].x (99)
w->mc_nelts (200) | w->mc_elts_allocated (200) | w->mc_elts[99].x (99)
w->mc_nelts (300) | w->mc_elts_allocated (300) | w->mc_elts[99].x (99)
w->mc_nelts (400) | w->mc_elts_allocated (450) | w->mc_elts[99].x (99)
w->mc_nelts (500) | w->mc_elts_allocated (675) | w->mc_elts[99].x (99)
w->mc_nelts (600) | w->mc_elts_allocated (675) | w->mc_elts[99].x (-1)
w->mc_nelts (700) | w->mc_elts_allocated (1012) | w->mc_elts[99].x (99)
w->mc_nelts (800) | w->mc_elts_allocated (1012) | w->mc_elts[99].x (-1)
w->mc_nelts (900) | w->mc_elts_allocated (1012) | w->mc_elts[99].x (-1)
w->mc_nelts (1000) | w->mc_elts_allocated (1012) | w->mc_elts[99].x (-1)
w->mc_nelts (1100) | w->mc_elts_allocated (1518) | w->mc_elts[99].x (99)
w->mc_nelts (1200) | w->mc_elts_allocated (1518) | w->mc_elts[99].x (-1)
w->mc_nelts (1300) | w->mc_elts_allocated (1518) | w->mc_elts[99].x (-1)
w->mc_nelts (1400) | w->mc_elts_allocated (1518) | w->mc_elts[99].x (-1)
w->mc_nelts (1500) | w->mc_elts_allocated (1518) | w->mc_elts[99].x (-1)
w->mc_nelts (1600) | w->mc_elts_allocated (2277) | w->mc_elts[99].x (99)


[-- Attachment #2: 2019_04_15__19_36_14_173.diff --]
[-- Type: application/diff, Size: 8668 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
  2019-04-16  2:57 Keith David Bershatsky
@ 2019-04-16  5:26 ` Alex Gramiak
  0 siblings, 0 replies; 20+ messages in thread
From: Alex Gramiak @ 2019-04-16  5:26 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: eliz, emacs-devel

Keith David Bershatsky <esq@lawlist.com> writes:

> I am working on implementing the suggestion of Alex to use structures to store
> the data for each fake cursor on a per window basis, instead of the prior method
> that created a Lisp_Object of cons cells (causing the excessive garbage
> collection issue).
>
> It seemed to me that the implementation of creating the vector to store glyph
> rows looked somewhat similar to what we want to do here. I attempted to borrow a
> portion of that vector glyph row implementation in the attached working example
> that applies to the master branch as of 04/08/2019
> (a038df77de7b1aa2d73a6478493b8838b59e4982).
>
> I had hoped to keep the cache (w->mc_elts) alive for the duration that the fake
> cursors feature is active in a visible window. However, the cache (w->mc_elts)
> disappears and becomes NULL. In the attached example, whenever it is time to
> increase the size of the dynamic vector, I set the value of w->mc_elts[99].x to
> 99. The value of -1 is used in the stderr printout whenever w->mc_elts is NULL.

The problem here is that since you changed the single struct cache to an
array of caches, you need to also change the memset call to match it.
Right now you are setting the pointer variable to 0/NULL. It should now
be:

  memset (w->mc_elts, 0, w->mc_nelts * (sizeof *w->mc_elts));

Which sets the memory block of mc_nelts cache structs pointed to
by mc_elts to 0.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: NS port: How to debug excessive garbage collection?
@ 2019-04-16  5:51 Keith David Bershatsky
  0 siblings, 0 replies; 20+ messages in thread
From: Keith David Bershatsky @ 2019-04-16  5:51 UTC (permalink / raw)
  To: Alex Gramiak; +Cc: eliz, emacs-devel

Thank you, Alex, for pinpointing my error in the call to memset.

Greatly appreciated!

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-15-2019 22:26:05] <15 Apr 2019 23:26:05 -0600>
> From: Alex Gramiak <agrambot@gmail.com>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: eliz@gnu.org, emacs-devel@gnu.org
> Subject: Re: NS port: How to debug excessive garbage collection?
> 
> * * *
> 
> The problem here is that since you changed the single struct cache to an
> array of caches, you need to also change the memset call to match it.
> Right now you are setting the pointer variable to 0/NULL. It should now
> be:
> 
>   memset (w->mc_elts, 0, w->mc_nelts * (sizeof *w->mc_elts));
> 
> Which sets the memory block of mc_nelts cache structs pointed to
> by mc_elts to 0.



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-04-16  5:51 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-14 19:46 NS port: How to debug excessive garbage collection? Keith David Bershatsky
2019-04-14 23:31 ` Alex Gramiak
  -- strict thread matches above, loose matches on Subject: below --
2019-04-16  5:51 Keith David Bershatsky
2019-04-16  2:57 Keith David Bershatsky
2019-04-16  5:26 ` Alex Gramiak
2019-04-15  5:19 Keith David Bershatsky
2019-04-15  2:55 Keith David Bershatsky
2019-04-15  3:44 ` Alex Gramiak
2019-04-14  7:41 Keith David Bershatsky
2019-04-13 18:07 Keith David Bershatsky
2019-04-13 21:41 ` Alex Gramiak
2019-04-14  3:47 ` Daniel Colascione
2019-04-13 16:31 Keith David Bershatsky
2019-04-13 17:02 ` Alex Gramiak
2019-04-13  5:55 Keith David Bershatsky
2019-04-13  6:48 ` Eli Zaretskii
2019-04-11 23:04 Keith David Bershatsky
2019-04-12  9:30 ` Eli Zaretskii
2019-04-11  3:27 Keith David Bershatsky
2019-04-11 14:14 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).