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

JsonLayout - Reduce allocations when needing to escape string (44% time improvement) #2743

Merged
merged 5 commits into from
May 29, 2018

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented May 25, 2018

Reuse truncated StringBuilder, instead of creating new.

Also skip the temporary StringBuilder, when using Encode=false. Performance optimization for nested JsonLayouts. But one can also consider using Encode=false for simple fields like ${longdate} or ${level} as it skips unnecessary validation.

@codecov
Copy link

codecov bot commented May 25, 2018

Codecov Report

Merging #2743 into master will decrease coverage by <1%.
The diff coverage is 84%.

@@           Coverage Diff           @@
##           master   #2743    +/-   ##
=======================================
- Coverage      81%     81%   -<1%     
=======================================
  Files         326     326            
  Lines       24242   24293    +51     
  Branches     3073    3078     +5     
=======================================
+ Hits        19611   19633    +22     
- Misses       3801    3829    +28     
- Partials      830     831     +1

@snakefoot snakefoot force-pushed the JsonLayoutEscapeStringAlloc branch from e8dc704 to e9c254d Compare May 25, 2018 22:30
@304NotModified 304NotModified added this to the 4.5.6 milestone May 26, 2018
@snakefoot
Copy link
Contributor Author

Think one could improve performance even more, by writing directly into the StringBuilder, and just truncate when needing to escape.

@snakefoot
Copy link
Contributor Author

snakefoot commented May 27, 2018

Now there is also a good performance improvement, even when NOT having to escape strings:

Logger Name Messages Size Args Threads Loggers
JsonLogger 2.500.000 128 5 4 1
Test Name Time (ms) Msgs/sec GC2 GC1 GC0 CPU (ms) Alloc (MB)
This PR 15.972 156.514 1 10 20 24.828 2.238,2
NLog 4.5.5 23.073 108.348 1 29 57 26.421 3.736,2

@304NotModified
Copy link
Member

nice, 44% improvement!

@snakefoot
Copy link
Contributor Author

snakefoot commented May 27, 2018

Think one could optimize all that inherits from WrapperLayoutRendererBuilderBase with the same technique used here (in my last commit).

@snakefoot snakefoot force-pushed the JsonLayoutEscapeStringAlloc branch from dd4e66f to 2369bd5 Compare May 27, 2018 21:08
@304NotModified 304NotModified changed the title JsonLayout - Reduce allocations when needing to escape string JsonLayout - Reduce allocations when needing to escape string (44% time improvement) May 29, 2018
@304NotModified
Copy link
Member

Think one could optimize all that inherits from WrapperLayoutRendererBuilderBase with the same technique used here (in my last commit).

sound good :)

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

Successfully merging this pull request may close these issues.

2 participants