Refactor code server to use a single queue
Prior to this patch, the code server had two internal queues, one to track module loading and another to track on_load callbacks. This pull requests refactors the code to have a single queue, in order to fix bugs and improve maintainability.
Closes #7466. Closes #8510.
CT Test Results
2 files 70 suites 1h 5m 20s :stopwatch: 1 551 tests 1 309 :white_check_mark: 242 :zzz: 0 :x: 1 767 runs 1 493 :white_check_mark: 274 :zzz: 0 :x:
Results for commit 72f2c90f.
:recycle: 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
Ideally this would be merged on maint because it fixes a regression on OTP 27 but it is a conceptually large change. My suggestion is to merge it to master and, once 27.1 is out, backport it to maint, which should give us time to battle test it for 27.2.
Thanks for your contribution. I plan to look at this after ICFP (in 10 days) approx
Please don't merge this yet. I have tried this branch in a project and, while I didn't notice any bugs, compilation times for Elixir (which loads several modules) was affected negatively.
False alarm, I had mixed my Erlang versions. Overall, I didn't measure any performance degradation in my usual code loading benchmarks. :)
Looking into it this week
This was on hold due to other investigations regarding current issues in code_server. As soon as I am given green light, I review this PR
Glad to hear so! 🎉 Code loading is a state machine and the previous version was encoding the states via anonymous functions and in two separate queues, which made it very hard to follow. :)
Completely agree, it was pretty tricky to follow the logic. This was a great improvement.
Since the next OTP release 27.2 is around beginning of December, I am wondering if we can place this already on maint, instead of master. That gives us close to two months of testing
As long as we are comfortable with potentially doing a 27.2.1 with this commit reverted, it sounds good to me. :D
Update This will be merged in maint after I add the following types
on_load = #{} :: #{module() => {on_load_file(), client_pid(), on_load_pid()}},
loading = #{} :: #{module() => [{loading_action(), client_pid()}]}}).
Please do some squashing of the commits before merging this. At least squash the second commit into the first, or squash all commits into one.
@kikofernandez let me know if you want me to do the squashing. :)
sure, go ahead! I will merge if our internal builds look good after lunch