[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#56623: memory leak introduced by new nonrecursive lisp reader
From: |
Greg Hendershott |
Subject: |
bug#56623: memory leak introduced by new nonrecursive lisp reader |
Date: |
Sun, 28 Aug 2022 13:29:35 -0400 |
User-agent: |
mu4e 0.9.18; emacs 25.2.2 |
Hi, Lars.
Your previous email from a month ago doesn't seem to have been delivered
to me; maybe same happened for Tom?
Today I took some time to try making both changes to lread.c. That did
not seem to help -- in my testing using Tom's artifacts, memory use
still grew unbounded (whereas it levels off with e.g. Emacs 25.2.2).
Bu as a sanity check, it would be great if Tom could try this, too, to
make sure?
I've gone to some non-trivial effort to make a much more minimal recipe
-- but so far I can't. On the one hand, Tom's bisect seems to point
clearly at this reader change. On the other hand, I don't have any
mental model for how/why, and I can't elicit similar symptoms so far by
working up from simpler examples exercising the reader. :(
Greg
On Sat 27 Aug 2022 at 15:35, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Mattias Engdegård <mattiase@acm.org> writes:
>
>> A counter-argument is that these undead objects will only be kept
>> artificially alive as long as GC only takes place in the reader. Anyway, the
>> hunch is easily tested: try either or both of these changes:
>>
>> 1. reduce stackbufsize:
>>
>> --- a/src/lread.c
>> +++ b/src/lread.c
>> @@ -2919,7 +2919,7 @@ digit_to_number (int character, int base)
>> /* Size of the fixed-size buffer used during reading.
>> It should be at least big enough for `invalid_radix_integer' but
>> can usefully be much bigger than that. */
>> -enum { stackbufsize = 1024 };
>> +enum { stackbufsize = 64 };
>>
>> static void
>> invalid_radix_integer (EMACS_INT radix, char stackbuf[VLA_ELEMS
>> (stackbufsize)],
>>
>> 2. zero the buffer on entry:
>>
>> --- a/src/lread.c
>> +++ b/src/lread.c
>> @@ -3678,6 +3678,7 @@ read_stack_push (struct read_stack_entry e)
>> read0 (Lisp_Object readcharfun, bool locate_syms)
>> {
>> char stackbuf[stackbufsize];
>> + memset (stackbuf, 0, stackbufsize);
>> char *read_buffer = stackbuf;
>> ptrdiff_t read_buffer_size = sizeof stackbuf;
>> char *heapbuf = NULL;
>>
>> If either makes things better, maybe we can come up with a solution that
>> doesn't hurt performance.
>
> This was a month ago -- Tom, did you try these changes?