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.

Sign up for GitHub

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

Jump to bottom

Performance improvements #26

Merged
merged 1 commit into from
Dec 14, 2023
Merged

Performance improvements #26

merged 1 commit into from
Dec 14, 2023

Conversation

Stratus3D
Copy link
Contributor

@Stratus3D Stratus3D commented Dec 13, 2023 •
edited
Loading

Card

This PR was branched off of #24 in order build on what was contributed there.

  • Switch to iolist to improvement performance
  • Update to latest version of Erlang 26
  • Minor performance tweaks
  • Document benchee benchmark
  • Add Elixir 1.15 and Erlang 26, remove Elixir 1.13 from GitHub workflow

Benchmarks

Before 
Operating System: macOS
CPU Information: Apple M1 Pro
Number of Available Cores: 10
Available memory: 32 GB
Elixir 1.15.7
Erlang 26.2

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
reduction time: 0 ns
parallel: 1
inputs: debug, info
Estimated total run time: 14 s

Benchmarking format/4 with input debug ...
Benchmarking format/4 with input info ...

##### With input debug #####
Name               ips        average  deviation         median         99th %
format/4      579.53 K        1.73 μs  ±1081.85%        1.38 μs        2.92 μs

##### With input info #####
Name               ips        average  deviation         median         99th %
format/4      377.40 K        2.65 μs   ±388.45%        2.33 μs        5.08 μs
After 
Compiling 1 file (.ex)
Operating System: macOS
CPU Information: Apple M1 Pro
Number of Available Cores: 10
Available memory: 32 GB
Elixir 1.15.7
Erlang 26.2

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
reduction time: 0 ns
parallel: 1
inputs: debug, info
Estimated total run time: 14 s

Benchmarking format/4 with input debug ...
Benchmarking format/4 with input info ...

##### With input debug #####
Name               ips        average  deviation         median         99th %
format/4        1.55 M      643.44 ns  ±2151.32%         583 ns         750 ns

##### With input info #####
Name               ips        average  deviation         median         99th %
format/4      991.21 K        1.01 μs  ±1398.08%        0.92 μs        1.13 μs

pasha
pasha reviewed Dec 13, 2023
View reviewed changes
mix.exs Outdated
@@ -42,7 +42,8 @@ defmodule ExJsonLogger.Mixfile do
{:credo, "~> 1.6.7", only: [:dev, :test]},
{:dialyxir, "~> 0.5.1", only: [:dev], runtime: false},
{:plug, "~> 1.14"},
{:jason, "~> 1.0"}
{:jason, "~> 1.0"},
{:benchee, "~> 1.2", only: :bench}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should only load this in dev

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have it only set to load in bench it won't be loaded in any environment except bench. I can set this to dev if we think we might always want it in dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the benchmarks with environment changed to dev and got the same numbers so I guess there is no harm in changing it to dev. Just note that in dev benchee will always be downloaded when running mix deps.get. PR updated.

rkledesma
rkledesma reviewed Dec 14, 2023
View reviewed changes
Copy link

@rkledesma rkledesma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some failing unit tests

@Stratus3D
Copy link
Contributor Author

Some failing unit tests

I fixed the tests locally. CI workflow is running tests on all versions of Erlang.

Stratus3D
Stratus3D commented Dec 14, 2023
View reviewed changes
.github/workflows/elixir.yml
- otp: '22'
elixir: '1.13'
- otp: '23'
elixir: '1.14'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to add Erlang 26 and Elixir 1.15 to this matrix. I didn't see any reason to continue supporting Elixir 1.13.

@Stratus3D Stratus3D mentioned this pull request Dec 14, 2023
Closed
@Stratus3D @ruslandoga
Performance improvements
2320feb 
[Card](https://rentpath.atlassian.net/browse/SRV-5981)

This PR was branched off of #24 in order build on what was contributed there.

* Switch to iolist to improvement performance
* Update to latest version of Erlang 26
* Minor performance tweaks
* Document benchee benchmark
* Add Elixir 1.15 and Erlang 26, remove Elixir 1.13 from GitHub workflow

Co-authored-by: ruslandoga <67764432+ruslandoga@users.noreply.github.com>
@Stratus3D Stratus3D merged commit 452c251 into master Dec 14, 2023
7 checks passed
@Stratus3D Stratus3D deleted the tb/SRV-5981-improve-perf branch December 14, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkledesma rkledesma rkledesma approved these changes

@pasha pasha pasha approved these changes

Assignees

@Stratus3D Stratus3D

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Stratus3D @pasha @rkledesma