Re: Preview: portable dumper

[Pip Cet]

On Thu, Mar 29, 2018 at 3:31 PM, Daniel Colascione <address@hidden> wrote:
> On 03/29/2018 06:35 AM, Pip Cet wrote:
>> I've just skimmed the diff, and I think the change is a significant
>> improvement, and will help with my GC experiments even though I'm
>> probably never going to be able to use the portable dumper.
>
>
> Why not? I eventually want to remove unexec entirely.

No objections to removing unexec, as long as temacs remains an option
:-) I haven't thought about dumping/restoring SpiderMonkey state, but
as it's hairy enough to get things working with setjmp(), I'd rather
not worry about it now.

I do have a half-finished (working, but not thoroughly tested) unexec
for asm.js, too, which sped things up somewhat, but I don't see why
the portable dumper wouldn't work there, too.

>
>> Some minor
>> comments:
>>
>> it is not clear to me why some staticpro's are moved to happen after
>> the initialisation of the variable and some aren't.
>
>
> The code has always been a bit unclear about that; we don't GC that early,
> so it doesn't really matter- -- but if we did, we'd be saved by nil being
> all zero these days.

Well, Qnil isn't all zero (or even constant) here, and I do GC
unpredictably, though hopefully never that early; I'm still safe if
the staticpro goes first: the all-zero value, representing 0.0, is
fine for GC.

>> Maybe we should
>> change staticpro to have a
>> void staticpro(Lisp_Object *ptr, Lisp_Object initial_value);
>> signature and save some lines of code?
>
>
> Sometimes the value from .data is perfectly good though, and at that point,
> the initial_value setting would be a waste.

... as are explicit Qnil initializations, in general. So

inline void staticpro(Lisp_Object *ptr, Lisp_Object initial_value)
{
  assume(*ptr == Qnil);
  real_staticpro(ptr);
  if (initial_value != Qnil)
    *ptr = initial_value;
}

would actually reduce code size. But, again, it'd be better to merge
first, then optimize such minor things.

>> I would vaguely prefer if "pdumper" names were limited to the actual
>> pdumper implementation, and "dumper" names were used instead in code
>> that does not depend on the implementation of the dumper. So we'd have
>> dumper_object_p(), defined to false if there's no dumper, and
>> pdumper_object_p otherwise.
>
>
> I'd rather not do a big renaming at this point. Nothing stops someone doing
> cleanup work post-merge if sufficiently motivated.

Agreed.

>> Apart from that, there appear to be no disadvantages for people who
>> prefer to continue using unexec. Is that correct?
> That's correct.

Thanks!