Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code review: IAssassin scheme_start invokes gclib_alloc_rts #611

Open
larceny-trac-import opened this issue Jul 28, 2013 · 2 comments
Open

Comments

@larceny-trac-import
Copy link

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 ...)

@ghost ghost assigned pnkfelix Jul 28, 2013
@larceny-trac-import
Copy link
Author

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
Copy link
Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants