Remote GHCi, -fexternal-interpreter
ClosedPublic

Authored by simonmar on Dec 2 2015, 10:44 AM.

Details

Summary

(Apologies for the size of this patch, I couldn't make a smaller one
that was validate-clean and also made sense independently)

(Some of this code is derived from GHCJS.)

This commit adds support for running interpreted code (for GHCi and
TemplateHaskell) in a separate process. The functionality is
experimental, so for now it is off by default and enabled by the flag
-fexternal-interpreter.

Reaosns we want this:

  • compiling Template Haskell code with -prof does not require building the code without -prof first
  • when GHC itself is profiled, it can interpret unprofiled code, and the same applies to dynamic linking. We would no longer need to force -dynamic-too with TemplateHaskell, and we can load ordinary objects into a dynamically-linked GHCi (and vice versa).
  • An unprofiled GHCi can load and run profiled code, which means it can use the stack-trace functionality provided by profiling without taking the performance hit on the compiler that profiling would entail.

Amongst other things; see
https://ghc.haskell.org/trac/ghc/wiki/RemoteGHCi for more details.

Notes on the implementation are in Note [Remote GHCi] in the new
module compiler/ghci/GHCi.hs. It probably needs more documenting,
feel free to suggest things I could elaborate on.

Things that are not currently implemented for -fexternal-interpreter:

  • The GHCi debugger
  • :set prog, :set args in GHCi
  • recover in Template Haskell
  • Redirecting stdin/stdout for the external process

These are all doable, I just wanted to get to a working validate-clean
patch first.

I also haven't done any benchmarking yet. I expect there to be slight hit
to link times for byte code and some penalty due to having to
serialize/deserialize TH syntax, but I don't expect it to be a serious
problem. There's also lots of low-hanging fruit in the byte code
generator/linker that we could exploit to speed things up.

Test Plan
  • validate
  • I've run parts of the test suite with

EXTRA_HC_OPTS=-fexternal-interpreter, notably tests/ghci and tests/th.
There are a few failures due to the things not currently implemented
(see above).

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
ezyang added a comment.Dec 8 2015, 4:14 AM

A few more random comments.

compiler/utils/Panic.hs
220

Any reason we can't use the copy from the ghci library? (I guess this is related to the, "We should always compile GHCi bits")

libraries/ghci/GHCi/TH.hs
33

How do these fields relate to tcg_th_state/tcg_th_modfinalizers/etc? For example, in TcSplice I see a direct access to tcg_th_state, but shouldn't the access go to the external interpreter?

(I think the answer is, for in-memory GHCi, you'll only ever hit tcg_th_*; and for external GHCi, you'll only ever modify QState, in which case the codepath for tcg_th_* is dead. This might be a useful thing to add an assert or two for.)

98

What's going on here? Ostensibly a TODO?

libraries/ghci/SizedSeq.hs
3

This needs docs. Just a little note that this is some sort of spine(?) strict list with a precomputed size.

ezyang added a comment.Dec 8 2015, 4:16 AM

A few more random comments.

compiler/main/HscTypes.hs
406

I think this is not entirely true: if we build iserv with stage1 (and any of the libraries you want to load into iserv) then stage1 can easily call into it. So, even if your stage2 compiler isn't working, if iserv is (ha. ha. ha.) then stage1 external GHCi can be enabled.

It seems like a generally good principle to keep as much code unmacroed as possible. But that can be for another patch.

simonmar updated this revision to Diff 5571.Dec 8 2015, 11:11 AM

Some validate fixes

simonmar updated this revision to Diff 5577.Dec 8 2015, 3:24 PM
  • validate fixes
  • install iserv binaries into $(libexecdir)
  • hopefully make iserv_dyn work when installed
  • don't hang if iserv didn't start
simonmar marked an inline comment as done.Dec 9 2015, 2:33 AM
simonmar added inline comments.
compiler/utils/Panic.hs
220

iserv can't depend on GHC, and GHC stage1 can't depend on libraries/ghci.

libraries/ghci/GHCi/TH.hs
34

The in-memory GHC isn't using this at all, yes. I did try to make the in-memory TH share the same implementation of the Quasi monad but it didn't work out well, because Quasi needs to call back to TcM, and the mechanisms for doing that are very different for the internal and external interpreter. It was easier just to run the internal TH computation directly in the TcM monad as it was done before.

99

Yes, this is the one remaining bit of TH left to implement for the external interpreter. It doesn't prevent validate from going through though.

libraries/ghci/SizedSeq.hs
4

This code is copy/pasted, which I think exempts me from having to document it :)

I'm not very fond of this anyway, it could probably be replaced with something better.

simonmar updated this revision to Diff 5582.Dec 9 2015, 3:45 AM

fix a validate bug

simonmar updated this revision to Diff 5584.Dec 9 2015, 5:06 AM

rebase, and hopefully validate-clean this time

thomie added inline comments.Dec 9 2015, 12:49 PM
rules/build-prog.mk
96

utils/unlit/ghc.mk contains utils/unlit_dist_TOPDIR = YES.

There is a comment in mk/install.mk that says:

GHC needs package.conf and unlit to be in the same place

Either that comment is out of date and should be removed (and perhaps change mk/build-perl.mk to also put scripts in libdir/bin instead of libdir when TOPDIR=YES), or putting unlit in libdir/bin is a bug.

300

You can now remove all occurrences of INSTALL_TOPDIR_BINS in the toplevel ghc.mk.

At the moment it's looking like I won't have time to get this in before the freeze, because I still have to make it work on Windows and that's probably a couple of days work at least.

simonmar added inline comments.Dec 10 2015, 7:14 AM
rules/build-prog.mk
96

Good catch; I think the comment can be updated, it just means that GHC looks for unlit in the same place as package.conf, and I changed GHC so it looks in bin.

If you're wondering about the reason for this change, it is because iserv_dyn is dynamically linked and all dynamically-linked binaries look for their libraries in $ORIGIN/../<package>. Furthermore it is better to have all the binaries in one place, it was a bit strange to have some of them under bin and some not.

simonmar updated this revision to Diff 5677.Dec 14 2015, 1:13 PM

Attempt (untested) to make it validate on Windows

simonmar updated this revision to Diff 5723.Dec 15 2015, 10:50 AM

missing instance

simonmar updated this revision to Diff 5750.Dec 16 2015, 2:40 AM

fix validate

simonmar updated this revision to Diff 5756.Dec 16 2015, 6:32 AM

fix validate

simonmar updated this revision to Diff 5758.Dec 16 2015, 7:08 AM

Windows fix

simonmar updated this revision to Diff 5761.Dec 16 2015, 8:30 AM

Windows fix

simonmar updated this revision to Diff 5762.Dec 16 2015, 8:56 AM

rebase and merge again; sigh

simonmar updated this revision to Diff 5763.Dec 16 2015, 9:27 AM

bump binary dependency

simonmar updated this revision to Diff 5765.Dec 16 2015, 9:53 AM

bump binary dependency

simonmar updated this revision to Diff 5766.Dec 16 2015, 10:19 AM

Windows fixes

simonmar updated this revision to Diff 5772.Dec 16 2015, 11:13 AM

Another Windows fix

simonmar updated this revision to Diff 5778.Dec 16 2015, 3:13 PM

Ok, it validates for me, and I've checked that on Windows there are only benign-looking test failures, so one last try in harbourmaster and then I'm going to push this.

This revision was automatically updated to reflect the committed changes.