full moon, vm status update

full moon, vm status update

[Andy Wingo]

'Sup,

I was going to divide this update into the good, the bad, and the ugly,
but the bad is the ugly, and the good contains within it the bad as
well.

The good:

  * I've hacked the disassembler to weave together the bits of
    information that it has into a more coherent text.

    So for example where in the past you would have a separate section
    after the program text listing the instruction <-> source
    associations, now the instruction disassembly is annotated with the
    source information directly.

    This required me to become format's daddy.
    http://git.savannah.gnu.org/gitweb/?p=guile.git;a=blob;f=module/system/vm/disasm.scm;h=5ec1c004b42092a621792c1bc93e1c1b566a11fc;hb=vm#l171

    I've appended a bit of disassembly to this mail.

  * Fixed another bug in quasiquote compilation. I don't expect any
    more, but then again, I didn't expect any more.

  * The VM stack is now marked precisely.

  * There is a now a debugging mode, currently turned on, in which we
    try ensure that the top of the stack is non-NULL and that all
    elements past the top are set to NULL. There are a number of checks
    in various places that this is the case. The idea is to avoid lost
    references when GC runs, and the heap structure's idea of the VM
    registers is out of sync with what the VM regs actually are; or
    there is some sloppy programming somewhere. When turned off, this
    code incurs no overhead.

    This mode helped to catch a number of stack GC bugs.

  * All of Guile's test suites pass now, with (ice-9 ...) compiled,
    including boot-9.

The bad:
  (It's like a C switch statement, it all falls through to the ugly.)

The ugly:

  * Actually the bit about all of the test suites passing was a lie in
    another respect: the elisp test fails, with a C stack overflow,
    indicating too much recursion into the interpreter.

    I suspect this is because some part of the elisp code depends on
    tail calls within interpreted code, but you don't get tail call
    semantics when calling between VM code and interpreted code, just as
    you do not with calling between e.g. interpreted and C code.

    I was hoping that the stack calibration patches would land sooner or
    later in master so that I could merge and pick up this code, because
    the truth is that it's all too easy to hit the limit, for whatever
    reason.

    I have not yet tested the calibration patches.

  * I had to disable compilation of popen.scm in order to get the tests
    to pass, as it was causing really strange, nondeterministic things
    to happen. I don't know why.

    My current speculation is that when you compile --with-threads, as I
    do, that the socketpair between the signal receiving thread and the
    main thread is not closed after the fork, therefore signals in the
    child might reach the parent or vice versa, causing random code to
    run which itself might cause VM problems.

    Julian: if you have read this far and have the cycles, can you say
    something about the state of the signal socketpair after a fork?

  * Still there are a couple other modules not being compiles.

The future:

  * Compile more modules -- srfi, third-level modules -- (ice-9 foo
    bar)

  * Fix GOOPS to be VM-compatible

  * Compile *and* interpret the test suites

  * Update docs (maybe merging with guile.texi)

  and, once things are correct...

  * Profile, profile, profile

My goal is: correct execution of all existing code that:
  * does not do runtime side-effects in macros
  * does not call (the-environment)
  * does not unquote in values into macros

Well that's a long enough update! Questions, comments, and help are most
welcome.

Happy hacking,

Andy


ps. Here's the disassembly I promised:

scheme@(guile-user)> ,x with-output-to-file
Disassembly of #<program with-output-to-file (file thunk)>:

nargs = 2  nrest = 0  nlocs = 2  nexts = 0

Bytecode:

   0    (late-variable-ref 0)           ;; `open-output-file'
   2    (local-ref 0)                   ;; `file' (arg)
   4    (call 1)                                              at r4rs.scm:145:16
   6    (local-set 2)                   ;; `nport'
   8    (late-variable-ref 1)           ;; `with-output-to-port'
  10    (local-ref 2)                   ;; `nport'
  12    (local-ref 1)                   ;; `thunk' (arg)
  14    (call 2)                                              at r4rs.scm:146:14
  16    (local-set 3)                   ;; `ans'
  18    (late-variable-ref 2)           ;; `close-port'
  20    (local-ref 2)                   ;; `nport'
  22    (call 1)                                              at r4rs.scm:147:4
  24    (drop)                          
  25    (local-ref 3)                   ;; `ans'
  27    (return)                        

Properties:

  #f    (documentation . "THUNK must be a procedure of no arguments, and FILE must be a
string naming a file.  The effect is unspecified if the file already exists. 
The file is opened for output, an output port connected to it is made
the default value returned by `current-output-port', 
and the THUNK is called with no arguments.
When the THUNK returns, the port is closed and the previous
default is restored.  Returns the value yielded by THUNK.  If an
escape procedure is used to escape from the continuation of these
procedures, their behavior is implementation dependent.")

(If you read this and worry about runtime costs, don't -- the
annotations and properties are not loaded until they are asked for)
-- 
http://wingolog.org/

Re: full moon, vm status update

[Julian Graham]

Hi Andy,

>    My current speculation is that when you compile --with-threads, as I
>    do, that the socketpair between the signal receiving thread and the
>    main thread is not closed after the fork, therefore signals in the
>    child might reach the parent or vice versa, causing random code to
>    run which itself might cause VM problems.
>
>    Julian: if you have read this far and have the cycles, can you say
>    something about the state of the signal socketpair after a fork?

Well, yes, that's possible; the signal handling system certainly isn't
aware of forking.  But since only the thread calling fork() gets
created in the child process (and the signal delivery thread should
never fork), errant signal propagation would only be one-way -- in the
child-to-parent direction.  I think I'm missing a little context,
though.  Is forking (without doing an execve) something that happens
during compilation of VM code?  What should the behavior be in this
situation?  If the child process doesn't need async support, I guess
it could close the write end of the signal pipe, but that seems...
wrong, somehow.


Regards,
Julian

Re: full moon, vm status update

[Andy Wingo]

Hi,

On Thu 16 Oct 2008 07:35, "Julian Graham" <joolean@gmail.com> writes:

>>    My current speculation is that when you compile --with-threads, as I
>>    do, that the socketpair between the signal receiving thread and the
>>    main thread is not closed after the fork, therefore signals in the
>>    child might reach the parent or vice versa, causing random code to
>>    run which itself might cause VM problems.

This loopy speculation was only part-right: it *was* async handling that
was causing tests with compiled popen.scm to fail, because we didn't
save the registers before running the asyncs. I have now fixed that.

But it does not appear to have been related to the socketpair.

> Well, yes, that's possible; the signal handling system certainly isn't
> aware of forking.  But since only the thread calling fork() gets
> created in the child process (and the signal delivery thread should
> never fork), errant signal propagation would only be one-way -- in the
> child-to-parent direction.

Ah yes, good point. Still sounds like a bug though.

    scheme@(guile-user)> (sigaction SIGINT (lambda (x) (pk x (getpid))))
    $1 = (#<program #(3361 48 boot-9.scm) (sig)> . 268435456)
    scheme@(guile-user)> (define (fork-sleep-int)
                           (primitive-fork)
                           (pk 'after-fork (getpid))
                           (sleep 2)
                           (raise SIGINT))
    scheme@(guile-user)> (fork-sleep-int)

    ;;; (after-fork 878)

    ;;; (after-fork 932)
    scheme@(guile-user)> 
    ;;; (2 878)
    scheme@(guile-user)> scheme@(guile-user)> 

    ;;; (2 878)
    scheme@(guile-user)> 

You might have to press enter a couple of times to get the second pk to
come through, I think. It seems fork + signal handling is borked with a
guile compiled --with-threads -- and that's not specific to the vm
branch.

> I think I'm missing a little context, though. Is forking (without
> doing an execve) something that happens during compilation of VM code?

No, I'm just running Guile's test suite with a compiled ice-9.
Socket.test does fork without exec, but even with an exec the fd table
is kept the same iirc.

> What should the behavior be in this situation? If the child process
> doesn't need async support, I guess it could close the write end of
> the signal pipe, but that seems... wrong, somehow.

Well, in popen.scm, we already close all of the fd's that guile knows
about in the child process. So it looks like in the popen case we're
just missing one, if my suspicions are right. Then in a Guile child, we
should probably re-spawn the thread to handle signals? I have no idea
here.

An interesting bug, eh. Who wants to fix it? (Ludo? :-)

Andy
-- 
http://wingolog.org/

Re: full moon, vm status update

[Ludovic Courtès]

Hi Andy,

Andy Wingo <wingo@pobox.com> writes:

>   * The VM stack is now marked precisely.

Did you mean stack frame objects that link `program' object invocations?
I guess this stack is referenced by the C stack, so why does something
special need to be done?

>   * There is a now a debugging mode, currently turned on, in which we
>     try ensure that the top of the stack is non-NULL and that all
>     elements past the top are set to NULL. There are a number of checks
>     in various places that this is the case. The idea is to avoid lost
>     references when GC runs, and the heap structure's idea of the VM
>     registers is out of sync with what the VM regs actually are; or
>     there is some sloppy programming somewhere. When turned off, this
>     code incurs no overhead.
>
>     This mode helped to catch a number of stack GC bugs.

Are you referring to leaks due to a stack that contains references to
Scheme objects that have not been overwritten for a while?  Or are there
other bugs?

>   * All of Guile's test suites pass now, with (ice-9 ...) compiled,
>     including boot-9.

Woow!

> The ugly:
>
>   * Actually the bit about all of the test suites passing was a lie in
>     another respect: the elisp test fails, with a C stack overflow,
>     indicating too much recursion into the interpreter.

I've seen `elisp.test' trigger a stack overflow with the interpreter
more often than any other test.  Don't know why.

>   * I had to disable compilation of popen.scm in order to get the tests
>     to pass, as it was causing really strange, nondeterministic things
>     to happen. I don't know why.
>
>     My current speculation is that when you compile --with-threads, as I
>     do, that the socketpair between the signal receiving thread and the
>     main thread is not closed after the fork, therefore signals in the
>     child might reach the parent or vice versa, causing random code to
>     run which itself might cause VM problems.

Ouch, that's a tricky area, and I think Rob had some ideas about it (I
remember discussions on IRC about that quagmire).

> My goal is: correct execution of all existing code that:
>   * does not do runtime side-effects in macros
>   * does not call (the-environment)
>   * does not unquote in values into macros

How about code that does "(read-set! keywords 'prefix)" and the likes?
:-)

> Well that's a long enough update! Questions, comments, and help are most
> welcome.

You rock!

> Bytecode:
>
>    0    (late-variable-ref 0)           ;; `open-output-file'
>    2    (local-ref 0)                   ;; `file' (arg)
>    4    (call 1)                                              at r4rs.scm:145:16

And that's very nice too!

Thanks,
Ludo'.

Re: full moon, vm status update

[Andy Wingo]

Howdy,

On Thu 16 Oct 2008 21:25, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>>   * The VM stack is now marked precisely.
>
> Did you mean stack frame objects that link `program' object invocations?
> I guess this stack is referenced by the C stack, so why does something
> special need to be done?

I mean that instead of using scm_mark_locations() to mark the active
region of the allocated VM stack (vp->stack_base to vp->sp), we use
vm.c:vm_mark_stack() to mark that region precisely.

>>   * There is a now a debugging mode, currently turned on, in which we
>>     try ensure that the top of the stack is non-NULL and that all
>>     elements past the top are set to NULL. There are a number of checks
>>     in various places that this is the case. The idea is to avoid lost
>>     references when GC runs, and the heap structure's idea of the VM
>>     registers is out of sync with what the VM regs actually are; or
>>     there is some sloppy programming somewhere. When turned off, this
>>     code incurs no overhead.
>>
>>     This mode helped to catch a number of stack GC bugs.
>
> Are you referring to leaks due to a stack that contains references to
> Scheme objects that have not been overwritten for a while?  Or are there
> other bugs?

For example, the fix to libguile/vm-engine.h:POP_LIST in 11ea1aba9eb9.

  (POP_LIST): Hoo, fix a good bug: if CONS triggered a GC, the elements
  of the list that had not yet been consed would not be marked, because
  the sp was already below them.

  @@ -275,10 +283,12 @@
   do                                             \
   {                                              \
     int i;                                       \
  -  SCM l = SCM_EOL;                             \
  -  sp -= n;                                     \
  -  for (i = n; i; i--)                          \
  -    CONS (l, sp[i], l);                                \
  +  SCM l = SCM_EOL, x;                          \
  +  for (i = n; i; i--)                           \
  +    {                                           \
  +      POP (x);                                  \
  +      CONS (l, x, l);                           \
  +    }                                           \
     PUSH (l);                                    \
   } while (0)

How's that for a hard-to-find bug!!! 

>>   * Actually the bit about all of the test suites passing was a lie in
>>     another respect: the elisp test fails, with a C stack overflow,
>>     indicating too much recursion into the interpreter.
>
> I've seen `elisp.test' trigger a stack overflow with the interpreter
> more often than any other test.  Don't know why.

