Re: Preview: portable dumper

[Pip Cet]

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. 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. Maybe we should
change staticpro to have a
void staticpro(Lisp_Object *ptr, Lisp_Object initial_value);
signature and save some lines of code?

I don't understand this hunk in sysdep.c:

@@ -2138,7 +2149,7 @@ init_signals (bool dumping)
 #ifdef SIGUSR2
   add_user_signal (SIGUSR2, "sigusr2");
 #endif
-  sigaction (SIGABRT, &thread_fatal_action, 0);
+  //sigaction (SIGABRT, &thread_fatal_action, 0);
 #ifdef SIGPRE
   sigaction (SIGPRE, &thread_fatal_action, 0);
 #endif

Similarly, this hunk in pcase.el:

@@ -63,6 +63,7 @@
 ;; FIXME: Now that macroexpansion is also performed when loading an interpreted
 ;; file, this is not a real problem any more.
 (defconst pcase--memoize (make-hash-table :weakness 'key :test 'eq))
+;; (defconst pcase--memoize (make-hash-table :test 'eq))
 ;; (defconst pcase--memoize-1 (make-hash-table :test 'eq))
 ;; (defconst pcase--memoize-2 (make-hash-table :weakness 'key :test 'equal))

Are those two intentional?

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. This would make things slightly easier for
(a) competing implementations (b) abusing the DUMPER_* machinery for
GC experiments (c) possibly extending pdumper to combine several
images rather than use a single one. It would also help avoid _impl
names.

I think it's worth mentioning that staticpro is now required for
variables that used to be safe: this change comes at the price of a
tiny performance hit because of the additional GC roots, but it's
worth it, IMHO.

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

On Thu, Mar 29, 2018 at 9:39 AM, Robert Pluim <address@hidden> wrote:
> Daniel Colascione <address@hidden> writes:
>
>> On 03/29/2018 12:12 AM, Angelo Graziosi wrote:
>>>
>>> They are 4 weeks that pdumper branch is not synchronized, should we 
>>> continue to test it?
>>>
>>> During this time, builds of that branch have been tested on Windows, 
>>> GNU/Linux and macOS without showing any specific issue...
>>
>> I'm not sure what's left to test. It works. What's stopping a merge?
>
> I just test-merged it to master, with only two issues:
>
> 1. a merge conflict in src/nsfont.m as a result of 7ff62ed221c
> 2. The Lisp_Buffer_Local_Value hash has changed, although itʼs not
> clear to me why, since I only see comment changes in its definition. I
> updated dmpstruct.h, and the build works fine (it runs gnus, as proven
> by this message).
>
> Robert
>