Universal functions to manage multiple window caches.

Universal functions to manage multiple window caches.

[Keith David Bershatsky]

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

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

window.h defines four different caches of fake cursors, with the only difference between them being their names:

  struct multiple_cursors_cache *temp_elts;
  ptrdiff_t temp_elts_allocated;
  int temp_nelts;

  struct multiple_cursors_cache *mc_elts;
  ptrdiff_t mc_elts_allocated;
  int mc_nelts;

  struct multiple_cursors_cache *ch_elts;
  ptrdiff_t ch_elts_allocated;
  int ch_nelts;

  struct multiple_cursors_cache *fc_elts;
  ptrdiff_t fc_elts_allocated;
  int fc_nelts;

I am trying to create two functions (reset and populate) to manage all four caches, but am having troubling figuring out how to substitute the names of the applicable caches depending upon which cache is needed.  I tried several variations of the following approach, but ran into problems either compiling or Emacs crashed when the crosshairs feature was activated.

    struct multiple_cursors_cache **foo_elts = w->ch_elts;
    ptrdiff_t *foo_elts_allocated = &w->ch_elts_allocated;
    int *foo_nelts = &w->ch_nelts;

Q:  What is the best way to create (1) a reset function and (2) a populate function that can handle all four caches?

void
mc_reset_cache (struct window *w, ...)
{
  if (BUFFERP (w->contents) && NILP (BVAR (XBUFFER (w->contents), crosshairs))
      && foo_nelts > 1)
    {
      /* Decrease the size of the array to a bare minimum. */
      xnrealloc (foo_elts, 1, sizeof *foo_elts);
      foo_nelts = 0;
      foo_elts_allocated = 1;
    }
    else if (BUFFERP (w->contents) && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
      {
        /* Set all _used_ elements of the array to zero.  elts_allocated remain
        the same. */
        memset (foo_elts, 0, foo_nelts * (sizeof *foo_elts));
        foo_nelts = 0;
      }
}

void
mc_populate_cache (struct window *w, ...)
{
  if (BUFFERP (w->contents)
      && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
    {
      /* Increase the size of the array. */
      if (foo_elts_allocated < foo_nelts
          && foo_nelts < max_elts)
        {
          int old_alloc = foo_elts_allocated;
          int new_elts = foo_nelts - foo_elts_allocated;
          foo_elts = xpalloc (foo_elts, &foo_elts_allocated,
                                  new_elts, INT_MAX, sizeof *foo_elts);
          memset (foo_elts + old_alloc, 0,
                   (foo_elts_allocated - old_alloc) * sizeof *foo_elts);
        }
      /* Below this comment is where the applicable cache will be populated. */
    }
}

Attached is a draft patch that applies to the master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).


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

Re: Universal functions to manage multiple window caches.

[Keith David Bershatsky]

The following two functions for reset / populate are working; however, several sections of code are essentially repeated (for lack of figuring out a more concise approach).  Although not critical to the overall design, a more efficient way to write these up would be greatly appreciated.

void
mc_reset_cache (struct window *w, enum type_of_cache cache_type)
{
  switch (cache_type)
    {
      case NO_CACHE:
        {
          return;
        }
      case MC_TEMP_CACHE:
        {
          if (w->temp_nelts > 1)
            {
              /* Decrease the size of the array to a bare minimum. */
              xnrealloc (w->temp_elts, 1, sizeof *w->temp_elts);
              w->temp_nelts = 0;
              w->temp_elts_allocated = 1;
            }
            else
              {
                /* Set all _used_ elements of the array to zero.  elts_allocated remain
                the same. */
                memset (w->temp_elts, 0, w->temp_nelts * (sizeof *w->temp_elts));
                w->temp_nelts = 0;
              }
          break;
        }
      case MC_CACHE:
        {
          if (BUFFERP (w->contents) && NILP (BVAR (XBUFFER (w->contents), mc_conf))
              && w->mc_nelts > 1)
            {
              /* Decrease the size of the array to a bare minimum. */
              xnrealloc (w->mc_elts, 1, sizeof *w->mc_elts);
              w->mc_nelts = 0;
              w->mc_elts_allocated = 1;
            }
            else if (BUFFERP (w->contents) && !NILP (BVAR (XBUFFER (w->contents), mc_conf)))
              {
                /* Set all _used_ elements of the array to zero.  elts_allocated remain
                the same. */
                memset (w->mc_elts, 0, w->mc_nelts * (sizeof *w->mc_elts));
                w->mc_nelts = 0;
              }
          break;
        }
      case CH_CACHE:
        {
          if (BUFFERP (w->contents) && NILP (BVAR (XBUFFER (w->contents), crosshairs))
              && w->ch_nelts > 1)
            {
              /* Decrease the size of the array to a bare minimum. */
              xnrealloc (w->ch_elts, 1, sizeof *w->ch_elts);
              w->ch_nelts = 0;
              w->ch_elts_allocated = 1;
            }
            else if (BUFFERP (w->contents) && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
              {
                /* Set all _used_ elements of the array to zero.  elts_allocated remain
                the same. */
                memset (w->ch_elts, 0, w->ch_nelts * (sizeof *w->ch_elts));
                w->ch_nelts = 0;
              }
          break;
        }
      case FC_CACHE:
        {
          if (BUFFERP (w->contents) && NILP (BVAR (XBUFFER (w->contents), fc_visible))
              && w->fc_nelts > 1)
            {
              /* Decrease the size of the array to a bare minimum. */
              xnrealloc (w->fc_elts, 1, sizeof *w->fc_elts);
              w->fc_nelts = 0;
              w->fc_elts_allocated = 1;
            }
            else if (BUFFERP (w->contents) && !NILP (BVAR (XBUFFER (w->contents), fc_visible)))
              {
                /* Set all _used_ elements of the array to zero.  elts_allocated remain
                the same. */
                memset (w->fc_elts, 0, w->fc_nelts * (sizeof *w->fc_elts));
                w->fc_nelts = 0;
              }
          break;
        }
    }
}

static void
mc_helper (struct window *w, struct glyph_matrix *matrix, struct glyph_row *row,
           struct glyph *glyph, int x, int fx, int y, int fy, int hpos, int vpos,
           int wd, int h, enum text_cursor_kinds cursor_type, int cursor_width,
           struct RGB foreground, struct RGB background, bool active_p,
           enum mc_flavor glyph_flavor, bool draw_p, enum type_of_cache cache_type)
{
  . . .
  struct multiple_cursors_cache *foo_elts;
  ptrdiff_t *foo_elts_allocated;
  int *foo_nelts;
  switch (cache_type)
    {
      case NO_CACHE:
        {
          return;
        }
      case MC_TEMP_CACHE:
        {
          ++w->temp_nelts;
          if (w->temp_elts_allocated < w->temp_nelts)
            {
              int old_alloc = w->temp_elts_allocated;
              int new_elts = w->temp_nelts - w->temp_elts_allocated;
              w->temp_elts = xpalloc (w->temp_elts, &w->temp_elts_allocated,
                                      new_elts, INT_MAX, sizeof *w->temp_elts);
              memset (w->temp_elts + old_alloc, 0,
                       (w->temp_elts_allocated - old_alloc) * sizeof *w->temp_elts);
            }
          foo_elts = w->temp_elts;
          foo_elts_allocated = &w->temp_elts_allocated;
          foo_nelts = &w->temp_nelts;
          break;
        }
      case MC_CACHE:
        {
          ++w->mc_nelts;
          if (w->mc_elts_allocated < w->mc_nelts)
            {
              int old_alloc = w->mc_elts_allocated;
              int new_elts = w->mc_nelts - w->mc_elts_allocated;
              w->mc_elts = xpalloc (w->mc_elts, &w->mc_elts_allocated,
                                      new_elts, INT_MAX, sizeof *w->mc_elts);
              memset (w->mc_elts + old_alloc, 0,
                       (w->mc_elts_allocated - old_alloc) * sizeof *w->mc_elts);
            }
          foo_elts = w->mc_elts;
          foo_elts_allocated = &w->mc_elts_allocated;
          foo_nelts = &w->mc_nelts;
          break;
        }
      case CH_CACHE:
        {
          ++w->ch_nelts;
          if (w->ch_elts_allocated < w->ch_nelts)
            {
              int old_alloc = w->ch_elts_allocated;
              int new_elts = w->ch_nelts - w->ch_elts_allocated;
              w->ch_elts = xpalloc (w->ch_elts, &w->ch_elts_allocated,
                                      new_elts, INT_MAX, sizeof *w->ch_elts);
              memset (w->ch_elts + old_alloc, 0,
                       (w->ch_elts_allocated - old_alloc) * sizeof *w->ch_elts);
            }
          foo_elts = w->ch_elts;
          foo_elts_allocated = &w->ch_elts_allocated;
          foo_nelts = &w->ch_nelts;
          break;
        }
      case FC_CACHE:
        {
          ++w->fc_nelts;
          if (w->fc_elts_allocated < w->fc_nelts)
            {
              int old_alloc = w->fc_elts_allocated;
              int new_elts = w->fc_nelts - w->fc_elts_allocated;
              w->fc_elts = xpalloc (w->fc_elts, &w->fc_elts_allocated,
                                      new_elts, INT_MAX, sizeof *w->fc_elts);
              memset (w->fc_elts + old_alloc, 0,
                       (w->fc_elts_allocated - old_alloc) * sizeof *w->fc_elts);
            }
          foo_elts = w->fc_elts;
          foo_elts_allocated = &w->fc_elts_allocated;
          foo_nelts = &w->fc_nelts;
          break;
        }
    }
  int nth = *foo_nelts - 1;
  foo_elts[nth].x = x;
  foo_elts[nth].fx = fx;
  foo_elts[nth].y = y;
  foo_elts[nth].fy = fy;
  foo_elts[nth].hpos = hpos;
  foo_elts[nth].vpos = vpos;
  foo_elts[nth].wd = wd;
  foo_elts[nth].h = h;
  foo_elts[nth].cursor_type = cursor_type;
  foo_elts[nth].cursor_width = cursor_width;
  foo_elts[nth].foreground.red = foreground.red;
  foo_elts[nth].foreground.green = foreground.green;
  foo_elts[nth].foreground.blue = foreground.blue;
  foo_elts[nth].background.red = background.red;
  foo_elts[nth].background.green = background.green;
  foo_elts[nth].background.blue = background.blue;
  foo_elts[nth].active_p = active_p;
  foo_elts[nth].glyph_flavor = glyph_flavor;
  foo_elts[nth].enabled_p = true;
}

Re: Universal functions to manage multiple window caches.

[Alex Gramiak]

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

> I am working on feature requests 22873 (multiple fake cursors) and 17684 (crosshairs that track the cursor position).
>
> window.h defines four different caches of fake cursors, with the only difference between them being their names:
>
>   struct multiple_cursors_cache *temp_elts;
>   ptrdiff_t temp_elts_allocated;
>   int temp_nelts;
>
>   struct multiple_cursors_cache *mc_elts;
>   ptrdiff_t mc_elts_allocated;
>   int mc_nelts;
>
>   struct multiple_cursors_cache *ch_elts;
>   ptrdiff_t ch_elts_allocated;
>   int ch_nelts;
>
>   struct multiple_cursors_cache *fc_elts;
>   ptrdiff_t fc_elts_allocated;
>   int fc_nelts;

Each of the four could just be structs themselves. Something like:

  struct multiple_cursor_cache
  {
    ptrdiff_t allocated;
    ptrdiff_t used;
    struct items
    {
      int x;
      int fx;
      int y;
      int fy;
      int hpos;
      int vpos;
      int wd;
      int h;
      int cursor_type;
      int cursor_width;
      struct RGB
      {
        double red;
        double green;
        double blue;
      } foreground, background;
      bool active_p;
      int glyph_flavor;
      bool enabled_p;
    } *caches;
  };

If you need to differentiate them in a helper procedure, you can add an
enum element to the outermost struct.

P.S. Why do you need to memset the used portion of the caches on every
window update? I would think that just setting the used/*_nelts count
would be sufficient as long as you make sure not to go past that and
access garbage data.

Re: Universal functions to manage multiple window caches.

[Keith David Bershatsky]

Thank you, Alex, for the suggestions to better organize the new cache design.

I did not stop to think that calling memset on every window update potentially carried with it a cost to performance, and the OCD side of my brain wanted to continuously tidy things up.  I will just need to keep reminding myself that its okay to leave garbage data where it is so long as I steer clear of it by using a counter.  :)

I created a minimal working example based upon my interpretation of how to use your sample struct.  Was it your intention that each main cache would be defined as follows (and the MWE being nth 0 below):

1.  TEMP flavor:  w->mc_elts[0]

2.  MC flavor:  w->mc_elts[1]

3.  CH flavor:  w->mc_elts[2]

4.  FC flavor:  w->mc_elts[3]

if (BUFFERP (w->contents)
    && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
  {
    /* Increase the size of the array. */
    if (w->mc_elts_allocated == 0)
      {
        ++w->mc_nelts;
        int old_alloc = w->mc_elts_allocated;
        int new_elts = w->mc_nelts - w->mc_elts_allocated;
        w->mc_elts = xpalloc (w->mc_elts, &w->mc_elts_allocated,
                                new_elts, INT_MAX, sizeof *w->mc_elts);
        memset (w->mc_elts + old_alloc, 0,
                 (w->mc_elts_allocated - old_alloc) * sizeof *w->mc_elts);
      }
    int max_elts = 25000;
    w->mc_elts->used += (w->mc_elts->used < max_elts)
                   ? 100
                   : 0;
    /* Increase the size of the array. */
    if (w->mc_elts->allocated < w->mc_elts->used
        && w->mc_elts->used < max_elts)
      {
        int old_alloc = w->mc_elts->allocated;
        int new_elts = w->mc_elts->used - w->mc_elts->allocated;
        w->mc_elts->caches = xpalloc (w->mc_elts->caches, &w->mc_elts->allocated,
                                      new_elts, INT_MAX, sizeof *w->mc_elts->caches);
        memset (w->mc_elts->caches + old_alloc, 0,
                 (w->mc_elts->allocated - old_alloc) * sizeof *w->mc_elts->caches);
        for (int elt = 0; elt < w->mc_elts->used; ++elt)
          {
            w->mc_elts[0].caches[elt].x = elt;
            double red = elt; 
            w->mc_elts[0].caches[elt].foreground.red = red;
            w->mc_elts[0].caches[elt].enabled_p = true;
          }
      }
    fprintf (stderr, "w->mc_elts->used (%d) | w->mc_elts->allocated (%d) | w->mc_elts[0].caches[99].x (%d)\n",
                      w->mc_elts->used, w->mc_elts->allocated, w->mc_elts[0].caches[99].x);
  }




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

> Date: [04-18-2019 14:02:15] <18 Apr 2019 15:02:15 -0600>
> From: Alex Gramiak <agrambot@gmail.com>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Subject: Re: Universal functions to manage multiple window caches.
> 
> Keith David Bershatsky <esq@lawlist.com> writes:
> 
> > I am working on feature requests 22873 (multiple fake cursors) and 17684 (crosshairs that track the cursor position).
> >
> > window.h defines four different caches of fake cursors, with the only difference between them being their names:
> >
> >   struct multiple_cursors_cache *temp_elts;
> >   ptrdiff_t temp_elts_allocated;
> >   int temp_nelts;
> >
> >   struct multiple_cursors_cache *mc_elts;
> >   ptrdiff_t mc_elts_allocated;
> >   int mc_nelts;
> >
> >   struct multiple_cursors_cache *ch_elts;
> >   ptrdiff_t ch_elts_allocated;
> >   int ch_nelts;
> >
> >   struct multiple_cursors_cache *fc_elts;
> >   ptrdiff_t fc_elts_allocated;
> >   int fc_nelts;
> 
> Each of the four could just be structs themselves. Something like:
> 
>   struct multiple_cursor_cache
>   {
>     ptrdiff_t allocated;
>     ptrdiff_t used;
>     struct items
>     {
>       int x;
>       int fx;
>       int y;
>       int fy;
>       int hpos;
>       int vpos;
>       int wd;
>       int h;
>       int cursor_type;
>       int cursor_width;
>       struct RGB
>       {
>         double red;
>         double green;
>         double blue;
>       } foreground, background;
>       bool active_p;
>       int glyph_flavor;
>       bool enabled_p;
>     } *caches;
>   };
> 
> If you need to differentiate them in a helper procedure, you can add an
> enum element to the outermost struct.
> 
> P.S. Why do you need to memset the used portion of the caches on every
> window update? I would think that just setting the used/*_nelts count
> would be sufficient as long as you make sure not to go past that and
> access garbage data.

Re: Universal functions to manage multiple window caches.

[Alex Gramiak]

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

> I created a minimal working example based upon my interpretation of how to use your sample struct.  Was it your intention that each main cache would be defined as follows (and the MWE being nth 0 below):
>
> 1.  TEMP flavor:  w->mc_elts[0]
>
> 2.  MC flavor:  w->mc_elts[1]
>
> 3.  CH flavor:  w->mc_elts[2]
>
> 4.  FC flavor:  w->mc_elts[3]

No, my intention was for each flavor to be a different element in the
window struct. It's fine either way, but different elements could have
more descriptive names than an index allows.

> if (BUFFERP (w->contents)
>     && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
>   {
>     /* Increase the size of the array. */
>     if (w->mc_elts_allocated == 0)
>       {
>         ++w->mc_nelts;
>         int old_alloc = w->mc_elts_allocated;
>         int new_elts = w->mc_nelts - w->mc_elts_allocated;
>         w->mc_elts = xpalloc (w->mc_elts, &w->mc_elts_allocated,
>                                 new_elts, INT_MAX, sizeof *w->mc_elts);
>         memset (w->mc_elts + old_alloc, 0,
>                  (w->mc_elts_allocated - old_alloc) * sizeof *w->mc_elts);
>       }

Did you forget to update this part?

Re: Universal functions to manage multiple window caches.

[Keith David Bershatsky]

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

Thank you, Alex, for helping me to better understand how best to organize the cache structures.

Attached is another attempt by me at implementing your suggestions (as I understand them).  [The need for a temporary cache has been eliminated in the most recent working draft of multiple fake cursors, so there are only three flavors of caches now:  mc, ch and fc.]

Does the attached patch accurately reflect your suggested approach?


[-- Attachment #2: cache_rewrite_002.diff --]
[-- Type: application/diff, Size: 10574 bytes --]

Re: Universal functions to manage multiple window caches.

[Alex Gramiak]

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

> Thank you, Alex, for helping me to better understand how best to organize the cache structures.
>
> Attached is another attempt by me at implementing your suggestions (as I
> understand them). [The need for a temporary cache has been eliminated in the
> most recent working draft of multiple fake cursors, so there are only three
> flavors of caches now: mc, ch and fc.]
>
> Does the attached patch accurately reflect your suggested approach?

Looks about what I expected, yeah (you could also have the enum be an
element of each cache type instead of it being global, but it's up to
preference). Though is it your intention to be using w->mc_elts in the
CH_CACHE case instead of w->ch_elts?

Re: Universal functions to manage multiple window caches.

[Keith David Bershatsky]

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

Thank you, Alex, for the suggested edits.  I moved the enum for cache types into the main struct for multiple_cursor_cache, and also moved the sections that apply to w->mc_elts into the MC_CACHE case instead of the CH_CACHE case.  Attached is the updated diff for the new cache management design.

struct multiple_cursor_cache
{
  ptrdiff_t allocated;
  ptrdiff_t used;
  struct items
  {
    int x;
    int fx;
    int y;
    int fy;
    int hpos;
    int vpos;
    int wd;
    int h;
    int cursor_type;
    int cursor_width;
    struct RGB
    {
      double red;
      double green;
      double blue;
    } foreground, background;
    bool active_p;
    int glyph_flavor;
    bool enabled_p;
  } *caches;
  enum type_of_cache
  {
    NO_CACHE,
    MC_CACHE,
    CH_CACHE,
    FC_CACHE
  } cache_type;
};

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

> Date: [04-20-2019 10:17:01] <20 Apr 2019 11:17:01 -0600>
> From: Alex Gramiak <agrambot@gmail.com>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: emacs-devel@gnu.org
> Subject: Re: Universal functions to manage multiple window caches.
> 
>* * *
> 
> Looks about what I expected, yeah (you could also have the enum be an
> element of each cache type instead of it being global, but it's up to
> preference). Though is it your intention to be using w->mc_elts in the
> CH_CACHE case instead of w->ch_elts?


[-- Attachment #2: cache_rewrite_003.diff --]
[-- Type: application/diff, Size: 10595 bytes --]