I hacked around this -- see what I've pushed to vm for more info.

>> My goal is: correct execution of all existing code that:
>>   * does not do runtime side-effects in macros
>>   * does not call (the-environment)
>>   * does not unquote in values into macros
>
> How about code that does "(read-set! keywords 'prefix)" and the likes?
> :-)

Oooh, just because you do dastardly things with the reader ;) You are
right. Code that causes side effects to the reader will not cause those
side effects until after the rest of the file is read, even if you
(eval-case ((load-toplevel compile-toplevel) ...)) it.

For what you want, I suggest #!lang things and reader macros (neither of
which we have yet).

Cheers,

Andy
-- 
http://wingolog.org/

Re: full moon, vm status update

[Neil Jerram]

2008/10/16 Andy Wingo <wingo@pobox.com>:
>>>   * Actually the bit about all of the test suites passing was a lie in
>>>     another respect: the elisp test fails, with a C stack overflow,
>>>     indicating too much recursion into the interpreter.
>>
>> I've seen `elisp.test' trigger a stack overflow with the interpreter
>> more often than any other test.  Don't know why.
>
> I hacked around this -- see what I've pushed to vm for more info.

I plan to take a look at this after the srfi-18.test hang.  (Not that
that should stop anyone else, of course.)

   Neil

Re: full moon, vm status update

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

>>> I've seen `elisp.test' trigger a stack overflow with the interpreter
>>> more often than any other test.  Don't know why.
>>
>> I hacked around this -- see what I've pushed to vm for more info.
>
> I plan to take a look at this after the srfi-18.test hang.  (Not that
> that should stop anyone else, of course.)

Not the stack overflow yet, but see attached for fixing the elisp apply error.

It's not rocket science, and you probably guessed at that solution
already - but I think it really is the _right_ fix, because

- the principle of the elisp integration is that there is a new value
#nil, which acts as EOL in list contexts, and as #f in boolean
contexts

- as one example of this, the non-VM apply accepts and handles a list
whose tail is #nil

- therefore the VM apply should too.

What do you think?  Let me know if you'd like me to commit.  (Note I
haven't checked in detail if any of the other places that use
PUSH_LIST should definitely NOT allow a list whose tail is #nil.  If
yes, I guess we'll need two variants of PUSH_LIST.)

Regards,
       Neil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-elisp-apply-error-list-ending-with-nil.patch --]
[-- Type: text/x-patch; name=0001-Fix-elisp-apply-error-list-ending-with-nil.patch, Size: 1652 bytes --]

From 25ea77ee209bbacab0d07e5fb01e9d64917158f1 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Mon, 27 Oct 2008 23:42:02 +0000
Subject: [PATCH] Fix elisp apply error (list ending with nil)

---
 libguile.h             |    1 +
 libguile/vm-engine.h   |    2 +-
 libguile/vm-i-system.c |    2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/libguile.h b/libguile.h
