Opened 7 months ago

Last modified 3 weeks ago

#13604 patch bug

ghci no longer loads dynamic .o files by default if they were built with -O

Reported by: George Owned by: dfeuer
Priority: highest Milestone: 8.2.3
Component: Compiler Version: 8.2.1-rc1
Keywords: RecompilationCheck Cc: ezyang, elaforge
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4123
Wiki Page:

Description

In 8.2.1-rc1 loading a file compiled with -O2 into ghci results in ghci recompiling the file into interpreted byte code. In 8.0.2 it simply loads the compiled object file.

8.2.1

 ghc -dynamic -O2 eh2.hs
[1 of 1] Compiling Main             ( eh2.hs, eh2.o )
Linking eh2 ...
bash-3.2$ ghci -ignore-dot-ghci
GHCi, version 8.2.0.20170404: http://www.haskell.org/ghc/  :? for help
Prelude> :load eh2
[1 of 1] Compiling Main             ( eh2.hs, interpreted ) [flags changed]
Ok, modules loaded: Main.

8.0.2

 ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.0.2
bash-3.2$ pwd
/Users/gcolpitts/haskell
bash-3.2$ ghc -dynamic -O2 eh2.hs
[1 of 1] Compiling Main             ( eh2.hs, eh2.o )
Linking eh2 ...
bash-3.2$ ghci -ignore-dot-ghci
GHCi, version 8.0.2: http://www.haskell.org/ghc/  :? for help
Prelude> :load eh2
Ok, modules loaded: Main (eh2.o).

Change History (50)

comment:1 Changed 7 months ago by George

This was on a Mac with 8.2.1-rc1 compiled from source. Setting OS and architecture to Unknown/Multiple as I don't see any reason why this would be Mac specific. See also #13002

Last edited 7 months ago by George (previous) (diff)

comment:2 Changed 7 months ago by George

Version: 8.0.18.2.1-rc1

comment:3 Changed 7 months ago by George

Summary: regression in ghc 8.2.1-rc1 (8.2.0.2017040)regression in ghc 8.2.1-rc1 (8.2.0.20170404)

comment:4 Changed 7 months ago by RyanGlScott

I wonder if this bug has the same underlying cause as #13500? The [flags changed] part makes me suspect so.

comment:5 Changed 7 months ago by RyanGlScott

