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

Introduce CompileMonoItem DepNode #84123

Merged
merged 2 commits into from
Apr 19, 2021

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Apr 12, 2021

This is likely required for allowing efficient hot code swap support in cg_clif's jit mode. My prototype currently requires re-compiling all functions, which is both slow and uses a lot of memory as there is not support for freeing the memory used by replaced functions yet.

cc https://github.com/bjorn3/rustc_codegen_cranelift/issues/1087

@bjorn3 bjorn3 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-incr-comp Area: Incremental compilation A-cranelift Things relevant to the [future] cranelift backend labels Apr 12, 2021
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 12, 2021
@cjgillot cjgillot self-assigned this Apr 12, 2021
@cjgillot
Copy link
Contributor

Can you expand how you plan to use this DepNode? Why isn't a query not sufficient?

@bjorn3
Copy link
Member Author

bjorn3 commented Apr 12, 2021

This is similar to CompileCodegenUnit. A query has a return value. In this case it is only done for the side-effect of compiling the mono item to a cranelift_jit::JITModule stored in a static. By using a dep node directly, I can do tcx.try_mark_green and simply do nothing if it is green or use tcx.dep_graph.with_task if it is red to run it. If it was a query, the query would always run if it isn't added to the list of queries whose result can be stored on the disk. Adding it to this list would reduce performance a bit as it is unnecessary extra work. In addition the return value of a query must be serializable, so I can't switch to some way that avoids this global static in the future but instead for example returns a Cranelift Function directly as Function doesn't implement rustc_serialize::Encodable.

@petrochenkov
Copy link
Contributor

r? @wesleywiser or @davidtwco maybe

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 19, 2021

📌 Commit 21f13af has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2021
@bors
Copy link
Contributor

bors commented Apr 19, 2021

⌛ Testing commit 21f13af with merge 77ab14be6a948c8c34ff011e12caafb6a6a631d3...

@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
      Memory: 14 GB
      Boot ROM Version: VMW71.00V.13989454.B64.1906190538
      Apple ROM Info: [MS_VM_CERT/SHA1/27d66596a61c48dd3dc7216fd715126e33f59ae7]Welcome to the Virtual Machine
      SMC Version (system): 2.8f0
      Serial Number (system): VM1SsKmQg2JZ

hw.ncpu: 3
hw.byteorder: 1234
hw.memsize: 15032385536
---
[  7%] Building CXX object lib/Core/CMakeFiles/lldCore.dir/Error.cpp.o
[  8%] Building CXX object Common/CMakeFiles/lldCommon.dir/Args.cpp.o
error: Connection to server timed out
error: Connection to server timed out
make[2]: *** [lib/Core/CMakeFiles/lldCore.dir/DefinedAtom.cpp.o] Error 2
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [lib/Core/CMakeFiles/lldCore.dir/Error.cpp.o] Error 2
make[1]: *** [lib/Core/CMakeFiles/lldCore.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
error: Connection to server timed out
make[2]: *** [Common/CMakeFiles/lldCommon.dir/Args.cpp.o] Error 2
make[2]: *** Waiting for unfinished jobs....
[  9%] Building CXX object Common/CMakeFiles/lldCommon.dir/DWARF.cpp.o
clang-10: warning: argument unused during compilation: '-static-libstdc++' [-Wunused-command-line-argument]
clang-10: warning: argument unused during compilation: '-static-libstdc++' [-Wunused-command-line-argument]
clang-10: warning: argument unused during compilation: '-static-libstdc++' [-Wunused-command-line-argument]
make[1]: *** [Common/CMakeFiles/lldCommon.dir/all] Error 2
make: *** [all] Error 2
command did not execute successfully, got: exit code: 2


build script failed, must exit now', /Users/runner/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/cmake-0.1.44/src/lib.rs:885:5
 finished in 37.086 seconds
failed to run: /Users/runner/work/rust/rust/build/bootstrap/debug/bootstrap dist
Build completed unsuccessfully in 0:49:28

@bors
Copy link
Contributor

bors commented Apr 19, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 19, 2021
@bjorn3
Copy link
Member Author

bjorn3 commented Apr 19, 2021

Seems to be spurious.

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2021
Rollup of 7 pull requests

Successful merges:

 - rust-lang#84123 (Introduce CompileMonoItem DepNode)
 - rust-lang#84126 (Enable sanitizers for x86_64-unknown-linux-musl)
 - rust-lang#84168 (Lower async fn in traits.)
 - rust-lang#84256 (doc: use U+2212 for minus sign in floating-point -0.0 remarks)
 - rust-lang#84291 (fix aliasing violations in thread_local_const_init)
 - rust-lang#84313 (fix suggestion for unsized function parameters)
 - rust-lang#84330 (Remove unused footer section)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Apr 19, 2021

⌛ Testing commit 21f13af with merge 9d9c2c9...

@bors bors merged commit 817b7e0 into rust-lang:master Apr 19, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 19, 2021
@bjorn3 bjorn3 deleted the compile_mono_item_dep_node branch February 12, 2022 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cranelift Things relevant to the [future] cranelift backend A-incr-comp Area: Incremental compilation S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants