-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 code path caching #6729
Add code path caching #6729
Conversation
CT Test Results 4 files 187 suites 1h 46m 34s ⏱️ For more details on these failures, see this check. Results for commit a1bdb2c. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
I'd like to add another benchmark. This is for a 1 GHz single core 64-bit RISC-V board running Livebook with Nerves. The timing is from boot until the system is usable. Usable means that it's possible to use the Elixir shell over a UART cable. For running Livebook on Nerves this ends up being close to the time to access Livebook in a web browser, but without network setup variations between runs. OTP 26-dev/unpatched/embedded - 41 seconds This is a huge improvement for Nerves since on these slower processors, we'd avoid running releases in interactive mode due to the boot time penalty. As you can imagine, I'm super happy with this PR and I hope that there's a way to include it in OTP 26. |
You may want to test with a ram disk to avoid storage device latency. A ram disk could be a way of solving the problem without source code changes. Testing with a ram disk would also help you focus on the speedup related to the source code changes. Caching in the source code may help but it would make the interactive mode less interactive while adding the potential to get stale data. |
@okeuday I'd say a huge part of this PR is exactly to remove filesystem lookups, so I think showing numbers from environments where those lookups can be very expensive is important, especially since the gains are almost free. :) It is in a way an upper bound on the benefits we will get. |
1950849
to
55ae5e9
Compare
What happens when a new module is added into a directory that has been previously cached? Or rather, how does the cache gets invalidated? It is very common scenario in our environment to add extra modules and hot-code-load them. My understanding is, this feature should only be enabled at startup, and when startup is complete, boot script should disable the cache. |
@max-au not implemented in this PR yet but the goal is that This will also delete all caches:
For Elixir, I plan to leave Erlang, Elixir and all non-local dependencies (from Hex.pm/Git) permanently cached. YMMV. :) |
What I mean is, the cache suggested by this PR requires explicit invalidation (add_path, replace_path, ...). This behaviour is not backwards-compatible. In all prior versions, I could put the *.beam file anywhere in the existing code path, and load the module, without the need to explicitly flush the cache. We often leveraged that (while doing hot code loads without proper release upgrade, for it was too complex to maintain). It may not be a big issue, but something to be really aware of. |
@max-au the cache will be opt-in. It is enabled by default only for now, to ease testing and benchmarking. |
Hi, we want to move forward with this. I think at least what I would prefer, but I have to check with my peers, that caching during boot should be the default, unless there are any downsides with that, and then disabling of the cache once booted. At least a flag to control this to give users the best of both worlds. I have a difficult time to come up with a fitting name for that. If you have specified -pac or -pzc, they should remain cached afterwards. And specifying -cache_init_paths should be a shorthand for caching otp, init file paths and ERL_LIBS after the boot. Not sure we need to be more fine granular than that. |
Thank you @frazze-jobb! I will work on bringing this to the finish line. However, note I don't think we can enable this only during boot. During In other words, I think all of the
|
I see, anyhow I discussed with @bjorng and he agreed that we should cache OTP by default. Reloading modules in OTP only affects us developing for OTP, and we could turn it off in that case. For OTP26 we can just have one flag for everything, and if more granulatiry is needed, we will deal with it then. |
55ae5e9
to
8cc544e
Compare
@josevalim How is it going with this? |
I will focus on getting this over the finish line, but since there is a release candidate soon, I think we can break this in two.
This way we can get the version that changes the defaults sooner and test it. And the additional APIs, which imply no breaking changes, will come soon after. |
8cc544e
to
8c5d179
Compare
@frazze-jobb ok, V1 of this feature is now in this pull request. It caches the boot paths and that's it. I will work on follow up pull requests assuming this one has been merged, unless you would later prefer for me to squeeze it all in. |
Great, I will have a look and wait for the tests |
@frazze-jobb second part is here: #6823 I gave up on adding |
Okey, I didn't like the -pac and pzc in the first place, so I am fine with that. We can revisit it if someone wants it later. |
0ad10a3
to
1282d6e
Compare
When an application has several entries in its code path, loading modules in interactive mode becomes more expensive because of code path misses. This commit introduces code path caching, where we can opt-in into caching each directory individually. By default, all code paths known during boot (the OTP root, ERL_LIBS, and the ones from the boot script) are cached. This can be disabled by calling `-cache_boot_paths false`. An extended API in the `code` module for caching will be added in future commits, as well as support for `-pac` and `-pzc`.
1282d6e
to
6efdf5b
Compare
6efdf5b
to
a1bdb2c
Compare
We (Klarna's KRED system) is affected by this as I'm preparing to enable OTP-26 for it. We do depend on hot-code loading of new or updated .beam files. I'm currently investigating our options. Just one data point, but this was not a backwards-compatible change. |
@mikpe for clarity, the initial version of this pull request cached all paths by default. The version that was merged caches all paths from the .boot file by default but not dynamically added paths or the ones via One quick work-around is to add |
We use |
When an application has several entries in its code path, loading modules in interactive mode becomes more expensive because of code path misses.
This commit introduces code path caching, where we can opt-in into caching each directory individually.
This won’t be applied to all paths but a project has several paths that are unlikely to change while a system is running in development or test:
Loading of the cache happens lazily, as to avoid introducing rehashes, or any upfront cost.
Benchmarks
This change brought the boot time of Livebook (the time to start all apps in interactive mode for dev+test) on my machine (MacStudio M1 Max) from 1.095s to 0.940, which is roughly a ~15% win.
If you want to try it out, compile Erlang/OTP from this branch and measure the time to boot a project (in Elixir, for example, this is
mix run -e 1
). Run the command at least 5 times.Then open up
code_server.erl
and change this-define(CACHE_DEFAULT, cache).
to-define(CACHE_DEFAULT, nocache).
, runerlc -o lib/kernel/ebin lib/kernel/srccode_server.erl
to recompile it without cache, and run the command again.Decisions to be taken
Assuming we want to move forward with this, we need to take some decisions. Currently this commit enables caching by default, but I assume we don't want this in practice. I propose we add the following features for low-level control:
add_path*
now acceptcache
/nocache
as second argument-pac
and-pzc
to be added to the command lineHowever, Erlang also loads code paths from three other locations:
ERL_LIBS
{path, ...}
instruction in the init scriptWe need to decide if we want one option for caching all three, such as
-cache_init_paths
, or one option for each, such as-cache_otp_lib
,-cache_erl_libs
, and-cache_init_path
.TODO
code:ensure_modules_loaded/1
does not benefit from this patch as it uses another code for loading. We should unify those code paths (which will also help address bugs)cache
/nocache
argument toadd_path*
cache
/nocache
argument toset_path*
cache
/nocache
argument toreplace_path*
-pac
and-pzc
code:del_paths/1
code:clear_caches/1
Future work
A future optimization is to remove parts of the linear lookup. This patch simply changes the code path to be
{Path, #{Beam := Path}}
. Therefore, if we have two cached paths in a row inside thecode_server
, we could merge their maps. This would be useful for Erlang/OTP, for instance, as its directories are typically stored sequentially in the code path.Also note that
where_is_file/1
andwhich/1
are not optimized by this pull request, as they traverse code paths on the caller. Maybe with the cache is worth moving to the server, but I would consider those changes as future work.