Re: Three small patches

[Jan Nieuwenhuizen]

Gabriel Wicki writes:

Hello Gabriel,

> I interpret the recent increase in messages (and their contents) on this
> list as a sign that development is rather accelerating than slowing
> down, which is nice!

Yes!  I finally got round to release v0.24!

> So i shall use that opportunity to report from my end.  Unfortunately
> i had to renice my MES tasks to a lower priority; but i'm still on it.
> Slowly, but steadily :)

Great!

> I currently am on a medium- to large detour from my original goal
> (porting MEScc to RISC-V), by diving into the internals and inner
> workings of this marvelous project, but i'm still focussed on my initial
> goal.  Thus my personal agenda has been altered: clean up, simplify
> and comment the code-base, document the more important parts of the
> internals (inspired by Guile's Internal documentation, though probably
> less exhaustive), generalize the existing infrastructure (i.e.
> MEScc's assembly-module) to more naturally support non i386
> architectures and finally: implement the RISC-V MEScc assembly
> module.

I'm working on wip-riscv (laanwj's patch set), and wondering what to do
best with the open/openat, x/xat, ... thing.  I suppose you have seen
that?  How is your work related to this?

If you intend to make changes to the core, you may want to keep an eye
on the wip-guile-module/wip-mescc-module work that should land on master
right after wip-arm and wip-riscv.

> I've attached three tiny patches.  There are more to come!

Thanks.  The patches lack a GNU style ChangeLog message, see

    https://www.gnu.org/software/mes/manual/html_node/Submitting-Patches.html

I have added them for you.

> I wish you all the best,

> From: Gabriel Wicki <gabriel@erlikon.ch>
> Subject: [PATCH] gc: remove news_bytes
>
> it is identical to cell_bytes; the only reference to it was replaced

Changed to:

    Subject: [PATCH 1/3] core: Remove code duplication.

    * src/gc.c (news_bytes): Remove copy of cell_bytes.
    (gc_copy): Use cell_bytes instead.
    * include/mes/mes.h (news_bytes): Remove prototype.

