snowleopard / hadrian

Hadrian: a new build system for the Glasgow Haskell Compiler. Now merged into the GHC tree!
https://gitlab.haskell.org/ghc/ghc/tree/master/hadrian
MIT License
208 stars 37 forks source link

Hadrian 7x slower than make for changing base C files #464

Closed nh2 closed 6 years ago

nh2 commented 6 years ago

On Linux, ghc/ghc@07ac921f48baea84b40835b0b7c476806f7f63f6, when I change inputReady.c and recompile with Hadrian:

Build completed in 0:42m

hadrian/build.stack.sh -j --flavour=quickest inplace/bin/ghc-stage2  96.30s user 7.18s system 243% cpu 42.572 total

With make on the quick flavour:

make -j4 inplace/bin/ghc-stage2  6.09s user 1.13s system 113% cpu 6.378 total

Here make is 7x faster and makes iterating on the RTS much more pleasant.

The difference is that make seems to recompile only the changes while Hadrian seems to recompile all C files in the RTS.

simonmar commented 6 years ago

With the make build system I normally use cd rts; make fast -j8 to get really fast rebuilds, I really hope Hadrian can support that. It's a similar problem to make 2 and the other workflows we have for building a subset of the targets.

nh2 commented 6 years ago

Fixed issue description; inputReady.c isn't actually in rts/, it's a C file in libraries/base.

I should also add, recompiling the same change this way is another 2x faster on make:

cd ghc
make -j4 2 FAST=YES  4.22s user 0.78s system 151% cpu 3.310 total
snowleopard commented 6 years ago

@nh2 Thanks for reporting!

As far as I understand, Hadrian rebuilds the right set of files (they all depend on the changes you made), but Make skips some of the rebuilds because of additional modes for building only subsets of files. Can you confirm that?

With the make build system I normally use cd rts; make fast -j8 to get really fast rebuilds, I really hope Hadrian can support that.

@simonmar Yes, we can support something like that. Would you like Hadrian to detect the current directory and make decisions based on it, just like Make? This sounds a bit fragile to me. What about adding a flag, for example --only=rts, that skips rebuilds everywhere outside a particular package?

Both approaches are not difficult to implement, but the second one seems a better fit for Hadrian.

bgamari commented 6 years ago

As far as I understand, Hadrian rebuilds the right set of files (they all depend on the changes you made),

I'm not sure that this is true. Why should we need to recompile transformers after changing base/cbits/inputReady.c? I would have thought that at most we would need to re-link.

simonmar commented 6 years ago

What about adding a flag, for example --only=rts, that skips rebuilds everywhere outside a particular package?

I don't mind what the flag is, provided we have a way to do this. Things that I commonly want to rebuild using make FAST=YES stage=0 or make fast:

snowleopard commented 6 years ago

@bgamari Hmm, this may indeed be a bug then. I'll take care of this.

@simonmar Thanks! I've created a separate issue for this: #472.

snowleopard commented 6 years ago

@nh2 I've pushed a fix. Now if I edit inputReady.c then only the base library and executables are reuilt.

Could you please try again?

nh2 commented 6 years ago

@snowleopard When upgrading and trying again, I got this:

