Skip to content

Conversation

taktoa
Copy link

@taktoa taktoa commented Aug 22, 2018

This pretty much works, there's just one test that doesn't seem to pass and I'm not sure why:

tape('should generate the same stateRoot', async t => { let tree1 = new RadixTree({ db }) let tree2 = new RadixTree({ db }) await tree1.flush() tree1.set('test', Buffer.from('cat')) tree2.set('test', Buffer.from('cat')) const stateRoot1 = await tree1.flush() const stateRoot2 = await tree2.flush() t.deepEquals(stateRoot2, stateRoot1) t.end() })

The output it gives is:

# should generate the same stateRoot not ok 4 should be equivalent --- operator: deepEqual expected: |- <Buffer a0 79 94 6f 11 e2 1b 49 6b 4f 0d 2e 75 ad 8f 40 1c 6e 1e 7a> actual: |- <Buffer 01 45 fb 4f 4b 0d 9b a0 62 b5 c1 f8 f9 ea 30 11 ab f0 5a 78> at: Test.tape (/home/remy/Documents/WorkWork/dfinity/js-dfinity-radix-tree/tests/index.js:47:5) stack: |- Error: should be equivalent at Test.assert [as _assert] (/home/remy/Documents/WorkWork/dfinity/js-dfinity-radix-tree/node_modules/tape/lib/test.js:225:54) at Test.bound [as _assert] (/home/remy/Documents/WorkWork/dfinity/js-dfinity-radix-tree/node_modules/tape/lib/test.js:77:32) at Test.deepEqual.Test.deepEquals.Test.isEquivalent.Test.same (/home/remy/Documents/WorkWork/dfinity/js-dfinity-radix-tree/node_modules/tape/lib/test.js:421:10) at Test.bound [as deepEquals] (/home/remy/Documents/WorkWork/dfinity/js-dfinity-radix-tree/node_modules/tape/lib/test.js:77:32) at Test.tape (/home/remy/Documents/WorkWork/dfinity/js-dfinity-radix-tree/tests/index.js:47:5) at <anonymous> ... 
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.9%) to 95.23% when pulling bbfcac4 on taktoa/blake2 into 7a862b1 on master.

@FloorLamp
Copy link
Contributor

Thanks, I fixed the tests and they all pass now.

@taktoa
Copy link
Author

taktoa commented Aug 22, 2018

You mentioned in the standup that the net effect on performance was neutral; this is unsurprising as blake2s.js is unlikely to be anywhere near as fast as the real implementation in C (and AFAIK it can't take advantage of SIMD). So it is still worth investigating. I'm going to work on benchmarking the Haskell radix tree, and if I come to the conclusion that the performance impact is positive, then I'll merge this PR and the corresponding PR on hs-dfinity-radix-tree.

@taktoa
Copy link
Author

taktoa commented Sep 4, 2018

https://github.com/dfinity-lab/hs-dfinity-radix-tree/pull/18 has now been merged. If the decrease in coverage reported by coveralls is acceptable, then this should be ready to merge.

@taktoa taktoa closed this Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants