Re: Guile 1.8.9 release

[Han-Wen Nienhuys]

On Sun, Feb 14, 2021 at 11:35 AM Han-Wen Nienhuys  wrote:
> >> Thanks. It turns out my previous fix introduced ABI
> >> breakage, so I reworked it to not change function
> >> signatures or struct sizes. It's also split up in more
> >> parts, so it becomes easier to understand.  Please see
> >> here: [...]
> >
> >Any news here? Can I do anything to get this fix in?

ping?

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Guile 1.8.9 release

[Han-Wen Nienhuys]

On Wed, Feb 10, 2021 at 11:18 PM Thien-Thi Nguyen  wrote:
>
>
> () Han-Wen Nienhuys 
> () Tue, 9 Feb 2021 09:59:07 +0100
>
>> Thanks. It turns out my previous fix introduced ABI
>> breakage, so I reworked it to not change function
>> signatures or struct sizes. It's also split up in more
>> parts, so it becomes easier to understand.  Please see
>> here: [...]
>
>Any news here? Can I do anything to get this fix in?
>
> IIUC, the second iteration achieves the same goals as the first
> one (i.e., reducing unnecessary allocation by refining the heap
> monitoring machinery).  Is that correct?  (What am i missing?)


the first iteration broke ABI compatibility. The second patch doesn't

>
> I would be happy to commit the second patch, if you could refine
> it to add the extensive explanation of the first.  (You could
> even mention the first approach, as an interesting but misguided
> dead end.)  That way, we have a full record.


Maybe this wasn't clear, but the second patch is actually a sequence
of patches, and in aggregate they have more detailed explanation of
what is going on.

The following are cosmetic changes.  While they don't have to be
merged, per se, the formatting fixes are the basis of the bugfix
changes:

701f6e2cae3dfc1e280711345fb5a75b0aae gc: cleanup DEBUGINFO printfs.
ce503b481e7486a7fb5152d3075c2d475fd33e06 gc: fix formatting inconsistencies
925edffc2d4efd19333582ca588e0aebb1c7adf8 gc-segment: clarify comments
on segment initialization

these are the real bug fixes:

2511e1fa97558e1d0f0620489cdd7550e7d77195 gc: reinterpret
scm_gc_cells_collected as garbage counter
594783b15b00133d73aed00bb7f3304a56725497 gc: use normal sweep for pre-mark sweep

the following are minor follow-on cleanups:

923c41cb94462140fa07120632f5736680a0c76e gc: calculate min_yield statelessly
e2d04fdd4d8a3d9cebd0d9289c5b8f9528e47d34 gc: simplify statistic keeping

> I would be extremely happy to commit a test along w/ the change,
> if we can figure that out.  But it's not critical (we can do it
> later).
>
> Re testing, i don't know how to go about setting up a test to
> avoid regressions.  (IIUC, this is a performance-related change
> and not a functionality-related one.)  Any ideas?

I can add a test, but it requires adding several fields to the
(gc-stats) output, which might surprise callers not expecting them.
Also, as it is a performance test, it is hard to construct a test that
always works.

--
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Guile 1.8.9 release

[Thien-Thi Nguyen]

() Han-Wen Nienhuys 
() Tue, 9 Feb 2021 09:59:07 +0100

   > Thanks. It turns out my previous fix introduced ABI
   > breakage, so I reworked it to not change function
   > signatures or struct sizes. It's also split up in more
   > parts, so it becomes easier to understand.  Please see
   > here: [...]

   Any news here? Can I do anything to get this fix in?

IIUC, the second iteration achieves the same goals as the first
one (i.e., reducing unnecessary allocation by refining the heap
monitoring machinery).  Is that correct?  (What am i missing?)

I would be happy to commit the second patch, if you could refine
it to add the extensive explanation of the first.  (You could
even mention the first approach, as an interesting but misguided
dead end.)  That way, we have a full record.

I would be extremely happy to commit a test along w/ the change,
if we can figure that out.  But it's not critical (we can do it
later).

Re testing, i don't know how to go about setting up a test to
avoid regressions.  (IIUC, this is a performance-related change
and not a functionality-related one.)  Any ideas?

-- 
Thien-Thi Nguyen ---
 (defun responsep (query)   ; (2021) Software Libero
   (pcase (context query)   ;   = Dissenso Etico
 (`(technical ,ml) (correctp ml))
 ...))  748E A0E8 1CB8 A748 9BFA
--- 6CE4 6703 2224 4C80 7502



signature.asc
Description: PGP signature

Re: Guile 1.8.9 release

[Han-Wen Nienhuys]

On Sun, Jan 17, 2021 at 7:37 PM Han-Wen Nienhuys  wrote:

> On Sun, Jan 17, 2021 at 12:10 AM Thien-Thi Nguyen  wrote:
> >your timing is fortuitous. I just spent the christmas
> >holidays delving into GUILE 1.8's heap expansion strategy and
> >found and fixed a bug with it.
> >
> >
> https://github.com/hanwen/guile/commit/9b32504780e0b604196be866b8c36079891e3cd6
> >
> >How does code review for proposed patches work these days?
> >
> > I don't know.  I think what you did is fine.  I invite experts
> > to review and comment.  They will surely be quicker than me (i
> > will require a week or two just to read/understand the patch).
> >
> >I'd be happy to polish it up (ie. add a proper test) for
> >inclusion in GUILE 1.8.9,
> >
> > Sounds good.  I think the general approach for 1.8.x releases
> > will be bugfixes and documentation changes primarily, so your
> > change would be most welcome (once i wrap my head around it).
>
> Thanks. It turns out my previous fix introduced ABI breakage, so I
> reworked it to not change function signatures or struct sizes. It's
> also split up in more parts, so it becomes easier to understand.
> Please see here:
>
>
> https://gcithub.com/hanwen/guile/commit/8fbe3222cac4b4e9b39a6a3570ac43f160faa516
> 



Any news here? Can I do anything to get this fix in?

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Guile 1.8.9 release

[Han-Wen Nienhuys]

On Sun, Jan 17, 2021 at 12:10 AM Thien-Thi Nguyen  wrote:
>
>
> () Han-Wen Nienhuys 
> () Tue, 12 Jan 2021 09:20:55 +0100
>
>your timing is fortuitous. I just spent the christmas
>holidays delving into GUILE 1.8's heap expansion strategy and
>found and fixed a bug with it.
>
>
> https://github.com/hanwen/guile/commit/9b32504780e0b604196be866b8c36079891e3cd6
>
>How does code review for proposed patches work these days?
>
> I don't know.  I think what you did is fine.  I invite experts
> to review and comment.  They will surely be quicker than me (i
> will require a week or two just to read/understand the patch).
>
>I'd be happy to polish it up (ie. add a proper test) for
>inclusion in GUILE 1.8.9,
>
> Sounds good.  I think the general approach for 1.8.x releases
> will be bugfixes and documentation changes primarily, so your
> change would be most welcome (once i wrap my head around it).

Thanks. It turns out my previous fix introduced ABI breakage, so I
reworked it to not change function signatures or struct sizes. It's
also split up in more parts, so it becomes easier to understand.
Please see here:

https://github.com/hanwen/guile/commit/8fbe3222cac4b4e9b39a6a3570ac43f160faa516

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Guile 1.8.9 release

[Thien-Thi Nguyen]

() Han-Wen Nienhuys 
() Tue, 12 Jan 2021 09:20:55 +0100

   your timing is fortuitous. I just spent the christmas
   holidays delving into GUILE 1.8's heap expansion strategy and
   found and fixed a bug with it.

   
https://github.com/hanwen/guile/commit/9b32504780e0b604196be866b8c36079891e3cd6

   How does code review for proposed patches work these days?

I don't know.  I think what you did is fine.  I invite experts
to review and comment.  They will surely be quicker than me (i
will require a week or two just to read/understand the patch).

   I'd be happy to polish it up (ie. add a proper test) for
   inclusion in GUILE 1.8.9,

Sounds good.  I think the general approach for 1.8.x releases
will be bugfixes and documentation changes primarily, so your
change would be most welcome (once i wrap my head around it).

-- 
Thien-Thi Nguyen ---
 (defun responsep (query)   ; (2021) Software Libero
   (pcase (context query)   ;   = Dissenso Etico
 (`(technical ,ml) (correctp ml))
 ...))  748E A0E8 1CB8 A748 9BFA
--- 6CE4 6703 2224 4C80 7502



signature.asc
Description: PGP signature