crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Compiler: enable parallel codegen with MT

Open ysbaddaden opened this issue 1 year ago • 1 comments

Implements parallel codegen of object files when MT is enabled in the compiler (-Dpreview_mt).

It only impacts codegen for compilations with more than one compilation unit (module), that is when neither of --single-module, --release or --cross-compile is specified. This behavior is identical to the fork based codegen.

Advantages:

  • allows parallel codegen on Windows (untested);
  • no need to fork many processes;
  • no repeated GC collections in each forked processes;
  • a simple Channel to distribute work efficiently (no need for IPC).

The main points are increased portability and simpler logic, despite having to take care of LLVM thread safety quirks (see comments).

Issues:

  1. The threads arg actually depicts the number of fibers, not threads, which is confusing and problematic: increasing threads but not CRYSTAL_WORKERS will lead to more fibers than threads, with fibers being sheduled on the same threads, which won't bring any improvement.

    In fact CRYSTAL_WORKERS defaults to 4, when threads defaulted to 8. With this patch it defaults to CRYSTAL_WORKERS, so MT can end up being slower if we don't specify CRYSTAL_WORKERS=8.

  2. This is still not as efficient as it could be. The main fiber (that feeds the worker fibers) can get blocked by a worker fiber doing codegen, leading the other workers to starve. This is easily noticeable when compiling with -O1 for example.

Both issues will be fixable with RFC 2 where we can start an explicit context to run the worker fibers or start N isolated contexts (maybe a better idea). Until then, one should increase CRYSTAL_WORKERS.

Supersedes #14227 and doesn't segfault (so far) with LLVM 18.1 :crossed_fingers:

TODO:

  • [ ] wait for #14760
  • [ ] cleanup
  • [ ] rename the method as mt_parallel(units, n_threads)
  • [ ] consider increasing the channel size (until we can use ExecutionContext)
  • [ ] consider a CRYSTAL_CONFIG_WORKERS to configure the default number of workers at compile time instead of the hardcoded 4 (in a distinct PR)

ysbaddaden avatar Jun 25 '24 09:06 ysbaddaden

This looks great. But it also seems to be a mix of different changes. Could we extract the independent refactorings (such as extracting sequential_codegen and fork_codegen, memoization of some methods) to their own PRs?

straight-shoota avatar Jun 25 '24 22:06 straight-shoota

Rebased on top of #14760.

ysbaddaden avatar Jul 02 '24 11:07 ysbaddaden

Rebased from master that merged #14760 (prerequisite) and ready for review.

ysbaddaden avatar Sep 03 '24 09:09 ysbaddaden

consider a CRYSTAL_CONFIG_WORKERS to configure the default number of workers at compile time instead of the hardcoded 4 (in a distinct PR)

Isn't this addressed (using CRYSTAL_WORKERS)? Or is it something different?

beta-ziliani avatar Sep 03 '24 11:09 beta-ziliani

@beta-ziliani this could be a compile time ENV to change the default number of threads/schedulers. It's tangential to this pull request.

ysbaddaden avatar Sep 03 '24 13:09 ysbaddaden

The RFC-2 link in the OP here points to a non-existing URL. Think it was supposed to be https://github.com/crystal-lang/rfcs/pull/2?

Fryguy avatar Sep 19 '24 19:09 Fryguy

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/why-cant-use-multi-core-when-compile-an-application-use-crystal-compiler/7435/2

crysbot avatar Nov 25 '24 17:11 crysbot

Hi, i didn't understand, can i know how to use this new feature in 1.14.0?

  1. Should we build compiler itself with -Dpreview_mt then this feature will be enabled.
  2. Or we still built compiler as usual, but if enable -Dpreview_mt when build app, will enable this feature?

Thanks

zw963 avatar Nov 25 '24 17:11 zw963

  1. Yes.
  2. No.

ysbaddaden avatar Nov 25 '24 19:11 ysbaddaden

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/why-cant-use-multi-core-when-compile-an-application-use-crystal-compiler/7435/6

crysbot avatar Nov 25 '24 20:11 crysbot

Hi, i do some test, it's seem like no any performance improvement, following is reproduce:

  1. install crystal prebuilt 1.14.0 version use asdf
 ╰──➤ $ cr version
Crystal 1.14.0 [dacd97bcc] (2024-10-09)

LLVM: 18.1.6
Default target: x86_64-unknown-linux-gnu
  1. built crystal compiler myself which preview_mt enabled
FLAGS='--no-debug -Dpreview_mt' LDFLAGS='-s' make crystal
 ╰──➤ $ cr version
Crystal 1.14.0 [dacd97bcc] (2024-10-09)

LLVM: 18.1.8
Default target: x86_64-pc-linux-gnu
  1. Copy the same project twice, set one of them use 1, another one use 2.

  2. Always delete all cached files in ~/.cache/crystal before do following every step.

  3. built use 1, as following:

 ╰──➤ $ time crystal build src/procodile.cr

real    0m2.872s
user    0m5.200s
sys     0m1.399s
  1. built use 2, as following:
 ╰──➤ $ export CRYSTAL_WORKERS=4

 ╰──➤ $ time crystal build src/procodile.cr

real    0m3.223s
user    0m5.126s
sys     0m0.928s

The latter even slower, did I do something wrong?

Thanks.

zw963 avatar Nov 26 '24 05:11 zw963

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/why-cant-use-multi-core-when-compile-an-application-use-crystal-compiler/7435/8

crysbot avatar Nov 26 '24 05:11 crysbot

@zw963 From the OP:

In fact CRYSTAL_WORKERS defaults to 4, when threads defaulted to 8. With this patch it defaults to CRYSTAL_WORKERS, so MT can end up being slower if we don't specify CRYSTAL_WORKERS=8.

Sija avatar Nov 26 '24 05:11 Sija

Okay, i saw a few performance improve when try to build one of my web project.

old:

 ╰──➤ $ time crystal build src/college.cr

real    0m43.939s
user    0m48.787s
sys     0m8.349s

new:

 ╰──➤ $ CRYSTAL_WORKERS=8 time crystal build src/college.cr

real    0m39.098s
user    0m48.551s
sys     0m4.828s

Time has been reduced by about 10%. the reduced time almost come from the sys, I guess the project more larger, the effects more obviously.

BTW: Not see multi-core be used even parallel codegen enabled, maybe this stage is very quickly, there is no chance to see it in htop?

zw963 avatar Nov 26 '24 06:11 zw963