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

Fix performance of some versions of add_agent #821

Merged
merged 1 commit into from
Jun 24, 2023
Merged

Conversation

Tortar
Copy link
Member

@Tortar Tortar commented Jun 24, 2023

Now I don't see any regression with these changes, running the benchmark in the related issue #820

julia> @benchmark model_1()
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min  max):  125.087 μs    2.810 ms  ┊ GC (min  max):  0.00%  90.66%
 Time  (median):     134.490 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   158.740 μs ± 207.101 μs  ┊ GC (mean ± σ):  12.16% ±  8.70%

  ▁▆███▇▆▅▄▄▃▃▂▂▂▁▁▁▁▁▁▁  ▁▁                                    ▃
  ██████████████████████████▇█▇▆██▇▆▆▇▇▇▆▆▅▅▅▁▅▅▃▅▄▅▄▄▃▅▄▄▅▃▃▁▃ █
  125 μs        Histogram: log(frequency) by time        257 μs <

 Memory estimate: 434.19 KiB, allocs estimate: 4385.

julia> @benchmark model_2()
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min  max):  114.889 μs    2.980 ms  ┊ GC (min  max):  0.00%  88.82%
 Time  (median):     126.441 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   153.133 μs ± 237.755 μs  ┊ GC (mean ± σ):  14.52% ±  8.83%

   ▆███▇▅▄▄▃▂▂▂▁▁▁ ▁                                            ▂
  █████████████████████▆▆▇▇▆▇▆▆▆▆▅▅▆▄▆▅▄▅▄▃▅▅▁▁▃▄▄▁▅▁▃▁▁▁▄▁▃▄▃▃ █
  115 μs        Histogram: log(frequency) by time        293 μs <

 Memory estimate: 434.89 KiB, allocs estimate: 4394.

all of this was based on https://discourse.julialang.org/t/why-using-a-mutable-struct-type-argument-to-create-instances-creates-a-50x-slowdown/100764

@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2023

Codecov Report

Merging #821 (4a613af) into main (14886a4) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #821   +/-   ##
=======================================
  Coverage   69.75%   69.75%           
=======================================
  Files          41       41           
  Lines        2642     2642           
=======================================
  Hits         1843     1843           
  Misses        799      799           
Impacted Files Coverage Δ
src/core/space_interaction_API.jl 91.86% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@Datseris Datseris left a comment

Choose a reason for hiding this comment

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

fantastic, thank you

@Datseris Datseris merged commit c64bcab into main Jun 24, 2023
@Datseris Datseris deleted the fix-add-agent branch June 24, 2023 14:51
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.

3 participants