Skip to content

Use Application.get_env/3 calls instead of module attributes #8

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frm
Copy link
Contributor

@frm frm commented Jul 7, 2020

Module attributes are defined in compile time. I just happened to download and compile the application (which includes compiling NanoID as a dependency) and then change the config in my app to use a different size by default.

However, since the @default_size module attribute was defined when NanoID compiled, it wasn't having any effect. I had to explicitly recompile NanoID.

Instead of doing so, we can define the module attribute to be the fallback default value and use Application.get_env/3 in runtime to get the value from the application config.

Note: I made this separate to #7 since this is an issue that might not necessarily be merged.

@frm frm force-pushed the frm/module-attrs branch from 3234e8a to 3912471 Compare July 7, 2020 17:07
@railsmechanic
Copy link
Owner

railsmechanic commented Jul 7, 2020

This was actually the main reason why I introduced the Configuration module. However, it turnes out that continuously reading the configuration via Application.get_env/3 has a significant effect on the speed of nanoid. In my tests, it lowers the generation speed (ips) about 1k. Please see #6 for further information. For this reason I have prioritized speed.

But you can recompile nanoid with mix deps.compile nanoid --force when the configuration has changed.
For the deployment, I used to compile everything in a fresh docker container to avoid such problems.

@frm
Copy link
Contributor Author

frm commented Jul 8, 2020

Ah, I missed that. My bad. That's actually really interesting. It would probably help other developers to be aware of this limitation. Would you consider adding a note referencing/explaining this in the configuration section of the README?

@c4710n
Copy link
Contributor

c4710n commented Jan 20, 2021

Most of developers introduce external libs for reducing mental burden. Because of that, I think it would be enough to tell them "how to do that", rather than "why to do that".

Curious boys will read the code.

Create a new PR - #13 for "how to do that".

@railsmechanic
Copy link
Owner

But I'm sure that many developers would like to know the reason why a descision was made...

@jc00ke
Copy link

jc00ke commented Mar 31, 2022

This piqued my interest and I thought of :persistent_term as a possible solution. Here's the benchmark results based on 41c6285

  Operating System: Linux
  CPU Information: AMD Ryzen 9 3900X 12-Core Processor
  Number of Available Cores: 24
  Available memory: 62.79 GB
  Elixir 1.13.0
  Erlang 24.1.7

  Benchmark suite executing with the following configuration:
  warmup: 2 s
  time: 5 s
  memory time: 0 ns
  reduction time: 0 ns
  parallel: 1
  inputs: none specified
  Estimated total run time: 21 s

  Benchmarking nanoid ...
  Benchmarking nanoid_ag ...
  Benchmarking nanoid_pt ...

  Name                ips        average  deviation         median         99th %
  nanoid          18.18 K       54.99 μs    ±11.61%       54.79 μs       67.70 μs
  nanoid_pt       17.51 K       57.10 μs    ±10.40%       56.86 μs       70.22 μs
  nanoid_ag       17.15 K       58.32 μs    ±10.16%       57.97 μs       74.22 μs

  Comparison:
  nanoid          18.18 K
  nanoid_pt       17.51 K - 1.04x slower +2.10 μs
  nanoid_ag       17.15 K - 1.06x slower +3.33 μs

5% slower for both is a lot better than the 14% reported in #6.

Maybe that 5% hit is worth it to be able to change the defaults w/o recompiling? Maybe not 😉

@ruslandoga
Copy link

ruslandoga commented Nov 22, 2023

Seems like newer Elixir version don't have that problem.

I've created a new project, installed :nanoid, and ran it:

$ iex -S mix
==> nanoid
Compiling 4 files (.ex)
Generated nanoid app
==> nanoid_example
Compiling 1 file (.ex)
Generated nanoid_example app
Erlang/OTP 26 [erts-14.1.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Interactive Elixir (1.15.7) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Nanoid.generate
"F0BE-ZmwBL2g_ZC7Zj-qg"
iex(2)> byte_size Nanoid.generate
21

Then changed the configuration in config/config.exs

import Config
config :nanoid, size: 10

And ran it again

$ iex -S mix
==> nanoid
Compiling 4 files (.ex)
Generated nanoid app
==> nanoid_example
Generated nanoid_example app
Erlang/OTP 26 [erts-14.1.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Interactive Elixir (1.15.7) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Nanoid.generate
"gA7DmeUhRz"
iex(2)> byte_size Nanoid.generate
10

Note that nanoid was recompiled and the generated values were of the requested size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants