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

Avoid using Linq as it generates GC each time. #296

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

karljj1
Copy link
Collaborator

@karljj1 karljj1 commented Jul 6, 2022

I was doing some performance comparisons between our old version (2.x) and after upgrading to the latest version.
Previous formatting of a simple string such as "Hello {0} world {1}. This is a {0} test." would generate 0.6KB of garbage each run(after warmup).
When running with the latest version it was 2.6KB.
I found that most of this was coming from Source.cs HasNullableOperator. Its uses Linq which generates GC each time. So I have removed the usage of Linq in some of the critical parts. The GC is now down to around 0.5KB for us.

I also made the Source.cs method TryEvaluateSelector abstract as it seemed to make more sense.

@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #296 (4976725) into main (ed610a9) will decrease coverage by 0%.
The diff coverage is 100%.

❗ Current head 4976725 differs from pull request most recent head 90d6660. Consider uploading reports for the commit 90d6660 to get more accurate results

@@         Coverage Diff         @@
##           main   #296   +/-   ##
===================================
- Coverage    96%    96%   -0%     
===================================
  Files        91     91           
  Lines      3178   3184    +6     
===================================
+ Hits       3061   3066    +5     
- Misses      117    118    +1     
Impacted Files Coverage Δ
src/SmartFormat/Core/Parsing/Format.cs 96% <100%> (+<1%) ⬆️
src/SmartFormat/Core/Sources/Source.cs 100% <100%> (+9%) ⬆️
src/SmartFormat/Extensions/ListFormatter.cs 98% <100%> (+<1%) ⬆️
...rc/SmartFormat/Extensions/LocalizationFormatter.cs 95% <0%> (-5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed610a9...90d6660. Read the comment docs.

@axunonb axunonb self-requested a review July 7, 2022 06:53
Copy link
Member

@axunonb axunonb left a comment

Choose a reason for hiding this comment

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

Very good points - excellent PR 🎉
Yes, with the tiny input format string the effect for speed and GC is most significant, while for other input formats it's not that much (around 5-20%).

@axunonb axunonb merged commit f745375 into axuno:main Jul 7, 2022
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.

None yet

2 participants