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

NLogLoggerFactory - ReOptimize concurrency for CreateLogger #769

Closed
wants to merge 1 commit into from

Conversation

RockNHawk
Copy link

@RockNHawk RockNHawk commented Nov 1, 2024

ReOptimize concurrency for CreateLogger. Followup to #692

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.75%. Comparing base (71af10a) to head (845ba8c).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #769      +/-   ##
==========================================
- Coverage   81.76%   81.75%   -0.02%     
==========================================
  Files          19       19              
  Lines        1810     1803       -7     
  Branches      321      320       -1     
==========================================
- Hits         1480     1474       -6     
  Misses        188      188              
+ Partials      142      141       -1     

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

@snakefoot
Copy link
Contributor

snakefoot commented Nov 1, 2024

Thank you for your contribution, but I prefer to stay aligned with the official LoggerFactory from DotNet-runtime:

https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs

See also: https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#logging

Or do you experience performance issue on docker-images with restricted number of cpu-cores?

@RockNHawk
Copy link
Author

Thank you for your contribution, but I prefer to stay aligned with the official LoggerFactory from DotNet-runtime:感谢您的贡献,但我更愿意与 DotNet-runtime 的官方 LoggerFactory 保持一致:

https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs

See also: https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#logging另请参阅:https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#logging

Or are you experience performance issue on docker-images with restricted number of cpu-cores?或者您是否在 cpu 内核数量受限的 docker-images 上遇到性能问题?

Okay, maybe I need a performance test to compare which is faster, ConcurrentDictionary also uses lock within TryAdd, but it's a fine-grained one.

@RockNHawk RockNHawk closed this Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants