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

AsyncWrapper: Improve performance when blocking #3435

Merged

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented May 26, 2019

ConcurrentRequestQueue - Always see queue as empty after reached RequestLimit (and resolved it)

When reaching the RequestLimit, then producers perform asymmetric update of queue-count.

Instead of trying to coordinate which of the producers should signal start-timer, then all producers will signal start-timer since it is "exceptional" situation.

@snakefoot snakefoot changed the title ConcurrentRequestQueue - Always see queue as empty when reached RequestLimit ConcurrentRequestQueue - Always see queue as empty after reached RequestLimit May 26, 2019
@codecov-io
Copy link

codecov-io commented May 26, 2019

Codecov Report

Merging #3435 into release/4.6.4 will increase coverage by <1%.
The diff coverage is 75%.

@@              Coverage Diff               @@
##           release/4.6.4   #3435    +/-   ##
==============================================
+ Coverage             80%     80%   +<1%     
==============================================
  Files                358     358            
  Lines              28514   28514            
  Branches            3813    3813            
==============================================
+ Hits               22825   22844    +19     
+ Misses              4589    4574    -15     
+ Partials            1100    1096     -4

@304NotModified
Copy link
Member

Hi,

Is this a bug fix or an (performance)enhancement?

@snakefoot
Copy link
Contributor Author

snakefoot commented May 26, 2019

It is continuation from #3416 where I tried to fix the "bug" of failing to handle a RequestLimit of 2 with Blocking-mode.

It is a simplification of the previous commit. So instead of trying to be clever then it just takes a performance hit when blocking mode has been activated.

The simplication reduces the number of modifications of the _count variable, so less memory write barriers are needed when blocking mode is activated. Improves the concurrency of the producers as they are less chatty.

But now all producers will eagerly try to wake the timer-thread after blocking mode has been activated. This might kill any performance gain from improving producer concurrency.

But I like the simplicity of this change, and my simple performance tests still shows good performance

@304NotModified 304NotModified changed the title ConcurrentRequestQueue - Always see queue as empty after reached RequestLimit Async / buffering - performance improvement May 26, 2019
@snakefoot snakefoot changed the title Async / buffering - performance improvement Async / buffering wrapper: Improve performance when blocking May 26, 2019
@snakefoot snakefoot force-pushed the NetCoreAsyncQueueDeadlock branch 4 times, most recently from 23fd656 to a561e08 Compare May 27, 2019 17:43
@304NotModified 304NotModified added this to the 4.6.4 milestone May 27, 2019
@304NotModified 304NotModified merged commit a186275 into NLog:release/4.6.4 May 27, 2019
@304NotModified
Copy link
Member

thanks! Merged!

@snakefoot snakefoot changed the title Async / buffering wrapper: Improve performance when blocking AsyncWrapper: Improve performance when blocking Jun 10, 2019
@snakefoot snakefoot deleted the NetCoreAsyncQueueDeadlock branch April 4, 2020 12:55
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.

None yet

3 participants