Re: [Chicken-hackers] [PATCH][5] Make temporary stack resizable (fixes #1098)

[Peter Bex]

On Mon, Nov 02, 2015 at 07:21:06PM +0100, address@hidden wrote:
> > I know what you mean, but we've seen that in practice there are some
> > libraries that heavily rely on apply, most notably SSAX's sxml
> > transformations.  And an XML element with 4096 child nodes isn't
> > especially huge.
> 
> That doesn't change the fact that it is very bad programming style. It doesn't
> scale, is not portable and very inefficient. Endorsing such a programming 
> style
> encourages writing bad code, therefore I really recommend a hard limit.

It's not up to us to tell other how to write their code (except when it
goes into the core, of course!), and SSAX *is* one of the few popular
portable Scheme libraries in actual use.  But if you think the change in
runtime.c is too nasty, I don't care *that* much to press the issue.

> > precisely when the stack is full.  This could result in extremely tricky
> > debug situations where a procedure works fine by itself, but in a program
> > under *certain conditions* it would fail with an assertion error.  That's
> > what happens now if you exceed the parameter limit in a direct
> > invocation.  Of course this is even rarer than manyarg apply, but it
> > could happen due to macro expansion.
> 
> I don't know, but this situation sounds a bit artificial to me, and such 
> corner
> cases are not limited to procedure calls. Or I don't understand the scenario
> you describe. With "direct call" you mean a call without "apply", right?

Correct.

> Can you elaborate on those "certain conditions"?

If you just called a function that accepts a whole lot of arguments and
needs to allocate on the stack, then it will trigger a GC.  This means
it saves all its arguments on the temporary stack.  This is when this
assertion will be triggered.

If the stack is *not* full, the number of arguments don't matter.
This means that you can have such a situation in your code lurking for
a long time before actually _hitting_ the assertion one day when the
stack layout changes due to external factors, like when you rewrite a
completely unrelated piece of code.  That's what I meant by hard to debug.

> > I don't think the change is that invasive; it only really affects one
> > C function, and I've cleaned up some cruft that's no longer needed
> > and simplified a test.
> 
> I understand, still every line added to runtime.c is a problem, IMHO.

Yeah, runtime.c should go on a serious diet after we release CHICKEN 5.

> But that is not the main motivation for my concern.

Then what is?  I don't think it's our place to be deciding on the coding
style of others.

> > Alternatively, we could just raise the limit of 4096 to something
> > higher (what's an acceptable limit?), but that means pre-allocating
> > more memory that's rarely actually used.  I think the less effectively
> > "useless" memory we pre-allocate, the better.
> 
> I think 4096 is fine, as is anything larger. Memory consumption is not a
> problem in the moment (a minimal program, or csi when started fresh, consumes 
> relatively
> little memory, at least the last time I checked)

I know it's only a minor win, but I think the new code is somewhat more
flexible with almost no runtime cost for "better" programs, which is a
bonus.

> Using apply with potentially unlimited argument lists (or even potentially 
> very
> large lists) looks sometimes like an easy way out, but is always wrong and 
> should be
> avoided (and can, in all cases I have encountered so far).

True, but sometimes when on a tight deadline it can be good if you
can get away with a nasty hack.  As long as it's for a throwaway program
or if not, that you later fix it.  As long as you actually do that :)

Cheers,
Peter

signature.asc
Description: Digital signature