Re: [epsilon-devel] [Patch] Integrating Jitter as a sub-package in Poke: first iteration

[Luca Saiu]

Good morning José.

I am taking the liberty of also including some background which will be
obvious to you in this response, in the hope that others will be
interested.

On 2019-09-16 at 03:28 +0200, Jose E. Marchesi wrote:

> Hm, so now you have routines and "executable routines" as separated
> concepts.  You can go from the first to the second, and there is a link
> back:
>
>         routine  >>> executable routine
>             ^               |
>             |               |
>             +---------------+

Right.  The circular structure is for easy freeing; it can also makes
data structures such as closures simpler, by letting you only include
one pointer instead of two.

> Something doesn't feel right here... isn't being executable (and
> therefore not editable) a property of a routine?  If so, why not
> exposing it as a property, instead of adding a new separated (but
> coupled, the weird "link back") abstraction?

Non-executable routines are good for code generation -- and they will
support some optimization as well; for example it is easy to eliminate
some jumps.  Non-executable routines need data structures handling, for
example, labels.  There are dynamically growing buffers, and hash
tables.  Moreover, rewriting happens on non-executable routines: the
last few generated VM instructions may be transparently replaced by
alternatives, while the user keeps generating more.  This involves a few
complications and some memory overhead.

Executable routines are purely for executing, and much simpler.  In
direct-threaded code you have an array containing pointers to native
code and residual VM instruction arguments; in no-threading an
executable routine is even simpler: one contiguous block of executable
memory containing hardware instructions.

I also toy with more radical optimizations in my mind, such as using an
obstack style of allocation for all the data structures in
non-executable routines, to make them easy to destroy in an instant and
more cache-friendly, and to allocate the structs for executable routines
directly in mmapped memory.


I decided to separate non-executable routines from executable routines
essentially to be able to free one without the other: at execution you
do not need non-executable routines, unless you want to debug.  The
printing of VM code works with non-executable routines only, while
disassembly relies on executable routines but works in a more precise
way when the companion non-executable routine exists as well: for each
VM instruction you get its name and precise boundaries, so that you see
which hardware instructions implement which VM instruction.

There might also be room for speed optimizations with the new solution,
but my main concern is memory usage in systems containing many small
pieces of compiled code.  From what I have seen even Poke may work like
that, in fact: do you want to keep all the predefined library code
inspectable and easy to debug at all times?

Here is a practical example.  JitterLisp, when given the --free-routines
option, will destroy non-executable routines right after making the
executable versions.  It works the same, except for debugging:

x[luca@moore ~/repos/jitter/_build/native-gcc-9]$ bin/jitterlisp--unsafe--boehm 
--free-routines --no-repl --eval '(disassemble 1+)'
WARNING: specialized instruction return (opcode 185) is defective but has no 
replacement
WARNING: specialized instruction tail-call/n0 (opcode 187) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n1 (opcode 188) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n2 (opcode 189) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n3 (opcode 190) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n4 (opcode 191) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n5 (opcode 192) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n6 (opcode 193) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n7 (opcode 194) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n8 (opcode 195) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n9 (opcode 196) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n10 (opcode 197) is defective but 
has no replacement
WARNING: specialized instruction tail-call/nR (opcode 198) is defective but has 
no replacement
    0x00007ffff427eea0 58                       popq   %rax
    0x00007ffff427eea1 49 89 06                 movq   %rax,(%r14)
    0x00007ffff427eea4 48 83 c1 10              addq   $0x10,%rcx
    0x00007ffff427eea8 49 83 ed 08              subq   $0x8,%r13
    0x00007ffff427eeac 41 ff 36                 pushq  (%r14)
    0x00007ffff427eeaf c3                       retq   
    0x00007ffff427eeb0 49 83 ee 08              subq   $0x8,%r14
    0x00007ffff427eeb4 e9 2d e3 ff ff           jmpq   0x00007ffff427d1e6

This is not so obvious to follow.

x[luca@moore ~/repos/jitter/_build/native-gcc-9]$ bin/jitterlisp--unsafe--boehm 
--no-free-routines --no-repl --eval '(disassemble 1+)'
WARNING: specialized instruction return (opcode 185) is defective but has no 
replacement
WARNING: specialized instruction tail-call/n0 (opcode 187) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n1 (opcode 188) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n2 (opcode 189) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n3 (opcode 190) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n4 (opcode 191) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n5 (opcode 192) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n6 (opcode 193) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n7 (opcode 194) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n8 (opcode 195) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n9 (opcode 196) is defective but has 
no replacement
WARNING: specialized instruction tail-call/n10 (opcode 197) is defective but 
has no replacement
WARNING: specialized instruction tail-call/nR (opcode 198) is defective but has 
no replacement
# 0x51aee0: !BEGINBASICBLOCK 0x7ffff427eea0 (0 bytes):
# 0x51aee8: procedure-prolog (4 bytes):
    0x00007ffff427eea0 58                       popq   %rax
    0x00007ffff427eea1 49 89 06                 movq   %rax,(%r14)
# 0x51aee8: primitive-one-plus/fR 0x51aef0 (4 bytes):
    0x00007ffff427eea4 48 83 c1 10              addq   $0x10,%rcx
# 0x51aef0: nip (4 bytes):
    0x00007ffff427eea8 49 83 ed 08              subq   $0x8,%r13
# 0x51aef0: return (13 bytes):
    0x00007ffff427eeac 41 ff 36                 pushq  (%r14)
    0x00007ffff427eeaf c3                       retq   
    0x00007ffff427eeb0 49 83 ee 08              subq   $0x8,%r14
    0x00007ffff427eeb4 e9 2d e3 ff ff           jmpq   0x00007ffff427d1e6
# 0x51aef0: !BEGINBASICBLOCK 0x7ffff427eeb9 (0 bytes):
# 0x51aef8: unreachable (0 bytes):

Much better: and now it becomes clear that return is, in fact,
defective.

Better for us, but in production and after these internal bugs are fixed
I would not expect a lot of users to ever really look at this, which is
why I want to provide the possibility of removing waste.

> This way you don't need to expose two different types, and I don't need
> to mess with that weird one-directional link from "executable routine"
> to "routine" like in:
>
> -        pvm_destroy_program (argv[i].val.prog);
> +        {
> +          pvm_routine rout = argv[i].val.erout->routine;
> +          if (rout != NULL)
> +            pvm_destroy_routine (rout);
> +          pvm_destroy_executable_routine (argv[i].val.erout);
> +        }
>
> Having both pvm_destroy_routine and pvm_destroy_executable_routine,
> why??  I want to destroy a routine, period.

I believe I addressed the point above, but I am perfectly willing to add
helper functions for your use case.

So, to repeat the conversation on IRC of which you may have missed the
last past:

You want to only ever work with non-executable routines.  For this you
would like:

1) a way of executing a non-executable routine;

2) a way of destroying both routines, given only the non-executable one.

This is essentially all that you require, and it can be achieved with
simple wrapper functions.  Apart from the ugly name I anticipate for 2)
I see no problem.

I would even give you two versions of 1), one automatically making an
executable version on the fly if none is present; and another, unsafe,
avoiding the check.

Would this be a satisfactory solution?

From the point of view of the API, it should be indistinguishable from
your proposal, with the advantage of allowing more efficient solutions
as well.

> I'm not sure what you mean with "the build system code selecting a
> Jitter dispatch".  Do you mean --with-debug?

Yes.  The Automake conditional POKE_DEBUG used in src/Makefile.am does
what now you can do by passing options to Jitter's configure through
Poke's configure:

  [poke]$ ./configure --enable-dispatch-no-threading

will just work, relaying the option to jitter/configure .

Minimal-threading and no-threading are disabled by default now.

>     This is off-topic for the change set:
>     
>     e) I really think you should keep your C files generated by Jitter into
>        the source directory.  This will make cross-compilation easy.
>
> They are, in the distributed tarball.  Automake does that with sources
> defined as BUILT.

I use BUILT_SOURCES as well, and sometimes I have to explicitly use
$(srcdir) and friends in Makefile.am.  However if your built sources get
distributed and actually used in a tarball build there is really no
problem.  Sorry.  I thought they were rebuilt every time.

>     f) I saw that you copied autoconf/jitter.a4 from the Jitter distribution
>        into acinclude.m4 .  I updated your copy with the new version, however
>        I think you should switch to using AC_CONFIG_MACRO_DIRS and distribute
>        unrelated Autoconf macro sets in different files, as recommended by
>        the Automake manual (§"Handling Local Macros").  This will make
>        integration easier if you include other Autoconf macro files, or add
>        your own.
>
> Yeah.  I could also acinclude the jitter.m4 file there.

It will change again soon.  For example I noticed that --with-jitter
makes no sense in sub-project mode; and there will be a new option, only
in sub-project mode, for disabling the test suite with a name containing
"jitter" so as not to conflict with the super-package.

However that remains trivial to integrate.

>     In any case Poke's test suite passes with no failures.  I normally use a
>     separate build directory.
>     
>     Incidentally, Jitter's test suite will run as a side effect of running
>     Poke's test suite.  I find this slightly annoying as Jitter's test suite
>     takes some time even with only one dispatch enabled, and I think I will
>     prevent this behavior by default.  This is a forthcoming change in
>     Jitter, which should not require any modification on your side.
>
> Yes, I would appreciate if you prevent that happening by default.

Agreed.  Soon.

>     Is all of this okay?
>
> Regarding the subpackage stuff, I simply LOVE IT :)

Thanks.

> Regarding the routine vs. executable routine split into two types,
> instead of considering the later a specialized version of the former, I
> really would like to understand the rationale behind it.

I think I addressed it.

I may work on Jitter tonight, or otherwise tomorrow.  Please tell me
what you think of the wrappers idea.

Thanks for your feedback,

-- 
Luca Saiu
* My personal web site:  http://ageinghacker.net
* GNU epsilon:           http://www.gnu.org/software/epsilon
* Jitter:                http://ageinghacker.net/projects/jitter

I support everyone's freedom of mocking any opinion or belief, no
matter how deeply held, with open disrespect and the same unrelented
enthusiasm of a toddler who has just learned the word "poo".

signature.asc
Description: PGP signature