search

larcenists / larceny

Larceny Scheme implementation
Other
203 stars 32 forks source link

Code review: IAssassin scheme_start invokes gclib_alloc_rts #611

Open larceny-trac-import opened 11 years ago

larceny-trac-import commented 11 years ago

Reported by: pnkfelix on Wed Feb 18 16:05:02 2009 While grepping for uses of gclib_alloc_rts, which is supposed to be infrequently invoked with requests for '''large''' chunks of memory, I found an invocation in [source:trunk/larceny_src/src/Rts/IAssassin/i386-driver.c IAssassin's scheme_start] that looks like it is allocating what is likely to be a small piece of memory (how big is a jmp_buf?)

That might violate the expectation that the chunk is large, but maybe that's okay if the request is infrequent. And it is for the typical case (you only start Scheme once). But its also invoked on every FFI callback invocation (!).

So that invocation should get reviewed; it might be something where invoking must_malloc could be more appropriate.

(Note: Felix admits that this situation is his own fault; see changeset:4015 ; even so, must_malloc may be a better option than malloc here, which is what was used prior to changeset:4015 ...)

larceny-trac-import commented 11 years ago

Author: pnkfelix Will tells me that scheme_start is also invoked on arithmetic operations, so resolving this is probably higher priority than I had thought when I first filed this ticket.

larceny-trac-import commented 11 years ago

Author: pnkfelix I just reviewed the control flow for arithmetic operations; it looks like control flows back into scheme_start via a longjmp, but the arithmetic operations are ''not'' calling the scheme_start procedure directly, and therefore control is not (or at least should not be) reaching the gclib_alloc_rts invocation in that case.

In other words, it looks to me like this invocation of gclib_alloc_rts is indeed infrequent (apart from FFI callbacks, which were the original reason I filed this ticket).