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

cache decrypted content based on record buffer #349

Closed
wants to merge 2 commits into from
Closed

Conversation

staltz
Copy link
Member

@staltz staltz commented Jun 8, 2022

This is a just-FYI PR. I ran unit tests (e.g. test/private.js) and confirmed that the cache causes the decryption to be skipped if it has been already done.

In the big picture though, unfortunately it doesn't seem like caching does any difference to performance. In fact in some cases it seems like performance got worse. I don't understand why. :(

I'm leaving this PR here in case I made a dumb mistake somewhere and there would still be hope for this optimization technique.

Numbers below

Perf test Before After
add a bunch of messages 530ms 530ms
box1 1132ms 1125ms
unbox first run 105ms 115ms
unbox second run 91ms 89ms
private box1 no decrypt, box 1117ms 1021ms
query first run 35ms 30ms
query second run 20ms 23ms
private box2, box 603ms 552ms
unbox duration first run 314ms 313ms
unbox duration second run 315ms 319ms
migrate (alone) 3928ms 18515ms
migrate (+db2) 5991ms 6550ms
migrate continuation (+db2) 742ms 1097ms
migrate (+db1) 10297ms 16482ms
migrate (+db1 +db2) 27527ms 5842ms
Memory usage without indexes 713.67 MB = 36.50 MB + etc 759.07 MB = 31.53 MB + etc
initial indexing 496ms 699ms
initial indexing maxcup 86% 5910ms 5200ms
initial indexing compat 623ms 1285ms
Two indexes updating concurrently 727ms 910ms
key one initial 50ms 352ms
key two 2ms 3ms
key one again 1ms 1ms
reboot and key one again 61ms 352ms
latest root posts 509ms 612ms
latest posts 12ms 8ms
votes one initial 468ms 537ms
votes again 1ms 1ms
hasRoot 279ms 384ms
hasRoot again 0ms 1ms
author one posts 270ms 280ms
author two posts 17ms 18ms
dedicated author one posts 258ms 327ms
dedicated author one posts again 0ms 1ms
maxium RAM used 813.08 MB = 52.54 MB + etc 786.40 MB = 52.74 MB + etc
Indexes folder size 9.97 MB 9.97 MB

@staltz staltz requested a review from arj03 June 8, 2022 13:55
indexes/private.js Outdated Show resolved Hide resolved
@arj03
Copy link
Member

arj03 commented Jun 8, 2022

Well, all indexes in db2 share the same stream on index update. So you'll only try to decrypt something once and give that to all indexes. It's true that jitdb has it's own stream, so there could be something here. This was one of the big differences compared to flume and db1 where each flume index was completely separate.

@staltz
Copy link
Member Author

staltz commented Jun 8, 2022

Well, that's the unsolved mystery: why do I get 4 calls to tryDecryptContent() for the same record? I don't know why, but I assumed that it's because of 3 leveldb indexes and 1 for jitdb. I checked, and at runtime there were 3 leveldb indexes.

If you want to try it yourself, put a console.log in tryDecryptContent that logs the recBuffer as hex, and then run the test test/private.js and you'll see 4 console.logs for 1 record.

@arj03
Copy link
Member

arj03 commented Jun 8, 2022

strange, I'll have a look

@arj03
Copy link
Member

arj03 commented Jun 8, 2022

It's actually jitdb that is doing the extra calls:

Trace: trying to decrypt Ex70yEq5H4M6Mnv1kmz6
    at tryDecryptContent (/home/arj/dev/ssb-db2/indexes/private.js:150:13)
    at Object.decrypt (/home/arj/dev/ssb-db2/indexes/private.js:207:23)
    at Object.o.write (/home/arj/dev/ssb-db2/log.js:98:57)
    at Stream._writeToSink (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/stream.js:79:33)
    at Stream._handleBlock (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/stream.js:109:12)
    at Stream._resumeCallback (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/stream.js:145:27)
    at Object.getBlock (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:201:7)
    at Stream._resume (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/stream.js:136:12)
    at Stream._next (/home/arj/dev/ssb-db2/node_modules/looper/index.js:11:9)
    at Stream.resume (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/stream.js:164:8)

Trace: trying to decrypt Ex70yEq5H4M6Mnv1kmz6
    at tryDecryptContent (/home/arj/dev/ssb-db2/indexes/private.js:150:13)
    at Object.decrypt (/home/arj/dev/ssb-db2/indexes/private.js:207:23)
    at /home/arj/dev/ssb-db2/log.js:69:31
    at gotBlock (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:223:7)
    at getBlock (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:201:7)
    at get (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:219:5)
    at waitForLogLoaded (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:516:12)
    at Object.log.get (/home/arj/dev/ssb-db2/log.js:65:5)
    at getRecord (/home/arj/dev/ssb-db2/node_modules/jitdb/index.js:1217:9)
    at filterRecord (/home/arj/dev/ssb-db2/node_modules/jitdb/index.js:1272:5)

Trace: trying to decrypt Ex70yEq5H4M6Mnv1kmz6
    at tryDecryptContent (/home/arj/dev/ssb-db2/indexes/private.js:150:13)
    at Object.decrypt (/home/arj/dev/ssb-db2/indexes/private.js:207:23)
    at /home/arj/dev/ssb-db2/log.js:69:31
    at gotBlock (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:223:7)
    at getBlock (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:201:7)
    at get (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:219:5)
    at waitForLogLoaded (/home/arj/dev/ssb-db2/node_modules/async-append-only-log/index.js:516:12)
    at Object.log.get (/home/arj/dev/ssb-db2/log.js:65:5)
    at getRecord (/home/arj/dev/ssb-db2/node_modules/jitdb/index.js:1217:9)
    at filterRecord (/home/arj/dev/ssb-db2/node_modules/jitdb/index.js:1272:5)

The last 2 calls are !streaming, because they are log.get calls.

@arj03
Copy link
Member

arj03 commented Jun 8, 2022

Ah right, I was instrumenting this and it does publish and then 2 queries for that author, so 3 decrypts is the correct number.

@staltz
Copy link
Member Author

staltz commented Jun 8, 2022

Have you tried test/private.js?

@arj03
Copy link
Member

arj03 commented Jun 8, 2022

Ahh :) It's because we are posting the same message 4 times. So by the time you get to publishAs classic, then { type: 'post', text: 'super secret', recps: [keys.id] } will be in the db encrypted 4 times ;)

@staltz
Copy link
Member Author

staltz commented Jun 8, 2022

How so? If I run only the first test in that file, it has just 1 publish(), and yet:

# publish encrypted message
ok 1 no err
ok 2 should be strictly equal
decryptAndReconstruct a527186b6579a003
decryptAndReconstruct a527186b6579a003
decryptAndReconstruct a527186b6579a003
decryptAndReconstruct a527186b6579a003
ok 3 should be strictly equal

@arj03
Copy link
Member

arj03 commented Jun 8, 2022

This seems to be a bug in AAOL related to multiple streams and a half-empty database. I'm going to reproduce this in AAOL and fix it there.

@staltz
Copy link
Member Author

staltz commented Jun 8, 2022

That sounds like a good hypothesis! It does feel to me like there are non-shared streams going on.

@arj03 arj03 mentioned this pull request Jun 8, 2022
@arj03
Copy link
Member

arj03 commented Jun 8, 2022

mystery solved solved. The 4 calls to decrypt were:

@staltz
Copy link
Member Author

staltz commented Jun 9, 2022

Thanks for figuring this out! Indeed, that's the truth. I think we can close this PR, there's nothing else here we can make use of.

@staltz staltz closed this Jun 9, 2022
@staltz staltz deleted the record-cache branch June 9, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants