Use ccache for emscripten library build #25392
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,6 +142,10 @@ commands: | |
| cd ~/emsdk | ||
| ./emsdk install tot | ||
| ./emsdk activate tot | ||
| # Write the version of clang into a file for use in the ccache key | ||
| ./upstream/bin/clang --version > clang_version.txt | ||
| echo "clang version:" | ||
| cat ~/emsdk/clang_version.txt | ||
| # Remove the emsdk version of emscripten to save space in the | ||
| # persistent workspace and to avoid any confusion with the version | ||
| # we are trying to test. | ||
|
|
@@ -171,7 +175,7 @@ commands: | |
| command: | | ||
| ./emcc --clear-cache | ||
| - pip-install | ||
| - run: apt-get install -q -y ninja-build ccache | ||
| - run: | ||
| name: embuilder build ALL | ||
| command: | | ||
|
|
@@ -196,6 +200,12 @@ commands: | |
| command: | | ||
| ./embuilder build MINIMAL --pic --lto | ||
| ./test/runner test_hello_world | ||
| - run: | ||
| name: "Ccache stats" | ||
| command: | | ||
| ccache --print-stats | ||
| ccache --zero-stats | ||
|
|
||
| persist: | ||
| description: "Persist the emsdk, libraries, and engines" | ||
| steps: | ||
|
|
@@ -209,6 +219,18 @@ commands: | |
| - vms/ | ||
| - wasi-sdk/ | ||
| - .jsvu/ | ||
| - when: | ||
| condition: | ||
| and: | ||
| - equal: [ "main", << pipeline.git.branch >> ] | ||
| - equal: [ "https://github.com/emscripten-core/emscripten", << pipeline.project.git_url >> ] | ||
| steps: | ||
| - save_cache: | ||
| name: "Save Ccache cache" | ||
| paths: | ||
| - ~/.ccache | ||
| key: clang-{{ checksum "~/emsdk/clang_version.txt" }} | ||
|
|
||
| prepare-for-tests: | ||
| description: "Setup emscripten tests" | ||
| steps: | ||
|
|
@@ -543,6 +565,7 @@ jobs: | |
| environment: | ||
| EMCC_CORES: 16 | ||
| EMCC_USE_NINJA: 1 | ||
| EM_COMPILER_WRAPPER: "ccache" | ||
|
Collaborator
Member
Author
There was a problem hiding this comment. Yeah I saw that but I don't really see why it's necessary. I figured I'd try the easy thing first, and if it looks like it works, dig more into that. It may be that since we just want to use it for library compilation, it's enough. @juj can you say more about why you wanted ccache to wrap the entire emscripten driver rather than just the underlying clang?
Collaborator
There was a problem hiding this comment. I think @dschuff is trying to integrate ccache in another way: in the backend between emcc and clang. My emsdk support is placed in Would be fantastic to see what the performance difference of this approach ends up being.
Member
Author
There was a problem hiding this comment. Performance-wise, the main difference would obviously be that we still have to run all the python code of the driver. Since this is a compile and not a link, there's not a huge amount of stuff that gets done, but certainly there's a little cost. Probably the builtin profiling support could estimate how much. But mostly I just picked this way because I didn't want to bother with a fork of ccache.
Collaborator
There was a problem hiding this comment. The reason was performance. Then it would also work for final link, and e.g. wrap over binaryen invocations of wasm-opt and so on. However I have to say that in my approach, I found performance gains to be very small, so we didn't end up deploying it at Unity. I do have an itch to re-try though, and see if I could optimize the ccache implementation.
Collaborator
There was a problem hiding this comment. BTW, @juj, we are trying this out as way to speed up our CI here in circleci. Currently there is no caching so each needs to build everything from scratch. We are looking at some kind of shared cache in combination with BTW, how do your building handle LLVM changes? I guess you somehow clobber the build when llvm changes?
Member
Author
There was a problem hiding this comment. Ah, I was under the impression that ccache doesn't work for linking or other cases that aren't basically just source -> object file. It just falls back to the underlying compiler.
Collaborator
There was a problem hiding this comment.
My ccache port looked at git hash if it was detected (developer installation), or if not, then emscripten-version.txt. Additionally the contents of EM_CONFIG was hashed into the state. The implementation can be seen here: ccache/ccache@master...juj:ccache:emscripten
Err actually, now that I scan through my fork, I think you are right: ccache does not cache link commands, only compile commands. So my fork didn't end up helping cache any wasm-opt calls either. I haven't worked on the ccache fork in a while now - it looks like something has changed in CMake that it does not build with CMake >= 4 anymore. So it would need some freshening to bring up to speed again.
Member
Author
There was a problem hiding this comment. My plan is to just key on the git version of clang or the emsdk installed by CI. Since I'm just trying to cache clang's output and not emscripten's, I think everything should be included in the clang version, the flags, the file inputs (i.e. the headers and sources).
Member
Author
There was a problem hiding this comment. I don't currently actually know whether ccache automatically takes the compiler version into account or not, but in order to have CircleCI automatically save and restore the cache across builds, I have to give it a cache key anyway. |
||
| steps: | ||
| - checkout | ||
| - run: | ||
|
|
@@ -570,6 +593,14 @@ jobs: | |
| tar xvf wasi-sysroot-11.0.tar.gz -C ~/wasi-sdk/ | ||
| - install-v8 | ||
| - install-emsdk | ||
| - when: | ||
| condition: | ||
| not: | ||
| equal: [ "main", << pipeline.git.branch >> ] | ||
| steps: | ||
| - restore_cache: | ||
| name: "Restore Ccache cache" | ||
| key: clang-{{ checksum "~/emsdk/clang_version.txt" }} | ||
| - build-libs | ||
| - run: | ||
| name: Clean build directory | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth comment here? Why this condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally had it in there to avoid the possibility of having a cache update from a forked PR from the main branch of another repo. But I just tested and I'm not sure it's actually necessary, the branch always appears as "pull/12345". So I could just take it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I meant the overall condition? What is it trying to achieve? It looks like "Only preserve the cache when building main branch"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the idea is that the cache will be written only by the main branch and read by the other branches. That prevents PRs (which are untrusted) from being able to pollute caches used by other branches.