[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Chicken-hackers] [PATCH] (hopefully) fix the massive random Salmone

From: Peter Bex
Subject: Re: [Chicken-hackers] [PATCH] (hopefully) fix the massive random Salmonella breakage
Date: Sun, 4 Oct 2015 14:55:44 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Sun, Oct 04, 2015 at 10:32:01PM +1300, Evan Hanson wrote:
> Hi Peter,
> This makes sense. I didn't know that was ever done, but after a search
> of the rest of the runtime it doesn't *look* like this temporary stack
> trick is used anywhere else. Nice find! I haven't been able to reproduce
> a crash since applying these fixes on a handful of platforms.

Cool.  Have you been able to reproduce the crash (without patch) at all?

> Just two minor things: (1) it looks like one too many words is allocated
> for the C_apply_values argvector

The argvector holds the continuation followed by each item in the
argument list, which is why I added 1 to it.  So I think my original
patch is correct, and removing the +1 would in fact introduce an
off-by-one error, unless I'm missing something here.

> (2) as a small performance tweak,
> we can avoid inspecting all the pair headers a second time during
> C_apply* by reusing the list's length when copying it into the argvector
> (since we already know its length (and that it's a regular list) by that
> point). See the attached patches.

Nice observation!  I like this second patch.

> It's also a bit silly to shift, then
> immediately unshift the C_u_i_length results, but that probably doesn't
> matter much overall.

Yeah, I considered that too but I didn't want to inline that code again.
We may refactor it into an "unboxed" version of the function that
calculates the length as a native integer, and then redefine C_u_i_length
in terms of that.  This is probably something we can do across the entire
runtime for various other functions, but that's a separate issue.

> If you're happy with those two changes, I'll push the whole lot. Then we
> can see a beautiful, error-free salmonella run tonight. :)

That would be great.  If you agree with my analysis about your first
patch, please push my changes with your second patch only.


Attachment: signature.asc
Description: Digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]