rocicorp / repc

The canonical Replicache client, implemented in Rust.
Other
31 stars 7 forks source link

Idb transactions around operations #275

Closed arv closed 3 years ago

arv commented 3 years ago

We now create individual IDB transactions for get/has instead of opening the IDB transaction in the kv open transaction

Towards https://github.com/rocicorp/replicache-sdk-js/issues/143

arv commented 3 years ago

@phritz This might impact #240

arv commented 3 years ago

Perf tests master:

Running benchmarks please wait...
populate 1024x1000 (clean, indexes: 0) x 3.83 MB/s ±0.0% (7 runs sampled)
populate 1024x1000 (dirty, indexes: 0) x 4.48 MB/s ±0.0% (10 runs sampled)
populate 1024x1000 (clean, indexes: 1) x 2.13 MB/s ±0.0% (5 runs sampled)
populate 1024x1000 (clean, indexes: 2) x 1.49 MB/s ±0.0% (5 runs sampled)
read tx 1024x1000 x 7.45 MB/s ±0.0% (15 runs sampled)
read tx 1024x5000 x 7.94 MB/s ±0.0% (5 runs sampled)
scan 1024x1000 x 18.78 MB/s ±0.0% (31 runs sampled)
scan 1024x5000 x 22.30 MB/s ±0.0% (9 runs sampled)
write single byte x 8.70 tx/s ±0.0% (18 runs sampled)
Done!

Perf tests with arv:idb-tx-around-ops:

Running benchmarks please wait...
populate 1024x1000 (clean, indexes: 0) x 3.23 MB/s ±0.0% (6 runs sampled)
populate 1024x1000 (dirty, indexes: 0) x 4.17 MB/s ±0.0% (9 runs sampled)
populate 1024x1000 (clean, indexes: 1) x 1.99 MB/s ±0.0% (5 runs sampled)
populate 1024x1000 (clean, indexes: 2) x 1.38 MB/s ±0.0% (5 runs sampled)
read tx 1024x1000 x 7.23 MB/s ±0.0% (15 runs sampled)
read tx 1024x5000 x 8.04 MB/s ±0.0% (5 runs sampled)
scan 1024x1000 x 16.01 MB/s ±0.0% (32 runs sampled)
scan 1024x5000 x 23.36 MB/s ±0.0% (10 runs sampled)
write single byte x 5.78 tx/s ±0.0% (12 runs sampled)
Done!
arv commented 3 years ago

It seems that opening an IDB read transaction has no discernible performance overhead. That is good news because it means that we do not need to refactor scan to open the IDB tx before we scan all the items.

Ready for review!

aboodman commented 3 years ago

The writes seem a little slower to me:

master:

populate 1024x1000 (clean, indexes: 0) x 3.83 MB/s ±0.0% (7 runs sampled) populate 1024x1000 (dirty, indexes: 0) x 4.48 MB/s ±0.0% (10 runs sampled) populate 1024x1000 (clean, indexes: 1) x 2.13 MB/s ±0.0% (5 runs sampled) populate 1024x1000 (clean, indexes: 2) x 1.49 MB/s ±0.0% (5 runs sampled)

write single byte x 8.70 tx/s ±0.0% (18 runs sampled)

branch:

populate 1024x1000 (clean, indexes: 0) x 3.23 MB/s ±0.0% (6 runs sampled) populate 1024x1000 (dirty, indexes: 0) x 4.17 MB/s ±0.0% (9 runs sampled) populate 1024x1000 (clean, indexes: 1) x 1.99 MB/s ±0.0% (5 runs sampled) populate 1024x1000 (clean, indexes: 2) x 1.38 MB/s ±0.0% (5 runs sampled)

write single byte x 5.78 tx/s ±0.0% (12 runs sampled)

Note these are medians on a single machine, so probably reliable. You could do it a few more times to be sure.

This drop seems reasonable to me, since we're now doing more work. Frankly better than I expected. So yay, I'm ok with this to proceed w/ serviceworker experiments.

The one exception is the write-single-byte test, which got a lot slower. This test does:


  const rep = await makeRep();
  const write = rep.register('write', tx => tx.put('k', i % 10));
  bench.name = 'write single byte';
  bench.size = 1;
  bench.formatter = formatTxPerSecond;
  bench.reset();

  await write(null);

  bench.stop();
  await rep.close();

So it seems it is doing the exact same amt of work in both master and branch - open a transaction, writing a byte, then committing. Just doing it a little later. Surprised to see any difference in that one. Worth investigating.

Erik, when I was investigating this stuff I found the timeline view in Chrome invaluable. If you run with what we currently call a "debug" build, you'll get symbols you can see in timeline, and you can easily see where time is getting spent inside Rust.

On Wed, Nov 18, 2020 at 1:58 PM Erik Arvidsson notifications@github.com wrote:

It seems that opening an IDB read transaction has no discernible performance overhead. That is good news because it means that we do not need to refactor scan to open the IDB tx before we scan all the items.

Ready for review!

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/rocicorp/repc/pull/275#issuecomment-730032332, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAATUBDRLXSTRFPTM44GPUDSQRNRNANCNFSM4TZIKU4A .

arv commented 3 years ago

I'm also surprised that write changed. Logically nothing should have changed there.

arv commented 3 years ago

Not doing this because we are not doing SW so it is not needed.