% hadrian/build.stack.sh -j --flavour=quickest inplace/bin/ghc-stage2
shakeArgsWith    0.000s    0%                           
Function shake   0.185s    0%                           
Database read    0.001s    0%                           
With database    0.000s    0%                           
Running rules  156.680s   99%  =========================
Total          156.866s  100%                           
Error when running Shake build system:
* inplace/bin/ghc-stage2
* inplace/lib/bin/ghc-stage2
* OracleQ (KeyValue ("_build/stage1/libraries/array/package-data.mk","BUILD_GHCI_LIB"))
* _build/stage1/libraries/array/package-data.mk
* _build/stage1/libraries/array/package-data.mk _build/stage1/libraries/array/setup-config
* inplace/lib/package.conf.d/ghc-prim-0.5.2.0.conf
* _build/stage1/libraries/ghc-prim/inplace-pkg-config
* _build/stage1/libraries/ghc-prim/package-data.mk
* _build/stage1/libraries/ghc-prim/package-data.mk _build/stage1/libraries/ghc-prim/setup-config
* inplace/bin/ghc-stage1
* inplace/lib/bin/ghc-stage1
* _build/stage0/libraries/ghci/libHSghci-8.3.a
* _build/stage0/libraries/ghci/SizedSeq.o
* _build/stage0/libraries/ghci/SizedSeq.o _build/stage0/libraries/ghci/SizedSeq.hi
* OracleQ (KeyValues ("_build/stage0/libraries/ghci/.dependencies","_build/stage0/libraries/ghci/SizedSeq.o"))
* _build/stage0/libraries/ghci/.dependencies
* _build/stage0/libraries/ghci/GHCi/FFI.hs
* _build/stage0/utils/hsc2hs/hsc2hs
user error (Development.Shake.cmd, system command failed
Command: /raid/stack/programs/x86_64-linux/ghc-8.0.2/bin/ghc -Wall -hisuf hi -osuf o -hcsuf hc -static -hide-all-packages -no-user-package-db '-package-db /home/niklas/src/ssd/ghc/_build/stage0/bootstrapping.conf' '-package-id base-4.9.1.0' '-package-id containers-0.5.7.1' '-package-id directory-1.3.0.0' '-package-id filepath-1.4.1.1' '-package-id process-1.4.3.0' -i -i_build/stage0/utils/hsc2hs -i_build/stage0/utils/hsc2hs/build/hsc2hs/autogen -iutils/hsc2hs/. -Iincludes -I_build/generated -I_build/stage0/utils/hsc2hs -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/process-1.4.3.0/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/directory-1.3.0.0/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/unix-2.7.2.1/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/time-1.6.0.1/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/bytestring-0.10.8.1/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/base-4.9.1.0/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/integer-gmp-1.0.0.1/include -I/raid/stack/programs/x86_64-linux/ghc-8.0.2/lib/ghc-8.0.2/include -I_build/generated -optc-I_build/generated -optP-include -optP_build/stage0/utils/hsc2hs/build/hsc2hs/autogen/cabal_macros.h -optc-fno-stack-protector -odir _build/stage0/utils/hsc2hs -hidir _build/stage0/utils/hsc2hs -stubdir _build/stage0/utils/hsc2hs -no-auto-link-packages -rtsopts -optc-Werror=unused-but-set-variable -optc-Wno-error=inline _build/stage0/utils/hsc2hs/Main.o _build/stage0/utils/hsc2hs/C.o _build/stage0/utils/hsc2hs/Common.o _build/stage0/utils/hsc2hs/CrossCodegen.o _build/stage0/utils/hsc2hs/DirectCodegen.o _build/stage0/utils/hsc2hs/Flags.o _build/stage0/utils/hsc2hs/HSCParser.o _build/stage0/utils/hsc2hs/UtilsCodegen.o -o _build/stage0/utils/hsc2hs/hsc2hs -O0 -H64m -XHaskell2010
Exit code: 1
Stderr:
_build/stage0/utils/hsc2hs/Main.o: In function `s41n_info':
(.text+0x77): undefined reference to `Pathszuhsc2hs_version_closure'
_build/stage0/utils/hsc2hs/Main.o: In function `s430_info':
(.text+0x3a5c): undefined reference to `Pathszuhsc2hs_getDataFileName_closure'
_build/stage0/utils/hsc2hs/Main.o: In function `S46U_srt':
(.data.rel.ro+0x640): undefined reference to `Pathszuhsc2hs_version_closure'
_build/stage0/utils/hsc2hs/Main.o: In function `S46U_srt':
(.data.rel.ro+0x810): undefined reference to `Pathszuhsc2hs_getDataFileName_closure'
collect2: error: ld returned 1 exit status
`gcc' failed in phase `Linker'. (Exit code: 1)
)

This remains even after a hadrian/build.stack.sh clean, and also when I run it without specific output target (time hadrian/build.stack.sh -j --flavour=quickest).

That is on top of commit ghc/ghc@a1950e6dc03b560105903ad44050b1570f3bc24f.

nh2 commented 6 years ago

Oops, forgot to pull submodules, will try again.

nh2 commented 6 years ago

Oops, forgot to pull submodules, will try again.

OK, I also did some git clean and noticed that I should have called hadrian with --configure (I suspect that already the latter would have sufficed). It works now so sorry for the noise.

Could you please try again?

My output:

% time hadrian/build.stack.sh -j --flavour=quickest inplace/bin/ghc-stage2
| Run Cc FindCDependencies Stage1: libraries/base/cbits/inputReady.c => _build/stage1/libraries/base/c/cbits/inputReady.o.d
| Run Ghc CompileCWithGhc Stage1: libraries/base/cbits/inputReady.c => _build/stage1/libraries/base/c/cbits/inputReady.o
| Remove file _build/stage1/libraries/base/libHSbase-4.11.0.0.a
| Run Ar Pack Stage1: _build/stage1/libraries/base/c/cbits/DarwinUtils.o (and 244 more) => _build/stage1/libraries/base/libHSbase-4.11.0.0.a
/usr/bin/ar: creating _build/stage1/libraries/base/libHSbase-4.11.0.0.a
| Run Ld: _build/stage1/libraries/base/c/cbits/DarwinUtils.o (and 244 more) => _build/stage1/libraries/base/HSbase-4.11.0.0.o
/------------------------------------------------------------\
| Successfully built library 'base' (Stage1, way v).         |
| Library: _build/stage1/libraries/base/libHSbase-4.11.0.0.a |
| Library synopsis: Basic libraries.                         |
\------------------------------------------------------------/
| Run Ghc LinkHs Stage1: _build/stage1/ghc/c/hschooks.o (and 5 more) => inplace/lib/bin/ghc-stage2
/----------------------------------------------------------\
| Successfully built program 'ghc-bin' (Stage1).           |
| Executable: inplace/lib/bin/ghc-stage2                   |
| Program synopsis: The Glorious Glasgow Haskell Compiler. |
\----------------------------------------------------------/
| Make 'inplace/bin/ghc-stage2' executable.
| Successfully created wrapper for 'ghc-bin' (Stage1).
shakeArgsWith                        0.000s    0%                           
Function shake                       0.183s    3%  =                        
Database read                        0.040s    0%                           
With database                        0.092s    2%                           
Running rules                        4.233s   92%  =========================
Pool finished (2434 threads, 4 max)  0.001s    0%                           
Lint checking                        0.046s    1%                           
Total                                4.596s  100%                           
Build completed in 0:05m

hadrian/build.stack.sh -j --flavour=quickest inplace/bin/ghc-stage2  6.06s user 0.76s system 123% cpu 5.521 total

@snowleopard Thanks a lot, it's much better now. After changing inputReady.c, rebuilding now takes only 5.5 seconds, that is almost as fast as the 3.3 seconds of a fast make.

nh2 commented 6 years ago

From my perspective, this can be closed, unless we want to tackle the remaining distance to make or the topics @simonmar mentioned as part of this ticket.

snowleopard commented 6 years ago

@nh2 That's great, many thanks for testing!

Let's close this issue. I've opened #472 to track progress with single-package builds. There are also #200 and #474 where we improve Hadrian's performance in general.

hvr commented 6 years ago

@snowleopard

Would you like Hadrian to detect the current directory and make decisions based on it, just like Make? This sounds a bit fragile to me.

Why do you consider this fragile? As you point out, CWD-sensitivity is a commonly used paradigm for make, and fwiw, cabal new-build also supports this paradigm (currently w/ package granularity; but it's planned to make it more fine-grained re CWD). Another example of a CWD-sensitive UI is Git. In general, tooling designed for Unix shells quite often is CWD sensitive.

snowleopard commented 6 years ago

@hvr I responded here: https://github.com/snowleopard/hadrian/issues/472#issuecomment-346976802.