> diff --git a/include/mes/mes.h b/include/mes/mes.h
[..]
> index 9ef6491e..337f03aa 100644
> --- a/include/mes/mes.h
> +++ b/include/mes/mes.h
> @@ -153,7 +153,6 @@ struct scm *vector_ref_ (struct scm *x, long i);
>  struct scm *vector_set_x_ (struct scm *x, long i, struct scm *e);
>  FUNCTION builtin_function (struct scm *builtin);
>  char *cell_bytes (struct scm *x);
> -char *news_bytes (struct scm *x);
>  int peekchar ();
>  int readchar ();
>  int unreadchar ();
> diff --git a/src/gc.c b/src/gc.c
> index 62643bbc..4a1df7cb 100644
> --- a/src/gc.c
> +++ b/src/gc.c
> @@ -40,13 +40,6 @@ cell_bytes (struct scm *x)
>    return p + (2 * sizeof (long));
>  }
>  
> -char *
> -news_bytes (struct scm *x)
> -{
> -  char *p = cast_voidp_to_charp (x);
> -  return p + (2 * sizeof (long));
> -}
> -
>  #define U10 10U
>  // CONSTANT U10 10
>  #define U100 100U
> @@ -504,7 +497,7 @@ gc_copy (struct scm *old)               /*:((internal)) */
>    else if (new->type == TBYTES)
>      {
>        char const *src = cell_bytes (old);
> -      char *dest = news_bytes (new);
> +      char *dest = cell_bytes (new);
>        size_t length = new->length;
>        memcpy (dest, src, length);
>        g_free = g_free + ((bytes_cells (length) - 1) * M2_CELL_SIZE);
> -- 

Good catch, LGTM!

(Note that mes 0.23 was using SICP-like number-based cells.  Mes v0.24
switched to pointer-based cells mainly because M2-Planet 1.7.x did not
support preprocessor macros).

> From 1290ff4f8d2e244afae8188bfc0fd0d94bbc259e Mon Sep 17 00:00:00 2001
> From: Gabriel Wicki <gabriel@erlikon.ch>
> Date: Tue, 10 May 2022 22:17:20 +0200
> Subject: [PATCH] gc: remove copy_news
>
> it is identical to copy_cell.  two references were replaced

Likewise,

    Subject: [PATCH 2/3] core: Remove code duplication.

    * src/gc.c (copy_news): Remove copy of copy_cell.
    (gc_copy): Use copy_cell instead.

> ---
>  src/gc.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/src/gc.c b/src/gc.c
> index 4a1df7cb..e0b16aa0 100644
> --- a/src/gc.c
> +++ b/src/gc.c
> @@ -212,14 +212,6 @@ copy_cell (struct scm *to, struct scm *from)
>    to->cdr = from->cdr;
>  }
>  
> -void
> -copy_news (struct scm *to, struct scm *from)
> -{
> -  to->type = from->type;
> -  to->car = from->car;
> -  to->cdr = from->cdr;
> -}
> -
>  void
>  copy_stack (long index, struct scm *from)
>  {
> @@ -483,14 +475,14 @@ gc_copy (struct scm *old)               /*:((internal)) 
> */
>      return old->car;
>    struct scm *new = g_free;
>    g_free = g_free + M2_CELL_SIZE;
> -  copy_news (new, old);
> +  copy_cell (new, old);
>    if (new->type == TSTRUCT || new->type == TVECTOR)
>      {
>        new->vector = g_free;
>        long i;
>        for (i = 0; i < old->length; i = i + 1)
>          {
> -          copy_news (g_free, cell_ref (old->vector, i));
> +          copy_cell (g_free, cell_ref (old->vector, i));
>            g_free = g_free + M2_CELL_SIZE;
>          }
>      }
> -- 

LGTM!

> From b2252af7781e06f7303410befed56311615ec9f5 Mon Sep 17 00:00:00 2001
> From: Gabriel Wicki <gabriel@erlikon.ch>
> Date: Tue, 10 May 2022 22:23:40 +0200
> Subject: [PATCH] gc: simplify math expressions

Likewise,

    Subject: [PATCH 3/3] core: Simplify math expressions.

    * src/gc.c (gc_up_arena): Use division instead of shift.
    (gc_flip): Simplify (free-news) * 1.5.

> diff --git a/src/gc.c b/src/gc.c
> index e0b16aa0..6834d6af 100644
> --- a/src/gc.c
> +++ b/src/gc.c
> @@ -326,11 +326,11 @@ void
>  gc_up_arena ()
>  {
>    long old_arena_bytes = (ARENA_SIZE + JAM_SIZE) * sizeof (struct scm);
> -  if (ARENA_SIZE >> 1 < MAX_ARENA_SIZE >> 2)
> +  if (ARENA_SIZE / 2 < MAX_ARENA_SIZE / 4)
>      {
> -      ARENA_SIZE = ARENA_SIZE << 1;
> -      JAM_SIZE = JAM_SIZE << 1;
> -      GC_SAFETY = GC_SAFETY << 1;
> +      ARENA_SIZE = ARENA_SIZE * 2;
> +      JAM_SIZE = JAM_SIZE * 2;
> +      GC_SAFETY = GC_SAFETY * 2;

Ah yes.  Division and multiplication are always somewhat problematic
(they need handwritten assembly code on ARM) so I opted for using simple
shifts instead...

>      }
>    else
>      ARENA_SIZE = MAX_ARENA_SIZE - JAM_SIZE;
> @@ -448,7 +448,7 @@ void
>  gc_flip ()
>  {
>    if (g_free - g_news > JAM_SIZE)
> -    JAM_SIZE = (g_free - g_news) + ((g_free - g_news) / 2);
> +    JAM_SIZE = ((g_free - g_news) * 3) / 2;

...however, it appears we were using division here already.  Oh well.
Nice.

Pushed 1,2 to to master and 3 to wip; I'd like to do a full check before
pushing to master.

Greetings,
Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond https://lilypond.org
Freelance IT https://JoyOfSource.com | Avatar® https://AvatarAcademy.com