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

Change flag storage to reduce memory use #108

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

FollowTheProcess
Copy link
Owner

Summary

Changes Entry to *Entry in the internal flagset storage so that we can look up the same memory from shorthand or flag name.

Benchstat seems to suggest that although it allocates more (pointers rather than stack copies), the end result is actually faster
and uses 37% less memory when constructing a typical cli with cli.New

goos: darwin
goarch: amd64
pkg: github.com/FollowTheProcess/cli
cpu: Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
              │ before.txt  │             after.txt              │
              │   sec/op    │   sec/op     vs base               │
ExecuteHelp-8   7.910µ ± 1%   7.799µ ± 1%  -1.41% (p=0.000 n=20)
New-8           1.886µ ± 0%   1.741µ ± 0%  -7.69% (p=0.000 n=20)
geomean         3.862µ        3.685µ       -4.60%

              │  before.txt  │              after.txt               │
              │     B/op     │     B/op      vs base                │
ExecuteHelp-8   5.347Ki ± 0%   5.347Ki ± 0%    0.00% (p=0.001 n=20)
New-8           3.079Ki ± 0%   1.953Ki ± 0%  -36.57% (p=0.000 n=20)
geomean         4.057Ki        3.232Ki       -20.36%

              │ before.txt │              after.txt              │
              │ allocs/op  │ allocs/op   vs base                 │
ExecuteHelp-8   81.00 ± 0%   81.00 ± 0%       ~ (p=1.000 n=20) ¹
New-8           24.00 ± 0%   26.00 ± 0%  +8.33% (p=0.000 n=20)
geomean         44.09        45.89       +4.08%
¹ all samples are equal

@FollowTheProcess FollowTheProcess added performance Performance refactoring Refactoring labels Oct 5, 2024
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.42%. Comparing base (e355191) to head (dd56c69).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/flag/set.go 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
- Coverage   92.60%   92.42%   -0.18%     
==========================================
  Files           7        7              
  Lines        1028     1030       +2     
==========================================
  Hits          952      952              
- Misses         44       45       +1     
- Partials       32       33       +1     

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

@FollowTheProcess FollowTheProcess merged commit 842c856 into main Oct 5, 2024
6 of 8 checks passed
@FollowTheProcess FollowTheProcess deleted the refactor/duplicate-entries branch October 5, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance refactoring Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant