[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: GUILE_MAX_HEAP_SIZE
From: |
Ludovic Courtès |
Subject: |
Re: GUILE_MAX_HEAP_SIZE |
Date: |
Mon, 18 Aug 2008 17:48:45 +0200 |
User-agent: |
Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) |
Han-Wen Nienhuys <address@hidden> writes:
> Ludovic Courtès escreveu:
>> * 51ef99f7fa9fb766fbb48619fc5863ab9914591d
>> Fix memory corruption issue with hell[] array: realloc/calloc need to
>> factor in sizeof(scm_t_bits)
>>
>> - hell = scm_malloc (hell_size);
>> + hell = scm_calloc (hell_size * sizeof(scm_t_bits));
>>
>> Good catch, but it should read:
>>
>> hell = scm_calloc (hell_size * sizeof (*hell));
>>
>> `sizeof (*hell)' is actually `sizeof (scm_t_bits *)', which is equal
>> to `sizeof (scm_t_bits)', but using `sizeof (*hell)' is clearer and
>> less error-prone.
>>
>> Besides, is that code only used when the one changes the class of an
>> instance? How did you trigger it?
>
> valgrind. Fixed.
Sorry, I meant: which Scheme code leads to the execution of that code?
>
>> * b61b5d0ebea924ee4b3930b2cba72e282d4751c7
>> Do not include private-gc.h in srfi-60.
>>
>> Hmm, well, we really need an `SCM_MIN ()' somewhere. I'd rather
>> duplicate its definition than expand it as you did here, since that
>> makes the code rather unreadable.
>
> I called private-gc.h private for a reason. Please do not include it
> unless you are called libguile/gc*c; feel free to transplant SCM_MIN to
> someplace else.
I agree on that, but I also think that expanding `SCM_MIN ()' in-place
is not a good idea.
>
>> * 569aa529d5379f3c942fa6eb01e8a1ad48ba9f77
>> Use word_2 to store mark bits for freeing structs and vtables in the
>> correct order.
>>
>> Can you explain this? Suppose we have struct S whose vtable is V;
>> V cannot be swept in the same GC cycle as S since it's still
>> referenced by S. Thus, I don't understand the need for
>> special-casing here.
>
> Freeing S requires a function stored in V.
Right, but my understanding is that V is still reachable (via S) when S
becomes candidate for sweeping. Is that right?
>> * d09752ffd17688b33a1e828cf4c11f66b86c3c3c
>> Introduce scm_i_marking to detect when GC mark bits are touched
>> outside of marking stage.
>>
>> `ensure_marking ()' must be `static'. The definition of
>> `scm_i_marking' clearly doesn't belong in a header. Besides, all
>> this is unused, so what's the point?
>
> I'm not sure where to put the code, perhaps in a ifdef DEBUG or something:
> the point was to extend SCM_GC_SET_MARK with ensure_marking() to catch
> illegal
> use of the mark bits.
But it's actually unused (at least in this commit), so I'd just remove
it.
> By rolling back changes, you have just robbed me of the opportunity to see
> what it was I did wrong.
I'd have preferred it if your changes had remained in your branch while
we discuss them. We'd have been able to take time to understand them,
and I wouldn't have had to rush because the thing is already committed.
> Can you remind me again of the sha1 of the stable
> branch? AFAICR I only pushed trivial fixes there.
Right, my mistake. The stable branch is `branch_release-1-8'.
> Also, if a core contributor apparently need some sort of review process to
> push
> code they feel comfortable with, can you please post a link to the process?
There's no such document, just an observation of what has been common
practice since I follow Guile development (c. 2004).
> It's not that I see many reviews of your commits on the list passing by.
Please, see the archive of `guile-devel'. I rarely commit without
posting here first, and it's proved to be a good tool to improve
quality.
Thanks,
Ludo'.
- Re: GUILE_MAX_HEAP_SIZE, (continued)
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/19
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/19
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/21
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/21
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/21
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/22
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/21
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/22
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/18
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/18
- Re: GUILE_MAX_HEAP_SIZE,
Ludovic Courtès <=
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/19
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/18
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/19
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/19
- GOOPS memory corruption in `go_to_hell ()', Ludovic Courtès, 2008/08/19
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/19
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/22
- Re: GUILE_MAX_HEAP_SIZE, Ludovic Courtès, 2008/08/22
- Re: GUILE_MAX_HEAP_SIZE, Han-Wen Nienhuys, 2008/08/22
Re: GUILE_MAX_HEAP_SIZE, dsmich, 2008/08/14