index 40122df..d28df69 100644
--- a/libguile.h
+++ b/libguile.h
@@ -58,6 +58,7 @@ extern "C" {
 #include "libguile/rdelim.h"
 #include "libguile/rw.h"
 #include "libguile/keywords.h"
+#include "libguile/lang.h"
 #include "libguile/list.h"
 #include "libguile/load.h"
 #include "libguile/macros.h"
diff --git a/libguile/vm-engine.h b/libguile/vm-engine.h
index 215f630..b6748e1 100644
--- a/libguile/vm-engine.h
+++ b/libguile/vm-engine.h
@@ -305,7 +305,7 @@ do						\
 {						\
   for (; scm_is_pair (l); l = SCM_CDR (l))      \
     PUSH (SCM_CAR (l));                         \
-  if (SCM_UNLIKELY (!SCM_NULLP (l))) {          \
+  if (SCM_UNLIKELY (!SCM_NULL_OR_NIL_P (l))) {  \
     err_args = scm_list_1 (l);                  \
     goto vm_error_improper_list;                \
   }                                             \
diff --git a/libguile/vm-i-system.c b/libguile/vm-i-system.c
index 2da2d42..4a7e439 100644
--- a/libguile/vm-i-system.c
+++ b/libguile/vm-i-system.c
@@ -39,7 +39,7 @@
  * whether to permit this exception to apply to your modifications.
  * If you do not wish that, delete this exception notice.  */
 
-/* This file is included in vm_engine.c */
+/* This file is included in vm-engine.c */
 
 \f
 /*
-- 
1.5.6

Re: full moon, vm status update

[Andy Wingo]

Hi Neil,

On Tue 28 Oct 2008 00:51, "Neil Jerram" <neiljerram@googlemail.com> writes:

> It's not rocket science, and you probably guessed at that solution
> already - but I think it really is the _right_ fix, because
>
> - the principle of the elisp integration is that there is a new value
> #nil, which acts as EOL in list contexts, and as #f in boolean
> contexts
>
> - as one example of this, the non-VM apply accepts and handles a list
> whose tail is #nil
>
> - therefore the VM apply should too.
>
> What do you think?

So, I'm really happy that you're hacking on this!

I think there are better ways of supporting multiple languages than the
approach taken historically by Guile, and that taken by this patch.

I do not think the interpreter / subr division is appropriate for a
multilingual system. To me, the division should be:

 * The evaluator / interpreter, that deals in Scheme;
 * A layered compiler, with different top-level layers for e.g. Guile,
   R6RS, Elisp, etc
 * A VM, with generic opcodes and opcodes specific to the languages that
   target the VM
 * A library of procedures written in C, where each procedure is related
   to /one/ language, Scheme in the case of most procedures.

So, the best option would be to have /a separate elisp compiler/. You
already have this, sortof. But in the specific context of apply, I would
have "apply" be a different operator in the elisp case, supported by an
elisp-specific opcode that deals with nil.

(Alternately, you could redefine `apply' in Scheme, for the elisp case:
 (define (elisp-apply f . lists)
   (apply apply f (frobate lists))))

Granted, we have ideals, and we have tradeoffs, and writing an elisp
compiler. (Then again, maybe not that bad.) If I have to add what I
consider to be a hack to the `apply' case in order to keep you hacking
on the VM, then the tradeoff is worth it to me.

But at some point I think you should indulge your secret desire of
writing a compiler! ;-)

What do you think of all this?

Cheers,

Andy
-- 
http://wingolog.org/

Re: full moon, vm status update

[Andy Wingo]

On Fri 31 Oct 2008 18:54, Andy Wingo <wingo@pobox.com> writes:

> Granted, we have ideals, and we have tradeoffs, and writing an elisp
> compiler. (Then again, maybe not that bad.) If I have to add what I

Meant to say here: "writing an elisp compiler might be a bit of work"

> consider to be a hack to the `apply' case in order to keep you hacking
> on the VM, then the tradeoff is worth it to me.

-- 
http://wingolog.org/

Re: full moon, vm status update

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> I do not think the interpreter / subr division is appropriate for a
> multilingual system. To me, the division should be:
>
>  * The evaluator / interpreter, that deals in Scheme;
>  * A layered compiler, with different top-level layers for e.g. Guile,
>    R6RS, Elisp, etc
>  * A VM, with generic opcodes and opcodes specific to the languages that
>    target the VM
>  * A library of procedures written in C, where each procedure is related
>    to /one/ language, Scheme in the case of most procedures.
>
> So, the best option would be to have /a separate elisp compiler/.

In the short term I'd go for the trade-off rather than the ideal: it'd
be great if the compiler could transparently support what the
interpreter already supports, like Elisp, `reader-options' and
`current-reader' to some extents (am I biased?  ;-)).

Something like MzScheme's "#lang" may be nice but I see it as the next
step.

Thanks,
Ludo'.

Re: full moon, vm status update

[Neil Jerram]

On 31/10/2008, Andy Wingo <wingo@pobox.com> wrote:

>  I do not think the interpreter / subr division is appropriate for a
>  multilingual system. To me, the division should be:
>
>   * The evaluator / interpreter, that deals in Scheme;
>   * A layered compiler, with different top-level layers for e.g. Guile,
>    R6RS, Elisp, etc
>   * A VM, with generic opcodes and opcodes specific to the languages that
>    target the VM
>   * A library of procedures written in C, where each procedure is related
>    to /one/ language, Scheme in the case of most procedures.

That last point seems odd to me.   Surely if Guile gets good multiple
language support,  we don't want to tell developers that they have to
write a different version of each new primitive for each high level
language?

>  So, the best option would be to have /a separate elisp compiler/. You
>  already have this, sortof.

You mean (lang elisp transform) ?  I agree that this is sort of a
compiler.  The problem is that it only handles code; it doesn't
translate data at all.

A key point of the current implementation is to avoid needing to
translate data when it is passed from one language to another; we
decided on that because data translation would be arbitrarily complex
(so potentially very bad for performance) and I think would require
tracking of language boundaries (which we don't need now).

> But in the specific context of apply, I would
>  have "apply" be a different operator in the elisp case, supported by an
>  elisp-specific opcode that deals with nil.

That's fine for elisp code calling apply directly, but not for passing
a list of args to a scheme -implemented function which calls apply.

>
>  (Alternately, you could redefine `apply' in Scheme, for the elisp case:
>   (define (elisp-apply f . lists)
>    (apply apply f (frobate lists))))

(Just the same point again; here shown by the introduction of frobate,
 and the fact that it needs to do conversion to arbitrary depth.)

>  But at some point I think you should indulge your secret desire of
>  writing a compiler! ;-)

So secret that I don't know about it?  Actually you might be right... :-)

   Neil

Re: full moon, vm status update

[Andy Wingo]

Hey Neil,

On Tue 28 Oct 2008 00:51, "Neil Jerram" <neiljerram@googlemail.com> writes:

> - the principle of the elisp integration is that there is a new value
> #nil, which acts as EOL in list contexts, and as #f in boolean
> contexts
>
> - as one example of this, the non-VM apply accepts and handles a list
> whose tail is #nil
>
> - therefore the VM apply should too.

Fixed. It's not the exact patch you had, but it's the same in the end.
I fixed the test suite as well. Apologies for the very long delay!

Peace,

Andy
-- 
http://wingolog.org/

Re: full moon, vm status update

[dsmich]

---- Andy Wingo <wingo@pobox.com> wrote: 
>It seems fork + signal handling is borked with a guile compiled --with-threads -- and that's not
>specific to the vm branch.

And this is the very reason that Debian guile was built --without-threads.

It's just not a good idea to mix fork() with pthreads.  The main issue is that the child process has all the state of the parent, including the locked state of all the mutexes.  However, the threads that hold them no longer exist, so deadlocks are almost inevitable.  There is a pthread_atfork() call that should allow you to make things safe before a fork() and restore them properly afterwards.  Guile *might* be able to use this to handle all the mutexes that it creates an knows about, but what the ones a user creates in their C code, or what some C library does?

Now, this is not much of a problem with fork()+exec().    The problem is mixing "fork threading" (where you keep the same process image in the child) and posix threads. 

-Dale