Ah, never mind. I tried using GHC HEAD with Phab:D3398 (the proposed fix for #13500) applied, but this bug was still present.

comment:6 Changed 7 months ago by RyanGlScott

Cc: ezyang added

I've identified commit 818760d68c0e5e4479a4f64fc863303ff5f23a3a (Fix #10923 by fingerprinting optimization level) as the culprit. cc'ing ezyang, the author of that commit.

comment:7 Changed 7 months ago by ezyang

Makes sense: GHCi asks for no optimization, but the object files are optimized, so GHCi has no choice but to reinterpret the files.

It would be an easy matter to revert this patch, but if we also want to keep #10923 fixed, we will need to have a discussion about what the intended semantics here are.

comment:8 Changed 7 months ago by George

Suppose I wanted ghci to load optimized files, specifically -O2, how would I do that? See #13002 as mentioned above. Assuming I could specify it then ghci would simply load the compiled file, right?

Currently there is no way to have a file compiled with -O2 loaded or compiled and loaded into ghci, right? That is a deficency and a regression. right?

Last edited 7 months ago by George (previous) (diff)

comment:9 Changed 7 months ago by ezyang

In principle, one ought to be able to pass -O2 to GHCi to make this happen. However, it turns out you also must pass -fobject-code, at which the desired behavior is seen.

ezyang@sabre:~$ ghc-8.2 -O2 -c A.hs
ezyang@sabre:~$ ghc-8.2 -O2 -c A.hs -dynamic
ezyang@sabre:~$ ghc-8.2 --interactive -O2

when making flags consistent: warning:
    -O conflicts with --interactive; -O ignored.
GHCi, version 8.2.0.20170413: http://www.haskell.org/ghc/  :? for help
Prelude> :load A.hs
[1 of 1] Compiling A                ( A.hs, interpreted ) [flags changed]
Ok, modules loaded: A.
*A> 
Leaving GHCi.
ezyang@sabre:~$ ghc-8.2 --interactive -O2 -fobject-code
GHCi, version 8.2.0.20170413: http://www.haskell.org/ghc/  :? for help
Prelude> :load A.hs
Ok, modules loaded: A (A.o).
Prelude A> 

comment:10 Changed 7 months ago by George

Thanks, that answers my question, you can do it as you described. Also you can specify those options in your .ghci file and that works as would be expected.

So the question seems to be, if you have an object file compiled with flags different than the ghci flags (possibly the default ones) should ghci load it (as it did in 8.0.2) or should it compile the source into interpreted byte code and load that (as it does now)

I think I prefer the old behavior, if you want interpreted byte code, remove the object file otherwise load will load your object file as is. However if we want to change this behavior to what it is currently I could live with that as long as the documentation is clear about the change and the current behavior.

comment:11 Changed 7 months ago by ezyang

So, the motivating principle for the change in behavior for ghc --make is that, if you run a ghc command, the end result should always be the same as if you had run GHC on a clean project. This means that, yes, if the optimization level changed, you better recompile your files.

I don't think this is necessarily what GHCi users are looking for. Without -fobject-code, optimization level doesn't matter at all and I can definitely see an argument for the semantics, "Please ignore my flags and use the on-disk compiled products as much as possible, so long as they accurately reflect the source code of my program." This interpretation is supported by the fact that -O flag doesn't do anything right now. (But, it is probable that -fobject-code should shunt us back into the --make style semantics.)

comment:12 Changed 7 months ago by simonpj

Whether or not it is the "right" behaviour, it would be good if the user manual documented the behaviour and the underlying principles. And describes how to work around it if you want something different.

comment:13 Changed 7 months ago by ezyang

Differential Rev(s): Phab:D3514

I don't think a proper fix can make it for 8.2, so I've put up a revert for review.

comment:14 Changed 7 months ago by rwbarton

Summary: regression in ghc 8.2.1-rc1 (8.2.0.20170404)ghci no longer loads dynamic .o files by default if they were built with -O

comment:15 Changed 7 months ago by bgamari

Frankly, I've been surprised by the converse of this bug: GHC doesn't recompile when I request different optimization options. I can see the argument for a "go fast" mode, which uses any existing build products to avoid recompilation if at all possible, but it's not clear to me that this should be the default behavior.

comment:16 Changed 7 months ago by George

I think I will be happy with whatever people come up with here. I filed the bug originally as I was surprised by the change in behavior. As Simon wrote above: it would be good if the user manual documented the behaviour and the underlying principles. And describes how to work around it if you want something different.

As Ben wrote above ideally the final fix will also address the converse of this bug.

comment:17 Changed 4 months ago by George

Milestone: 8.4.1

By 8.4.1, as Simon says in comment 12: it would be good if [at least] the user manual documented the behaviour and the underlying principles. And describes how to work around it if you want something different.

comment:18 Changed 4 months ago by elaforge

I just ran into this issue myself. The main thing I have to add is to note that this issue also affects -fhpc, so whatever solution should take that into account too. I took a look through FlagChecker.hs for other candidates, but didn't see anything else.

A simple fix would be for ghci to note that -fhpc or -O are being ignored, but still include them in its flag hash when it does the recompilation check. I didn't notice that suggestion in the discussion above.

comment:19 Changed 4 months ago by elaforge

By the way, if the fix is really put off until 8.4.1, it means I'll have to skip a whole major version! This issue is a show stopper for my program, and I'd expect for any program that uses the GHC API for a REPL (maybe not that many, but still, it's a compelling GHC feature for me). It also makes interactive testing really slow, which for me makes it a show stopper for my style of development as well.

If people like the "ghci allows but ignores -fhpc and -O" idea, I'd be happy to give the implementation a shot.

comment:20 Changed 4 months ago by George

I like the "ghci allows but ignores -fhpc and -O" idea and I'd like to see this in 8.2.2. As my original summary wrote this is a regression and I'd like to see it resolved sooner rather than later. However see also https://ghc.haskell.org/trac/ghc/ticket/13002, not sure if "ghci allows but ignores -fhpc and -O" means that 13002 would not get resolved.

Last edited 4 months ago by George (previous) (diff)

comment:21 Changed 4 months ago by bgamari

Component: GHCiCompiler
Milestone: 8.4.18.2.2
Priority: normalhigh

We can fix the checker for 8.2.2.

comment:22 Changed 2 months ago by bgamari

Keywords: RecompilationCheck added

comment:23 Changed 2 months ago by bgamari

elaforge, George, can you describe precisely what you would propose that you help with this? I'm not sure teaching ghci to ignore -O and -fhpc is a great idea since there may be users that want to use these flags from within an interactive session.

comment:24 Changed 2 months ago by George

I'll settle for what Simon suggested: " if the user manual documented the behaviour and the underlying principles. And describes how to work around it if you want something different." Whatever the solution, I'd like to be able to specify the optimization level of compilation, e.g. -O or -02 in a .ghci file as well as by an argument to ghci, so that it will take effect when I compile inside of emacs. i.e. in other words I'd like #13002 fixed too, if possible. The use case here is working with optimized compiled code in emacs/ghci, making changes and measuring the performance; thus you want to be able to compile those changes in emacs at a given optimization level.

I think elaforge may want something more specific but I'll let him speak for himself.

Last edited 2 months ago by bgamari (previous) (diff)

comment:25 Changed 2 months ago by bgamari

Owner: set to dfeuer

David is taking care of this one.

comment:26 Changed 8 weeks ago by elaforge

Sorry about the delay, I guess trac doesn't email me when tickets are updated.

The desired end result is that I have a bunch of .o files compiled with -O, and I need to load them into the GHC API. Similarly, I have a set of .o files compiled with -fhpc, and I need to load them into ghci. Any solution that reaches that result will probably work for me!

[ bgamari ]

I'm not sure teaching ghci to ignore -O and -fhpc is a great idea since there may be users that want to use these flags from within an interactive session.

I don't understand this, ghci already ignores -O and -fhpc, and as far as I know always has, whether or not people want to use them. So the request is to continue ignoring those flags as always, but to be able to load files compiled with them... which presumably means include them in the hash.

comment:27 Changed 6 weeks ago by dfeuer

What we really want, I think, is for users to be able to specify (globally, and perhaps also for individual loaded files) whether they want extra-aggressive recompilation avoidance. That would include optimization level and HPC (is that covered under the prof bit?), and perhaps other profiling options.

comment:28 Changed 6 weeks ago by dfeuer

I guess this probably means teasing apart fingerprints that are currently merged, recording these options separately.

comment:29 Changed 6 weeks ago by bgamari

Priority: highhighest

comment:30 Changed 5 weeks ago by dfeuer

Some summary

The essence of this ticket is explained quite well by Edward in comment:11. I don't think I agree with Edward about how the interpretation of -fobject-code should play into everything, but his essential points stand:

  1. We want to be able to run ghc --make and be sure that we get compilation products entirely equivalent to compiling from scratch.
  1. We want ghci (especially) to be able to load -dynamic-compiled modules even if those modules were compiled with slightly different options.

The question of what "slightly different" means is really up to the user.

The solution

Fortunately, it looks like this is probably not hard! Currently, we use fingerprintDynFlags to calculate a fingerprint of all the dflags that it believes can affect the compilation result and (in addFingerprints) record that fingerprint in the ModIface. When we are compiling with flags that don't match, we recompile the dependencies. What we want to do, I believe, is record not only the fingerprint but also information about some of the individual options.

Some thoughts:

  1. A change in whether cost center profiling is enabled gopt Opt_SccProfilingOn dflags absolutely mandates recompilation. I believe we want users to be able to (selectively) ignore changes to
  • -O
  • -fhpc
  • -fignore-asserts
  • Automatic cost-center insertion (-fprof-...)

I think we can do this by using one fingerprint for each of these options, or, even simpler, for each option and each module, either "This module and its dependencies use value X" or "At least one dependency uses a different value than this module".

  1. I believe we're currently somewhat too conservative about language flags in general. For example, I wouldn't expect enabling -X + DataKinds, AllowAmbiguousTypes, ExplicitNamespaces, ConstraintKinds, MultiParamTypeClasses, FunctionalDependencies, FlexibleInstances, FlexibleContexts, UndecidableInstances, TupleSections, TypeSynonymInstances, StandaloneDeriving, DefaultSignatures, NullaryTypeClasses, EmptyCase, MultiWayIf, or ConstrainedClassMethods to be able to change the result of compiling any module that was successfully compiled before. For these options, we're really only interested if one of them is turned off that was previously turned on. For these, rather than a proper fingerprint, we want to record, for each option and each module, whether the module or at least one of its dependencies was compiled with that flag.
  1. We should consider fingerprinting the result of running the preprocessor(s) over the source. If the -D, -U, or -I options change, or an #included file changes, we only need to recompile if the results of preprocessing have actually changed.

comment:31 Changed 5 weeks ago by George

I trust that this will be a good solution but I think it would be worthwhile to provide a draft of how this will be documented in the GHC user's guide so that end users can understand, at that level, what they will be getting with this fix.

comment:32 Changed 5 weeks ago by bgamari

Yes, having a description of what this will look like from the users' perspective would help us ascertain whether or not this will address the issue.

Once we have that perhaps elaforge could also comment.

comment:33 Changed 5 weeks ago by George

Cc: elaforge added

comment:34 Changed 5 weeks ago by dfeuer

I'm working on the separate optimization level tracking. I'll probably have that done by the end of the day. I'll likely need a bit of help to get the user interface sorted. I'm not sure how that should look. Maybe a separate -frecompile-for-opt-level-change? By the way, as far as I can tell, we don't track changes in individual optimization flags, like -ffull-laziness. I imagine we should do something about that.

comment:35 Changed 5 weeks ago by dfeuer

One flag we should ideally treat specially (at some point) is -fignore-interface-pragmas. If the importing module uses that pragma, we can be much more aggressive about recompilation avoidance. In particular, if we don't already do this, we should really produce two interface hashes, one of which ignores interface pragmas. That way we won't recompile a module just because the pieces it's explicitly said it doesn't care about have changed.

comment:36 Changed 4 weeks ago by dfeuer

Differential Rev(s): Phab:D3514Phab:D4123
Status: newpatch

comment:37 Changed 4 weeks ago by dfeuer

It took me a bit to understand enough about how the fingerprinting process worked to do this right. I think the differential I just put up should fix the optimization issue. If others agree that's the right approach, it can easily be applied to -fhpc as well.

comment:38 Changed 4 weeks ago by George

So after this fix if I load a file compiled with -O2 into ghci will ghci just load it without recompiling it?

comment:39 in reply to:  38 ; Changed 4 weeks ago by dfeuer

Replying to George:

So after this fix if I load a file compiled with -O2 into ghci will ghci just load it without recompiling it?

After this fix, you'll be able to load a compiled module (including one compiled with -O or -O2) into GHCi without recompiling it if nothing substantial has changed (e.g., source files) and both of the following are true:

  1. The file was compiled with -dynamic
  2. GHCi is run with -fignore-optim-changes

The latter tells GHC that a file shouldn't be recompiled just because an "optimization flag" has changed. That's a bit of a fuzzy designation, but it includes all the flags included in -O2 and several others as well. The one that might be most surprising is -fignore-asserts. If we need to add additional flags in the future to refine the way we handle such, we can consider it.

Will this let you do what you need?

I intend to do something similar for HPC, but I haven't yet.

comment:40 Changed 4 weeks ago by elaforge

It feels like an odd approach. The situation implied by -fignore-optim-changes is that I'm not passing the same flags, but I want to load '.o's anyway. But from my point of view, I *am* passing the same flags, the problem is that ghci is filtering them out. So with that new flag, it becomes: pass the same flags, ghci filters out some making them not the same, pass another flag that says ignore when some flags are not the same. We'll need another flag that does the same thing for -fhpc (as you mention) and then possibly in the future some more to ignore any other symptoms of ghci filtering out flags. Doesn't it seem a bit convoluted? If I weren't following this thread, and ran into this problem, I'm not sure I'd be able to find all proper flags to get it to work.

Compare that to making ghci no longer change the flags you pass, even if it can't implement them: it just works, no flags needed. You could add one to suppress the warning about "ignoring some of your flags" but we have some general verbosity level stuff already.

That said, I am following this thread, so I will know about the flags, so they (once you put in one for -fhpc of course) will fix my problem. So aside from my worry that it seems overcomplicated, I'm in favor.

comment:41 Changed 4 weeks ago by dfeuer

elaforge, so you want GHCi to load everything with the optimization level you specify? The downside I see is that if you have a bunch of object code (for dependencies) but you want to be able to set breakpoints and such in the specific module you're working on right now, you're stuck; you'll have to load everything to get that module in interpreted mode. Or do you want to load the modules you list on the command line in interpreted mode and load object code for the dependencies? Or something else? I'm not sure exactly what else you want.

comment:42 in reply to:  39 Changed 4 weeks ago by George

Replying to dfeuer:

Replying to George:

So after this fix if I load a file compiled with -O2 into ghci will ghci just load it without recompiling it?

After this fix, you'll be able to load a compiled module (including one compiled with -O or -O2) into GHCi without recompiling it if nothing substantial has changed (e.g., source files) and both of the following are true:

  1. The file was compiled with -dynamic
  2. GHCi is run with -fignore-optim-changes

The latter tells GHC that a file shouldn't be recompiled just because an "optimization flag" has changed. That's a bit of a fuzzy designation, but it includes all the flags included in -O2 and several others as well. The one that might be most surprising is -fignore-asserts. If we need to add additional flags in the future to refine the way we handle such, we can consider it.

Will this let you do what you need?

Works perfectly for me. Thanks!

I intend to do something similar for HPC, but I haven't yet.

comment:43 Changed 4 weeks ago by elaforge

I don't totally understand the point about the debugger. I thought ghci always loads binary if it can? I usually use the * syntax for :load, so the current module is always interpreted, so I can see private names if there an export list. Hasn't it always been true that to set a breakpoint you have to force the module to load as bytecode, either with * or by touching it so it thinks the binary is out of date? I don't really use the debugger so I might be missing some detail.

For context, I'm loading modules in two situations: one is from command line ghci, where I'm loading test modules, which were compiled with -fhpc. I also link the modules into a test binary, which I do I want -fhpc for so I can get coverage, but when testing from ghci I don't care about that stuff, I just want it to load the binary modules, not recompile everything as bytecode every time.

The other situation is that I use the GHC API to load modules into a running application. Those modules are compiled with -O, and I use GHC.setSessionDynFlags to get the same flags used to compile them when compiling the application itself so I can load them. But the GHCI API then goes and filters out -O, making the flags different... if I'm remembering the results of my research correctly. After that, I'll give it some expressions to evaluate, or maybe reload some modules as bytecode, just like you might do in ghci. Similar to the -fhpc case, I don't actually care that the interpreted code is not optimized, I just want to load the binary modules.

My suggestion was to turn off the thing that filters the flags. Of course even if it retains -O it doesn't mean the bytecode interpreter can magically do optimizations, so it would be a bit of a lie. But it seems like the lie is not so bad. It would be optimizing if it could, and it will act as if the flag is set for the purposes of loading modules, but by its nature bytecode is not optimized, so it just doesn't apply when it compiles bytecode.

comment:44 Changed 4 weeks ago by dfeuer

elaforge, I think I understand kind of what you're asking for now, but there are some tricky questions about the UI. The biggest question is probably how to make interaction with -fobject-code sensible. In particular, we don't currently produce object code (ever) when -fobject-code is off. So if I type ghci -O2 A, where A depends on B, what should happen if B was not compiled -O2? Should we generate object code for B anyway to obey -O2? That seems a bit surprising. Should we load it interpreted? That seems inconsistent. Or perhaps we should change the interpretation of -fobject-code in a slightly different direction. What if we make :load *A guarantee that it loads A interpreted whether or not A has been compiled already and whether or not GHCi was run with -fobject-code?

comment:45 Changed 4 weeks ago by bgamari

elaforge, thoughts? I would really like to wrap this up soon so we can get 8.2.2 out. If it's not done by Monday I'm afraid we'll need to punt this to 8.4.

comment:46 Changed 4 weeks ago by elaforge

For ghci -O2 A B example, I think it should load B as bytecode if the flags don't match. It doesn't seem inconsistent to me, here are the rules:

With -fobject-code, always load binary, which means recompile (as binary) if the flags don't match.

With -fbyte-code, load binary if there already is one, and the flags match, otherwise load as bytecode. Flags that don't apply to bytecode (namely -O and -fhpc) are ignored, but do affect whether or not the flags match when loading binary.

Can you expand on how it seems inconsistent? I'm guessing that you're thinking that -O means "binary and bytecode are optimized" while I'm happy for it to mean "binary is optimized" with no implication for bytecode. I admit the case might be weaker for -fhpc, in that people might not expect that -fhpc means binary only. But I guess that's just an aspect of bytecode that it doesn't support those things, and if there's a warning that says "we're ignoring these for bytecode", as there already currently is, then it seems fine to me.

I think the only change would be to have DynFlags.makeFlagsConsistent emit the warnings, but not mutate the dflags. Of course it might then trigger assertion failures down the line, but presumably they would be easy to fix.

I just did an experiment with -prof, because presumably it's also not supported by bytedcode, but unlike -O it doesn't warn for ghci. But it looks like while it's happy to load binary modules compiled with -prof, even if you don't pass it to ghci, it will then crash trying to run things:

ghc: panic! (the 'impossible' happened)
  (GHC version 8.0.2 for x86_64-apple-darwin):
	Loading temp shared object failed: dlopen(/var/folders/9p/tb878hlx67sdym1sndy4sxf40000gn/T/ghc93652_0/libghc_1.dylib, 5): Symbol not found: _CCS_DONT_CARE
  Referenced from: /var/folders/9p/tb878hlx67sdym1sndy4sxf40000gn/T/ghc93652_0/libghc_1.dylib
  Expected in: flat namespace
 in /var/folders/9p/tb878hlx67sdym1sndy4sxf40000gn/T/ghc93652_0/libghc_1.dylib

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

Maybe I should file this as a separate bug.

Last edited 4 weeks ago by elaforge (previous) (diff)

comment:47 Changed 4 weeks ago by bgamari

Maybe I should file this as a separate bug.

elaforge, yes, please do.

comment:48 Changed 4 weeks ago by dfeuer

elaforge, I have an idea that feels like it provides a reasonably consistent UI, but I'd like to see what you think.

  1. Optimization flags (including -O0) imply -fobject-code. This ensures that GHC respects optimization flags regardless of --interactive.
  1. Even when -fobject-code is on, :load *M will load M as bytecode. This provides the "escape hatch" from -fobject-code that you need to use debugging features, etc.
  1. We add -fignore-optim-changes and -fignore-hpc-changes (​Phab:D4123), enabling users to put together object code and bytecode with diverse optimization levels and HPC info while still updating automatically based on source changes and whether profiling is enabled.

comment:49 Changed 4 weeks ago by bgamari

Milestone: 8.2.28.2.3

Alright, I'm afraid we are going to have to punt on this for 8.2.2. Sorry elaforge!

comment:50 Changed 3 weeks ago by elaforge

bgamari: No problem, I understand about release schedules. I'm sorry to drag it out a bit, but on the other hand it's good to be careful about flag design since it's one of those APIs that is hard to fix later.

I'll copy paste this in the mailing list thread, just so there's a record in both places.

I still don't feel like 1 is necessary, I'd rather flags cause other flags to be ignored with a warning, rather than turn on other flags. But that's just a vague preference, with no strong evidence for it. Maybe it could emit a warning if you didn't put -fobject-code in explicitly, e.g. "-O implies -fobject-code, adding that flag." So as long as we accept 1, then 2 and 3 follow naturally. Given that, I support this UI.

Thanks for looking into it!

Note: See TracTickets for help on using tickets.