search

mbdavid / LiteDB

LiteDB - A .NET NoSQL Document Store in a single data file
http://www.litedb.org
MIT License
8.36k stars 1.22k forks source link

Fixed writing pages in the same thread that does EnqueuePage #2453

Closed alexbereznikov closed 3 days ago

alexbereznikov commented 3 months ago

AsyncManualResetEvent was using TaskCompletionSource which by default runs continuations synchronously. That led to writing pages in the same thread that called EnqueuePage and, as a result, ~4x performance degradation.

Used more correct way to asynchronously wait for wait handle.

This partially fixes #2451

JKamsker commented 3 weeks ago

I am concerned about the extra dependencySystem.Threading.ThreadPool. Will that be a problem somewhere?

Also, do you happen to have a Benchmark (result+code) at hand?

alexbereznikov commented 3 weeks ago

Hi @JKamsker , this dependency exists only because of netstandard1.3 target (for some reason thread pool routines are not available out of box in netstandard1.3). I don't think it is a vulnerable package or something.

Benchmark code is contained in this bug - #2451 , as well as results in terms of time taken to run the app. This pr aimed to partially fix it as well

alexbereznikov commented 3 weeks ago

Hey @alexbereznikov, thanks for this fix. I've a couple of suggestions.

And please, can you add unit tests for this, that way we can avoid regressions

Hey @pictos , regarding unit test for this - I can't think of a good way to create a unit test for this without exposing DiskWriterQueue internals or coming up with additional abstractions that required only for tests.

I added an ENSURE check instead, so in case of regression - tests will fail.

alexbereznikov commented 3 days ago

Closed as redundant due to #2515