Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add missing links to Cranelift #273

Merged
merged 3 commits into from
May 17, 2023
Merged

add missing links to Cranelift #273

merged 3 commits into from
May 17, 2023

Conversation

mwcz
Copy link
Contributor

@mwcz mwcz commented Apr 10, 2023

I noticed these links were missing when reading the docs. I took my best guess at which page they're supposed to point to:

  • "Cranelift" -> The cranelift repo
  • "in Rust" -> the moz hacks article about Cranelift in Rust

@@ -6,7 +6,7 @@ The first step in addressing this is to figure out whether your app needs export

The next step is pretty trivial: `rustup override set nightly`. This command will set the default toolchain in your project (you have to run that command in your project directory) to `nightly`, which tends to nearly cut compile times in half! In general, the nightly compiler will be much faster than the stable one, because it uses all sorts of unstable features that improve compilation times. If you want maximum assurances though, switch back to the stable compiler before running `perseus deploy` so your production app is secured against any nightly compiler bugs (which do happen) and you get the best of both worlds.

From here, we get a little more radical. There's something called [Cranelift](), a compiler backend that can be used [for Rust]() to speed up development compilation times. Of course, the sacrifice here is runtime speed, but as long as you build with the usual compiler for production, you again get the best of both worlds. To set this up, you'll need to download the latest version of [this project](https://github.com/bjorn3/rustc_codegen_cranelift) from [here](https://github.com/bjorn3/rustc_codegen_cranelift/actions?query=branch%3Amaster+event%3Apush+is%3Asuccess) (click on the most recent action run, and then download the appropriate artifact). Then, unzip that (twice, there's a nested zip folder) and you'll get a `build/` folder, which you can put anywhere on your system. Then, add the contents of that (which should contain `cargo-clif` and `rustc-clif`) to your system's `PATH` (tutorial [for Windows](https://stackoverflow.com/a/44272417), and [for Linux/MacOS](https://linuxize.com/post/how-to-add-directory-to-path-in-linux/)).
From here, we get a little more radical. There's something called [Cranelift](https://github.com/bytecodealliance/wasmtime/tree/main/cranelift#readme), a compiler backend that can be used [for Rust](https://hacks.mozilla.org/2020/10/a-new-backend-for-cranelift-part-1-instruction-selection/) to speed up development compilation times. Of course, the sacrifice here is runtime speed, but as long as you build with the usual compiler for production, you again get the best of both worlds. To set this up, you'll need to download the latest version of [this project](https://github.com/bjorn3/rustc_codegen_cranelift) from [here](https://github.com/bjorn3/rustc_codegen_cranelift/actions?query=branch%3Amaster+event%3Apush+is%3Asuccess) (click on the most recent action run, and then download the appropriate artifact). Then, unzip that (twice, there's a nested zip folder) and you'll get a `build/` folder, which you can put anywhere on your system. Then, add the contents of that (which should contain `cargo-clif` and `rustc-clif`) to your system's `PATH` (tutorial [for Windows](https://stackoverflow.com/a/44272417), and [for Linux/MacOS](https://linuxize.com/post/how-to-add-directory-to-path-in-linux/)).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copying my comment on the commit introducing the original sentence here:

7829c74#r108264759

Since a couple of weeks I'm now releasing the lastest version on the master branch at https://github.com/bjorn3/rustc_codegen_cranelift/releases/tag/dev This saves a click and ensures that only the latest version that passes all tests is picked. In addition it should work even if you aren't logged in on github, unlike the GHA artifacts download links.

Maybe it would make sense to change the link in this PR?

Copy link
Contributor Author

@mwcz mwcz Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "this project" link should be kept, but the "from here" link that currently points to the github actions page could be changed to point to the release page for the dev tag. (and the instructions updated for this change)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjorn3 Happy to, but could you provide the rewritten instructions? I can try rewriting them but the one thing that gave me pause was the release archives have two copies of the binaries, one copy at the root and one inside bin/. Actually, it would probably be nice if the release install instructions lived in the rustc_codegen_cranelift repo and perseus could just link to that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The copy in bin/ also exists in the GHA artifacts now. The copy in the root is for user convenience, while the copy in bin/ is for tools that expect the compiler to be in bin/ of the compiler sysroot. I can add install instructions to the readme of cg_clif.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, when those instructions are in place I'll update this PR.

Copy link

@bjorn3 bjorn3 Apr 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think the following would work? If so I will push it in a couple of hours.

From 215dd7aa0d70cf7396100715326a37c81b06f056 Mon Sep 17 00:00:00 2001
From: bjorn3 <[email protected]>
Date: Sat, 29 Apr 2023 13:31:06 +0000
Subject: [PATCH] Add some extra instructions for using the precompiled builds

---
 Readme.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Readme.md b/Readme.md
index c5222982..34801dd7 100644
--- a/Readme.md
+++ b/Readme.md
@@ -22,7 +22,12 @@ $ ./test.sh
 
 For more docs on how to build and test see [build_system/usage.txt](build_system/usage.txt) or the help message of `./y.rs`.
 
+## Precompiled builds
+
 Alternatively you can download a pre built version from the [releases] page.
+Extract the `dist` directory in the archive anywhere you want.
+If you want to use `cargo clif build` instead of having to specify the full path to the `cargo-clif` executable, you can add the `bin` subdirectory of the extracted `dist` directory to your `PATH`.
+(tutorial [for Windows](https://stackoverflow.com/a/44272417), and [for Linux/MacOS](https://unix.stackexchange.com/questions/26047/how-to-correctly-add-a-path-to-path/26059#26059)).
 
 [releases]: https://github.com/bjorn3/rustc_codegen_cranelift/releases/tag/dev
 
-- 
2.34.1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arctic-hen7
Copy link
Member

Is this good to merge yet, or is there still more to do?

@mwcz
Copy link
Contributor Author

mwcz commented May 7, 2023

I still need to update the PR to link to cargo-clif's install instructions, then it should be good to go.

@arctic-hen7
Copy link
Member

Understood, could you maybe convert this to a draft until then?

@mwcz
Copy link
Contributor Author

mwcz commented May 15, 2023

Updated. I noticed I had missed updating the same text in the next docs, so I updated that one too.

Copy link
Member

@arctic-hen7 arctic-hen7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks so much! This is all ready to merge now @mwcz?

@mwcz
Copy link
Contributor Author

mwcz commented May 16, 2023

@arctic-hen7 If @bjorn3 is happy, I'm happy. :D Good to go.

@arctic-hen7 arctic-hen7 merged commit df922df into framesurge:main May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants