piskvorky / sqlitedict

Persistent dict, backed by sqlite3 and pickle, multithread-safe.
Apache License 2.0
1.16k stars 130 forks source link

Properly handle the race condition #164

Closed mpenkov closed 1 year ago

mpenkov commented 1 year ago

Use synchronization primitives instead of sleeping. This eliminates the need for the timeout parameter. The affected class is effectively internal, so this "should" not affect regular users.

Also fixed the tests: they didn't clean up after themselves.

Added benchmarks.

Fixes #152

mpenkov commented 1 year ago

On MacOS, there is a significant difference.

Before (note time is in seconds):

-------------------------------------------- benchmark: 1 tests --------------------------------------------
Name (time in s)        Min      Max     Mean  StdDev   Median     IQR  Outliers     OPS  Rounds  Iterations
------------------------------------------------------------------------------------------------------------
test                 9.8556  11.3276  10.8173  0.5690  10.9809  0.5847       1;0  0.0924       5           1
------------------------------------------------------------------------------------------------------------

After (note time is in ms):

------------------------------------------------ benchmark: 1 tests ------------------------------------------------
Name (time in ms)          Min       Max      Mean   StdDev    Median      IQR  Outliers     OPS  Rounds  Iterations
--------------------------------------------------------------------------------------------------------------------
test                  338.2111  401.0051  355.2427  26.0633  344.9050  23.6982       1;1  2.8150       5           1
--------------------------------------------------------------------------------------------------------------------

On Linux, there is no difference in performance, but I could not reproduce the problem, the benchmark times are about the same (~300ms).

piskvorky commented 1 year ago

Oh wow, 30x slower now? We'll have to look into that, that's too much. I'll try to check too (I have a macbook).

mpenkov commented 1 year ago

No, it's the other way around. Before it was 10s, now it's 338ms (0.338s).

piskvorky commented 1 year ago

Oh yes, I missed the bracket with "note" of course :) Sorry.

Great job!