tweag / rules_haskell

Haskell rules for Bazel.
https://haskell.build
Apache License 2.0
264 stars 79 forks source link

Profiling is unecessary slow #438

Open guibou opened 6 years ago

guibou commented 6 years ago

This is an updated description which summarize the discussion

We have two solution here:

guibou commented 5 years ago

I'm still not able to generate a small reproducible example, but I now have a lot of question related to the implementation of -prof in rules_haskell.

So, rules_haskell fails building some targets in the following context:

When experimenting, I found that removing the -fexternal-interpreter flag from https://github.com/tweag/rules_haskell/blob/f7b355a9c5da4e0b81fc60bc963cd78f0dc4fe0b/haskell/private/actions/compile.bzl#L202 leads to progress, i.e. targets which cannot be built previously can now be built.

I've tried the same change in rules_haskell codebase and the result is surprising. bazel test -c dbg //... fails all tests, but bazel test -c dbg //a_specific_test works for all tests. I'll try to understand that later.

There are two ways to build with profiling using ghc and TemplateHaskell, -fexternal-interpreter and a two phases build where you first build without -prof and then with. This is detailed in https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/glasgow_exts.html#using-template-haskell-with-profiling

For example, this experimentation (with only one module, in this case the two phases build is not really necessary, but will be for more modules).

Foo.hs:

{-# LANGUAGE TemplateHaskell #-}

main = do
  print $([| 10 |])

Build without profiling:

[guillaume@paddle:/tmp]$ ghc Foo.hs
[1 of 1] Compiling Main             ( Foo.hs, Foo.o )
Linking Foo ...
[guillaume@paddle:/tmp]$ ./Foo 
10
[guillaume@paddle:/tmp]$ ./Foo +RTS -p
Foo: the flag -p requires the program to be built with -prof
[...]

Failure with just -prof:

[guillaume@paddle:/tmp]$ ghc Foo.hs -prof
[1 of 1] Compiling Main             ( Foo.hs, Foo.o )
Foo.hs:4:9: fatal:
    Cannot load -prof objects when GHC is built with -dynamic
    To fix this, either:
      (1) Use -fexternal-interpreter, or
      (2) Build the program twice: once with -dynamic, and then
          with -prof using -osuf to set a different object file suffix.

(Note how it discusses the two solutions)

First solution with -fexternal-interpreter:

[guillaume@paddle:/tmp]$ ghc Foo.hs -prof -fexternal-interpreter
[1 of 1] Compiling Main             ( Foo.hs, Foo.o )
Linking Foo ...

[guillaume@paddle:/tmp]$ ./Foo +RTS -p
10

(This build command takes 30s on my computer)

Second solution, with the two builds:

[guillaume@paddle:/tmp]$ ghc Foo.hs
[1 of 1] Compiling Main             ( Foo.hs, Foo.o )
Linking Foo ...

[guillaume@paddle:/tmp]$ ghc Foo.hs -prof -osuf curryforpresident
[1 of 1] Compiling Main             ( Foo.hs, Foo.curryforpresident )
Linking Foo ...

[guillaume@paddle:/tmp]$ ./Foo +RTS -p
10

This solution takes 2s to build.

-fexternal-interpreter is discussed here: https://ghc.haskell.org/trac/ghc/wiki/RemoteGHCi

Now I have many discussions points to direct my work. Currently, rules_haskell uses -fexternal-interpreter AND the two build solution at the same time, see:

https://github.com/tweag/rules_haskell/blob/f7b355a9c5da4e0b81fc60bc963cd78f0dc4fe0b/haskell/private/actions/compile.bzl#L202

https://github.com/tweag/rules_haskell/blob/f7b355a9c5da4e0b81fc60bc963cd78f0dc4fe0b/haskell/private/haskell_impl.bzl#L66-L107

The -fexternal-interpreter flag was added in the middle of a pull request which seems totally unrelated:

https://github.com/tweag/rules_haskell/commit/be5cf672a366e572bf168dbbc727806c2e885892#diff-33a8c46955e2a9917090b5258346aee2R205

@mrkkrp could you comment please?

There is two possible directions:

Both solutions have pro and cons, mostly:

cc @mboes

guibou commented 5 years ago

338 is related because @mrkkrp proposes to uses -fexternal-interpreter in rules_haskell. But that's already the case apparently.

guibou commented 5 years ago

Note that cabal uses the "two step" build solution.

I changed title's issue to reflect the current discussion. I should have created this discussion in a new issue.

guibou commented 5 years ago

Progress notes:

-fexternal-interpreter approach

Keeping -fexternal-interpreter is slow and leads to a lot of error. See the following bug report I've opened:

I'm able to build a client codebase with -c dbg, -fexternal-interpreter and a lot of custom hacks. Most of the libraries used by the client needs to be added to LD_PRELOAD before calling GHC. A few of the client executables built with -c dbg and -fexternal-interpreter are failing with a segmentation fault inside the stg_noDuplicatezh symbol. I don't know yet if that's due to the building process or if the client code wrong in any way. But that code works well without -c dbg.

I'm waiting a bit for activities on the upstream bug reports, but I'm tempted to drop -fexternal-interpreter support for now.

removing -fexternal-interpreter

I'm trying to remove -fexternal-interpreter and fix the two step build process. However it fails during the second build step when ghc is looking for objects file built during the first step.

During the second phase, ghc looks for o_dyn object files for its template haskell phase. However it look at them in the directory specified by -odir in the second phase, so the obj_prof/target_name directory, when they are in the obj/target_name directory. This leads to build time error similar to:

cannot find object file obj_prof/target_name/ModuleName.o_dyn

(This message is hand crafted, but it looks similar to that). Observe the o_dyn and obj_prof.

How to fix that, a few solutions:

mboes commented 5 years ago

Nice writeup, but I'm failing to understand what the objective of this ticket is. The title of the ticket says profiling is "slow". But the description says there's some build error somewhere (but no repro). Which problem are you trying to solve?

guibou commented 5 years ago

@mboes

What I'm trying to solve:

Considering that the GHC linking bug won't be fixed soon (in a released GHC version), the slowdown of -fexternal-interpreter and the fact that this may impact all users of rules_haskell, I'm convinced that the solution is to fix the profiling build with-fexternal-interpreter`. Especially when I think that this option was added by mistake in the codebase.

mboes commented 5 years ago

There's a lot of good discussion in the comments here, but they read as a journal of incremental discoveries, so it's hard to follow. Meanwhile the ticket description is still... very minimal. @guibou could you summarize your findings in an updated ticket description?

Profpatsch commented 4 years ago

ping @guibou

guibou commented 4 years ago

I've updated the ticket description and I'm closing as won't fix.

Rational: there is two way to fix this issue:

guibou commented 4 years ago

Actually, I'm reopening it. Profiling is still slow, even if we did choose to keep the slow implementation, the problem is still there.

guibou commented 4 years ago

There is no more details in the upstream ticket, but the problem seems to be fixed starting ghc 8.8. https://gitlab.haskell.org/ghc/ghc/issues/16256

Now the issue is linked to #1323 .

guibou commented 4 years ago

https://gitlab.haskell.org/ghc/ghc/issues/14335 ghc plugins are not working with -fexternal-interpreter is another argument against using external-interpreter.