IRC meeting summary for 2017-06-08
- Optimization: Calculate block hash less times
- UI interaction with pertxout upgrade
- crc32 leveldb 1.20
Optimization: Calculate block hash less times
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.
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.
- Discuss further after the meeting and on PR #10339.
UI interaction with pertxout upgrade
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.
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.
- 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
The most recent version of levelDB implements hardware accelerated crc32 for intel which is used for calculating checksums.
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.
- 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.
|wumpus||Wladimir van der Laan|
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.