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

Avoid hammerring the Code server #12

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Conversation

dumbbell
Copy link
Member

@dumbbell dumbbell commented Apr 28, 2023

Why

When we are about to execute a standalone function, we first need to load the module. This is a synchronous call to the Code server.

So far, we checked that the module was already loaded using code:is_loaded/1 before doing that to avoid too many useless calls to the Code server.

Unfortunately, up to Erlang/OTP 25, code:is_loaded/1 is a synchronous call too! It is changed to an ETS query in Erlang/OTP 26, but before that, it remains an expansive and contentious call. If the same standalone function is executed concurrently a lot of time simultaneously, we end up spamming the Code server with a boat load of "is loaded" queries.

What

To solve the problem, we introduce a horus_utils:is_module_loaded/1 helper. On Erlang/OTP 26+, we call code:is_loaded/1. However on Erlang/OTP 25 and before, we replace that with a test of erlang:get_module_info/2, the function underneath any Module:module_info/1 calls.

This way, it's a relatively cheap way of checking that the module is loaded.

At the same time, we acquire a lock around that module load code to make sure there is only a single attempt to load the module and concurrent processes will stop at "is loaded".

@dumbbell dumbbell added the bug Something isn't working label Apr 28, 2023
@dumbbell dumbbell added this to the v0.2.4 milestone Apr 28, 2023
@dumbbell dumbbell self-assigned this Apr 28, 2023
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 94.44% and project coverage change: -0.01% ⚠️

Comparison is base (47b4295) 86.05% compared to head (5e73331) 86.05%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
- Coverage   86.05%   86.05%   -0.01%     
==========================================
  Files           3        3              
  Lines         918      932      +14     
==========================================
+ Hits          790      802      +12     
- Misses        128      130       +2     
Flag Coverage Δ
erlang-24 81.37% <93.33%> (+<0.01%) ⬆️
erlang-25 83.31% <93.33%> (-0.02%) ⬇️
erlang-26 78.01% <92.85%> (+0.02%) ⬆️
os-ubuntu-latest 86.05% <94.44%> (-0.01%) ⬇️
os-windows-latest 83.31% <93.33%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/horus.erl 85.14% <90.90%> (-0.12%) ⬇️
src/horus_utils.erl 88.46% <100.00%> (+4.25%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dumbbell dumbbell force-pushed the avoid-hammering-code-server branch from 2703388 to 973b12e Compare May 2, 2023 07:49
@dumbbell dumbbell marked this pull request as ready for review May 2, 2023 09:05
[Why]
When we are about to execute a standalone function, we first need to
load the module. This is a synchronous call to the Code server.

So far, we checked that the module was already loaded using
`code:is_loaded/1` before doing that to avoid too many useless calls to
the Code server.

Unfortunately, up to Erlang/OTP 25, `code:is_loaded/1` is a synchronous
call too! It is changed to an ETS query in Erlang/OTP 26, but before
that, it remains an expansive and contentious call. If the same
standalone function is executed concurrently a lot of time
simultaneously, we end up spamming the Code server with a boat load of
"is loaded" queries.

[What]
To solve the problem, we introduce a `horus_utils:is_module_loaded/1`
helper. On Erlang/OTP 26+, we call `code:is_loaded/1`. However on
Erlang/OTP 25 and before, we replace that with a test of
`erlang:get_module_info/2`, the function underneath any
`Module:module_info/1` calls.

This way, it's a relatively cheap way of checking that the module is
loaded.

At the same time, we acquire a lock around that module load code to make
sure there is only a single attempt to load the module and concurrent
processes will stop at "is loaded".
@dumbbell dumbbell merged commit 66ffdbc into main Aug 21, 2023
10 checks passed
@dumbbell dumbbell deleted the avoid-hammering-code-server branch August 21, 2023 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant