IRC meeting summary for 2017-06-08

• Link to this week logs
• Meeting minutes by meetbot

Main topics

• Optimization: Calculate block hash less times
• UI interaction with pertxout upgrade
• crc32 leveldb 1.20

Optimization: Calculate block hash less times

background

Currently accepting a block at the tip hashes the header ~6 times. Jtimon has made PR #10339 to improve that situation. Wumpus did some benchmarks and it resulted in 26% less hashing operations.

comments

Gmaxwell suggested to cache the hash in the block object, but Sipa prefered this solution. He considers adding more arguments to validation-specific functions less invasive than changing a primitive datastructure. Wumpus argues passing extra arguments is easier to reason about than extending primitive datastructures, however caching is always somewhat risky and error-prone. Wumpus thinks if it’s not worth it performance wise we shouldn’t do either.

Morcos wonders if the speedup is worth the tradeoff of making the code a little more complicated/involved. Gmaxwell brought the issue up as the repeated hashing is on the latency critical path for block propagation to the tune of maybe a millisecond. Codeshark prefers to sacrifice a little performance for better architecture.

Jtimon thinks people not agreeing with the concept should’ve made this clear earlier.

conclusion

• Discuss further after the meeting and on PR #10339.

UI interaction with pertxout upgrade

background

PR #10195 which switched the chainstate database and its cache from a per-tx model to a per-txoutput model requires an upgrade process in the database which can take a couple of minutes on decent hardware or longer elsewhere. Sipa thinks this needs some GUI interaction to make it clear to users what’s happening.

comments

Jonasschnelli proposes to use uiInterface.Progress, however that doesn’t allow you to interrupt the process. Users might want to delay the upgrade process to another time.

Luke-jr wonders what happens if you crash, run the old version, and later the new one again. Gmaxwell thinks the old version will tell you the database is corrupt and stop, but he hasn’t tested it. He does think it’s a case that should be handled.

Going back to an old version would require a reindex, something a pruned node can’t do. There should be clear warnings in the release notes.

Sipa notes there’s a trivial change that could be made to guarantee old versions will treat it as an empty database. One way of doing it would be to create a new database, however that would need twice the disk storage during the upgrade. The trivial way would be to set the record of the best block hash to something invalid.

conclusion

• Jonasschnelli will work on the logging process, similar to VerifyDB.
• Monitor disk usage during upgrade and do some more testing. Continue discussion based on those results.

crc32 leveldb 1.20

background

The most recent version of levelDB implements hardware accelerated crc32 for intel which is used for calculating checksums.

comments

Sipa really dislikes the approach the developers of levelDB are using, which requires a separate object compiled with different flags and they’re calling the new object without knowing the CPU supports it. Wumpus and Gmaxwell note compiling a separate object with special flags is standard and correct, however calling it without knowing whether the CPU supports it is not.

Jtimon proposes to open an issue on the levelDB github. Gmaxwell thinks it’s better to just submit a fix, as opening an issue won’t help much.

Cfields, who joined the meeting later, has a fix ready.

conclusion

• Fix levelDB

High priority review

• Sipa wants to add #10148 (Use non-atomic flushing with block replay) which will double the effective available dbcache.
• Luke-jr has rebased multiwallet.
• Gmaxwell reminds everyone BlueMatt’s caching change on #10192 is a 31% block connection speedup, and it needs review.

Comic relief

9:45   cfields_         here!
9:47   BlueMatt         oh, i was supposed to mention cfields_ would be late
9:47   cfields_         heh, thanks
9:47   BlueMatt         you're welcome :)

9:48   gmaxwell         we should submit a fix, it should be trivial.
9:48   cfields_         that's done already: https://github.com/theuni/bitcoin/commit/2cb7dda13884e44105f33c16e7e2c1a9aed46295
9:48   sipa             cfields_: oh!
9:48   cfields_         or are you guys talking about something else?
9:48   sipa             probably not
9:49   wumpus           lol <long discussion> oh, cfields did it already

Participants

Disclaimer

This summary was compiled without input from any of the participants in the discussion, so any errors are the fault of the summary author and not the discussion participants.