-
Notifications
You must be signed in to change notification settings - Fork 54
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
Performance: use :persistent_term instead of :ets #96
Comments
This sounds like it could be a good idea. I was not aware of This might require a version bump to 2.0.0, in order to drop support for earlier versions of OTP. |
Either way, I'd love to see a PR! (Benchmarks comparing it to |
I took @hissssst 's PR #97 and updated main branch, upgraded it to fully support Elixir 1.13.1 and Erlang 24.2. I did some really simple benchmarks with Benchee: defmodule MyVault do
use Cloak.Vault, otp_app: :cloak
def load do
key = :crypto.strong_rand_bytes(32)
{:ok, pid} =
MyVault.start_link(
ciphers: [
default: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V1", key: key}
],
json_library: Jason
)
pid
end
end
MyVault.load()
{:ok, ciphertext} = MyVault.encrypt("aqGwdxdSRg5XIpOvuDJv6YSDOyLwJfdjwkwgA8sr")
{:ok, "aqGwdxdSRg5XIpOvuDJv6YSDOyLwJfdjwkwgA8sr"} = MyVault.decrypt(ciphertext)
Benchee.run(
%{
"encrypt" => fn -> MyVault.encrypt("aqGwdxdSRg5XIpOvuDJv6YSDOyLwJfdjwkwgA8sr") end,
"decrypt" => fn -> MyVault.decrypt(ciphertext) end,
"encrypt_and_decrypt" => fn -> MyVault.encrypt!("aqGwdxdSRg5XIpOvuDJv6YSDOyLwJfdjwkwgA8sr")|> MyVault.decrypt!() end,
},
time: 100,
parallel: 1,
memory_time: 2
) :ets
:persistent_term
Merged results
All my changes are here, better view of all changes here. I think it's not ready for PR. I can remove that "import Config" changes and it should help with support for older versions of Elixir. Tests, credo and dialyzer checks passed well with Elixir 1.11.4, 1.12.3, 1.13.1 and Erlang 23.3, 24.2. See screenshots: |
@oliver-kriska , thank you for this benchmark! However, I think that the performance difference between |
For example, with the same benchmark with For
And for
What is a 20% increase in performance Configuration:
By the way, for 1 in parallel (without
|
This would tune the performance, because in current implementation
:ets
is created withoutread_concurrency
which makes all encryptions and decryptions synchronous.We can:
read_concurrency: true
:persistent_term
instead of:ets
I can implement this issue if you argee with this suggestion
The text was updated successfully, but these errors were encountered: