12015-11-06T00:12:21  *** phantomcircuit has quit IRC
  22015-11-06T00:12:58  *** phantomcircuit has joined #bitcoin-core-dev
  32015-11-06T00:41:40  *** PRab has quit IRC
  42015-11-06T01:17:25  *** mcelrath has quit IRC
  52015-11-06T01:24:23  *** Ylbam has quit IRC
  62015-11-06T01:30:30  *** zooko has quit IRC
  72015-11-06T01:31:27  *** dcousens has joined #bitcoin-core-dev
  82015-11-06T01:35:43  *** belcher has quit IRC
  92015-11-06T01:36:25  *** belcher has joined #bitcoin-core-dev
 102015-11-06T01:53:01  *** phantomcircuit has quit IRC
 112015-11-06T01:58:02  *** phantomcircuit has joined #bitcoin-core-dev
 122015-11-06T02:09:23  *** bsm1175321 has joined #bitcoin-core-dev
 132015-11-06T02:12:19  *** Thireus has quit IRC
 142015-11-06T02:40:00  *** fanquake has joined #bitcoin-core-dev
 152015-11-06T03:02:49  *** belcher has quit IRC
 162015-11-06T03:33:46  *** randy-waterhouse has joined #bitcoin-core-dev
 172015-11-06T03:42:07  <Luke-Jr> a claim/report that running std::bad_alloc can cause a corrupt chainstate.. https://www.reddit.com/r/Bitcoin/comments/3rp0jb/another_dos_attack_how_to_safely_restart_node/
 182015-11-06T03:43:51  *** CodeShark has quit IRC
 192015-11-06T03:45:37  *** JackH has quit IRC
 202015-11-06T03:46:06  *** CodeShark_ has joined #bitcoin-core-dev
 212015-11-06T04:02:14  *** PRab has joined #bitcoin-core-dev
 222015-11-06T04:09:01  <fanquake> jonasschnelli How easy is it for your nightly builder to spin up builds that contain multiple PRs?
 232015-11-06T04:32:36  <fanquake> Thinking #6931/2 + #6918 + #6954 + #6851
 242015-11-06T04:33:43  <dcousens> what is "Work Queue depth exceeded" coming from the bitcoin rpc
 252015-11-06T04:33:50  <dcousens> Its not JSON encoded either
 262015-11-06T04:33:59  <dcousens> Guessing something we didn't catch in libevent?
 272015-11-06T04:34:16  <dcousens> Seems to happen if I smash the RPC with a few hundred requests
 282015-11-06T04:34:58  <jgarzik> an internal libevent queue of requests waiting to be serviced by the rpc thread(s)
 292015-11-06T04:35:00  <fanquake> Look at the http request callback function in httpserver
 302015-11-06T04:36:13  <dcousens> Where is that limit set?
 312015-11-06T04:36:16  <jgarzik> basically a bunch of HTTPWorkItem's
 322015-11-06T04:36:40  <dcousens> also, wouldn't it be better to reply using application logic (aka, the RPC) rather than just "Work queue depth exceeded"
 332015-11-06T04:36:49  <jgarzik>    int workQueueDepth = std::max((long)GetArg("-rpcworkqueue", DEFAULT_HTTP_WORK
 342015-11-06T04:37:04  <dcousens> ta, why isn't my search finding this
 352015-11-06T04:37:05  <dcousens> lol
 362015-11-06T04:37:53  <fanquake> Need to reindex :p
 372015-11-06T05:04:50  <gmaxwell> sdaftuar: FWIW, in memory UTXO during a IBD currently peaks out at 5486.7MB according to updatetip messages.
 382015-11-06T05:05:55  <sipa> gmaxwell: would be interested in what that number is with #6914
 392015-11-06T05:06:18  <gmaxwell> I'll benchmark a reindex next, and then a reindex with 6914
 402015-11-06T05:06:24  <sipa> great
 412015-11-06T05:06:56  <gmaxwell> currently waiting on stopnode.... turns out flushing a 5468 MB cache takes an awful long time.
 422015-11-06T05:07:29  <gmaxwell> about a minute and a half.
 432015-11-06T05:09:12  <gmaxwell> FWIW that sync from the network with no checkpoints took 3hrs 39 minutes.
 442015-11-06T05:09:43  *** randy-waterhouse has quit IRC
 452015-11-06T05:20:13  <gmaxwell> https://people.xiph.org/~greg/utxo-memory-2015-11-06.png
 462015-11-06T05:20:17  <gmaxwell> :(
 472015-11-06T05:21:24  *** guest234234 has joined #bitcoin-core-dev
 482015-11-06T05:21:43  *** guest234234 has left #bitcoin-core-dev
 492015-11-06T05:22:11  *** guest234234 has joined #bitcoin-core-dev
 502015-11-06T05:38:10  *** guest234234 has quit IRC
 512015-11-06T06:01:15  *** fanquake has quit IRC
 522015-11-06T06:08:48  *** Thireus has joined #bitcoin-core-dev
 532015-11-06T06:18:40  <dcousens> gmaxwell: IBD?
 542015-11-06T06:20:05  *** ParadoxSpiral has joined #bitcoin-core-dev
 552015-11-06T06:22:27  <phantomcircuit> gmaxwell, libsecp?
 562015-11-06T06:23:37  <gmaxwell> dcousens: initial block download
 572015-11-06T06:23:39  <gmaxwell> phantomcircuit: yes.
 582015-11-06T06:24:20  <dcousens> gmaxwell: and before libsecp?
 592015-11-06T06:25:11  <phantomcircuit> "more"
 602015-11-06T06:25:14  <gmaxwell> probably also about 3 hours.
 612015-11-06T06:25:38  <gmaxwell> also with no signature validation.. also about three hours.
 622015-11-06T06:25:47  <gmaxwell> can't really make use of all the cpus on this host.
 632015-11-06T06:25:56  <dcousens> IO bound?
 642015-11-06T06:26:36  <gmaxwell> no, it's cpu bound in all the things that aren't signature validation.
 652015-11-06T06:26:41  <dcousens> haha
 662015-11-06T06:26:52  <gmaxwell> but thats what you get with over 8 cores or so.
 672015-11-06T06:28:39  <gmaxwell> (in particular it gets bound on the leveldb, block handing, hashing, etc. and that inserts bubbles in the downloading pipeline)
 682015-11-06T06:29:06  <phantomcircuit> oh no signature validation
 692015-11-06T06:29:08  <phantomcircuit> heh
 702015-11-06T06:31:01  <gmaxwell> phantomcircuit: no, this is with signature validation, specifically I'm testing the libsecp256k1 pull.
 712015-11-06T06:31:18  <gmaxwell> But if I bench with none it won't be much faster.
 722015-11-06T06:31:35  <gmaxwell> or openssl much slower. (because it'll just manage to use more of the cpus)
 732015-11-06T06:33:31  <phantomcircuit> ohh
 742015-11-06T06:35:01  <phantomcircuit> gmaxwell, it'll be a bit faster but yeah
 752015-11-06T06:35:14  *** ParadoxSpiral has quit IRC
 762015-11-06T06:36:04  *** Ylbam has joined #bitcoin-core-dev
 772015-11-06T06:57:46  <wumpus> dcousens: the work queue is part of the HTTP server, happens before requests are handed off to the appliction logic (REST or RPC)
 782015-11-06T06:59:08  <dcousens> wumpus: all good :), just hadn't received it before, caught me off guard
 792015-11-06T07:01:02  <wumpus> don't know if the default number is high enough, do you hit it under quasi-normal circumstances? at some point under heavy 'abuse' it's better to reject clients instead of adding them to an overlong queue (also to prevents the requests from filling up memory), but the default of 16 may well be too conservative
 802015-11-06T07:11:50  <GitHub62> [bitcoin] MarcoFalke opened pull request #6958: [trivial] Cleanup maxuploadtarget (doc & log) (master...MarcoFalke-2015-maxupload) https://github.com/bitcoin/bitcoin/pull/6958
 812015-11-06T07:20:16  *** randy-waterhouse has joined #bitcoin-core-dev
 822015-11-06T07:21:25  *** AtashiCon has quit IRC
 832015-11-06T07:24:37  <phantomcircuit> gmaxwell, bleh it's really annoying nto having a MAC for the plaintext key
 842015-11-06T07:24:44  <phantomcircuit> any opposition to storing one?
 852015-11-06T07:31:03  *** AtashiCon has joined #bitcoin-core-dev
 862015-11-06T07:31:44  <wumpus> what plaintext key and MAC are you talking about?
 872015-11-06T07:31:54  * jonasschnelli is also wondering...
 882015-11-06T07:33:09  <phantomcircuit> oh the encrypted wallet entires
 892015-11-06T07:33:12  <phantomcircuit> entires*
 902015-11-06T07:33:17  <phantomcircuit> entries*
 912015-11-06T07:33:41  <phantomcircuit> the only way to tell which keys go to which master key (technically we support more than one... kind of) is to derive the public key
 922015-11-06T07:33:42  <wumpus> why do you want to change wallet encryption? it works fine
 932015-11-06T07:34:06  <phantomcircuit> it's very slow and corruption can only be detected when you unlock
 942015-11-06T07:34:07  <jonasschnelli> i think phantomcircuits optimizations makes sense
 952015-11-06T07:34:41  <phantomcircuit> we're basically using the private key -> public key derivation as a hash function right now
 962015-11-06T07:34:45  <phantomcircuit> which doesn't make a lot of sense
 972015-11-06T07:34:52  <wumpus> requiring an explicit pubkey derivation was on purpose, although due to AES padding trick you don't actually need tot do it
 982015-11-06T07:35:06  <wumpus> what do you want to optimize? what is not fast enough?
 992015-11-06T07:35:15  <wumpus> 'trying more passphrases per second'? :p
1002015-11-06T07:36:05  <gmaxwell> phantomcircuit: why do you need one for the plaintext key? you're checking the ciphertext.
1012015-11-06T07:36:52  <phantomcircuit> gmaxwell, the current check will detect whether you have multiple master keys mixed together into a single wallet file
1022015-11-06T07:36:55  <wumpus> what are we even trying to accomplish here? what is too slow that needs to be optimized?
1032015-11-06T07:37:09  <wumpus> I really don't like aimless optimization in the context of crypto code :)
1042015-11-06T07:37:30  <gmaxwell> wumpus: please see his existing PR, thie performance is not the main focus (though its quite slow with large wallets)
1052015-11-06T07:37:51  <gmaxwell> the existing behavior has no integrity checking at all for keys in an encrypted wallet, which is not very safe.
1062015-11-06T07:37:57  <phantomcircuit> wumpus, the main focus is detecting corruption on load instead of at the first unlock
1072015-11-06T07:38:06  <wumpus> what I want to avoid is to make it really fast but insecure
1082015-11-06T07:38:35  <gmaxwell> Nothing he's talking about would reduce security.
1092015-11-06T07:38:40  <wumpus> then hashing database records (eg, add a checksum to the data) will work well enough
1102015-11-06T07:38:47  <wumpus> no need to mess with the crypto
1112015-11-06T07:38:57  <gmaxwell> wumpus: Yes thats what his PR does.
1122015-11-06T07:39:08  <wumpus> ok, then what's the problem?
1132015-11-06T07:39:58  <gmaxwell> he's trying to also eliminate the need for the really slow check on unlock, it sounds like. (which is problamatically slow for wallets with very large numbers of keys)
1142015-11-06T07:40:18  <gmaxwell> the same is avoided already for non-encrypted wallets.
1152015-11-06T07:40:22  <wumpus> that never used to be the case, we only have to check one key/value pair
1162015-11-06T07:40:30  <wumpus> if you can assure the database is correct some other way
1172015-11-06T07:40:50  <wumpus> at some point someone wanted to check *all* keys on first unlock, as an integrity check
1182015-11-06T07:40:59  <wumpus> but that's not necessary if you add one at the db level
1192015-11-06T07:41:07  <phantomcircuit> that's what it does now :)
1202015-11-06T07:41:15  <wumpus> yeah, I heavily disagreed with that
1212015-11-06T07:41:18  <gmaxwell> wumpus: We _do_ check all of them on first unlock, as there is no other integrity check.
1222015-11-06T07:41:45  <wumpus> that indeed makes the first unlock slow on big wallets, that was clear, I even commented that back then
1232015-11-06T07:41:55  <phantomcircuit> gmaxwell, if we're OK with saying all the encrypted keys in the wallet have to be from the same master key
1242015-11-06T07:42:02  <gmaxwell> Sure, and thats still better than having no integrity check.
1252015-11-06T07:42:04  <phantomcircuit> then we dont need the hash on the plaintext key
1262015-11-06T07:42:05  <wumpus> anyhow, adding an integrity mechanism at the db level sounds good to me
1272015-11-06T07:42:15  <phantomcircuit> but then we should assert if there's more than 1 master key entry
1282015-11-06T07:42:39  <wumpus> but please don't change this code unnecessarily
1292015-11-06T07:44:08  <gmaxwell> phantomcircuit: does it actually work correctly now if there is more than one?
1302015-11-06T07:44:43  <phantomcircuit> gmaxwell, it correctly fails
1312015-11-06T07:44:57  <phantomcircuit> with the hashes it will silently not fail
1322015-11-06T07:45:02  <phantomcircuit> which would be double plus not good
1332015-11-06T07:45:28  *** Thireus has quit IRC
1342015-11-06T07:45:29  <gmaxwell> ah so without the exhaustive test it won't know that some are encryted but with another key.
1352015-11-06T07:45:38  <wumpus> if you just add a checksum to the data, how can it add paths where it will silently work? you just add more checks, so more failures
1362015-11-06T07:46:05  <gmaxwell> wumpus: if he adds the checksum and turns off the exhaustive test (now not so important with the checksum)
1372015-11-06T07:46:17  <gmaxwell> phantomcircuit: so make it fail the load if there is more than one master key and call it done.
1382015-11-06T07:46:17  <wumpus> right
1392015-11-06T07:46:37  <wumpus> so that was the same in most actual releases (before the exhaustive check was added)
1402015-11-06T07:47:17  <wumpus> agreed
1412015-11-06T07:47:28  <phantomcircuit> sounds reasonable to me :)
1422015-11-06T07:47:38  <wumpus> at this point having more master keys *means* corruption
1432015-11-06T07:47:55  <wumpus> are you protecting all records or just the keys?
1442015-11-06T07:48:29  <wumpus> in any case, an assert to only have one master key is a good sanity check
1452015-11-06T07:48:43  <wumpus> who knows what happens with more, did anyone ever test that part...
1462015-11-06T07:49:42  <phantomcircuit> wumpus, it's unfortunately a huge nuisance to protect all the records
1472015-11-06T07:49:57  <phantomcircuit> i should look at doing that more carefully though...
1482015-11-06T07:50:01  <wumpus> phantomcircuit: ok, then doing just the keys is a good compromise
1492015-11-06T07:50:28  <wumpus> it is the most important data in the wallet
1502015-11-06T07:50:35  <gmaxwell> key records are already protected, ckey are not. but yea, most concerning is keys, and I think its a reasonable compromise.
1512015-11-06T07:51:13  <wumpus> if this was starting from scratch I'd want to do it for all data, but I can believe you if you say that becomes too ugly now
1522015-11-06T07:54:43  <phantomcircuit> hmm maybe it's not as hard as i was thinking
1532015-11-06T07:55:44  <wumpus> we should keep in mind that older versions should still be able to open the wallet
1542015-11-06T07:56:04  *** guest234234 has joined #bitcoin-core-dev
1552015-11-06T07:56:34  <phantomcircuit> wumpus, yeah i need to look at the serialization format to be sure
1562015-11-06T07:59:50  <jonasschnelli> does -debug=bench affect the performance in a recognizable way?
1572015-11-06T08:00:21  <wumpus> it shouldn't - I think the timings are always done, just not logged normally
1582015-11-06T08:00:52  <wumpus> so any performance overhead will be in generating more log data
1592015-11-06T08:01:08  <gmaxwell> really chatty log messages do, but the debug bench log data is not that chatty.
1602015-11-06T08:01:49  <jonasschnelli> okay. thanks... i do no compare the IBD time of the secp256k1 branch with master (with standard parameters)
1612015-11-06T08:06:35  <jonasschnelli> *now
1622015-11-06T08:07:31  <phantomcircuit> are there windows builds for 6954 ?
1632015-11-06T08:09:49  <gmaxwell> As far as protecting all the data goes, --- assuming it could be compatible it would be useful, though otoh, it would be nice to keep the checksums for pubkeys around in memory so we could test against at point of use to reduce the window for corruption further.
1642015-11-06T08:12:17  <jonasschnelli> phantomcircuit: sure: https://bitcoin.jonasschnelli.ch/pulls/6954/
1652015-11-06T08:20:30  *** Thireus has joined #bitcoin-core-dev
1662015-11-06T08:30:13  <gmaxwell> phantomcircuit: 3h 16m for the reindex on that host.
1672015-11-06T08:30:58  *** randy-waterhouse has quit IRC
1682015-11-06T08:33:02  *** tripleslash has quit IRC
1692015-11-06T08:42:26  *** NLNico has joined #bitcoin-core-dev
1702015-11-06T08:54:38  *** tripleslash has joined #bitcoin-core-dev
1712015-11-06T09:15:53  <arowser> gmaxwell: I got the the error: ‘EVENT_LOG_WARN’ when try to build master on ubuntu12.04, the libevent version is 2.0-5, and I found you got the same error in 10-21 https://botbot.me/freenode/bitcoin-core-dev/2015-10-21/?msg=52437191&page=2
1722015-11-06T09:16:23  <arowser> Is that a known issue
1732015-11-06T09:18:13  <wumpus> yes, that is a known issue, should be easy to solve by adding some ifdefs
1742015-11-06T09:20:10  *** davec has quit IRC
1752015-11-06T09:48:03  *** davec has joined #bitcoin-core-dev
1762015-11-06T10:11:18  *** guest234234 has quit IRC
1772015-11-06T10:16:11  *** arowser_ has joined #bitcoin-core-dev
1782015-11-06T10:16:42  *** arowser_ has quit IRC
1792015-11-06T10:21:39  *** arowser_ has joined #bitcoin-core-dev
1802015-11-06T10:26:33  *** CodeShark_ has quit IRC
1812015-11-06T10:28:46  *** paveljanik has quit IRC
1822015-11-06T10:37:17  *** arowser_ has quit IRC
1832015-11-06T10:57:50  *** BashCo has joined #bitcoin-core-dev
1842015-11-06T10:57:55  *** rubensayshi has joined #bitcoin-core-dev
1852015-11-06T11:19:56  *** JackH has joined #bitcoin-core-dev
1862015-11-06T11:28:48  <GitHub98> [bitcoin] MarcoFalke opened pull request #6961: luke-jr constants (master...luke-jr-const) https://github.com/bitcoin/bitcoin/pull/6961
1872015-11-06T12:13:23  *** dcousens has quit IRC
1882015-11-06T12:20:22  *** dcousens has joined #bitcoin-core-dev
1892015-11-06T12:26:05  <GitHub13> [bitcoin] MarcoFalke opened pull request #6962: translations: Don't translate markup or force English grammar (master...MarcoFalke-2015-translations) https://github.com/bitcoin/bitcoin/pull/6962
1902015-11-06T12:28:27  *** arowser_ has joined #bitcoin-core-dev
1912015-11-06T12:30:59  *** dcousens has quit IRC
1922015-11-06T12:51:09  *** ParadoxSpiral has joined #bitcoin-core-dev
1932015-11-06T12:57:43  *** dcousens has joined #bitcoin-core-dev
1942015-11-06T12:58:07  *** aburan28 has quit IRC
1952015-11-06T13:01:59  *** guest234234 has joined #bitcoin-core-dev
1962015-11-06T13:08:20  <GitHub153> [bitcoin] laanwj closed pull request #6825: Backport bugfixes to 0.11 (2015-10-22 / f2c869a) (0.11...backport-bugfixes-to-0.11-20151014) https://github.com/bitcoin/bitcoin/pull/6825
1972015-11-06T13:08:21  <GitHub120> [bitcoin] laanwj pushed 18 new commits to 0.11: https://github.com/bitcoin/bitcoin/compare/df616ae43ed4...6c31ac019f1b
1982015-11-06T13:08:22  <GitHub120> bitcoin/0.11 9b9acc2 Diego Viola: Fix spelling of Qt
1992015-11-06T13:08:22  <GitHub120> bitcoin/0.11 01878c9 Alex Morcos: Fix locking in GetTransaction....
2002015-11-06T13:08:23  <GitHub120> bitcoin/0.11 b3eaa30 MarcoFalke: [Qt] Raise debug window when requested...
2012015-11-06T13:18:49  *** dcousens has quit IRC
2022015-11-06T13:33:58  <GitHub30> [bitcoin] laanwj pushed 1 new commit to 0.11: https://github.com/bitcoin/bitcoin/commit/4e895b08da5dc2dfa94ccefb24632062481a8d97
2032015-11-06T13:33:58  <GitHub30> bitcoin/0.11 4e895b0 Pieter Wuille: Always flush block and undo when switching to new file...
2042015-11-06T13:37:06  <sdaftuar> gmaxwell: sipa: off the top of your head, do you have a sense of the relative speed of calculating a sha256 hash versus performing a signature validation using secp?
2052015-11-06T13:38:54  <jgarzik> 256x2 itym?
2062015-11-06T13:39:59  <sdaftuar> yeah probably that is what i mean
2072015-11-06T13:41:47  <sdaftuar> actually it might not be what i mean, i think #6918 (which i'm analyzing performance on now) does a single sha256 if i'm reading the code right
2082015-11-06T13:52:30  *** arowser_ has quit IRC
2092015-11-06T13:57:31  *** danielsocials has joined #bitcoin-core-dev
2102015-11-06T13:58:53  *** MarcoFalke has joined #bitcoin-core-dev
2112015-11-06T14:09:47  *** treehug88 has joined #bitcoin-core-dev
2122015-11-06T14:15:59  *** molly has quit IRC
2132015-11-06T14:16:20  *** molly has joined #bitcoin-core-dev
2142015-11-06T14:26:33  *** MarcoFalke has quit IRC
2152015-11-06T14:29:42  *** Guyver2 has joined #bitcoin-core-dev
2162015-11-06T14:41:08  <morcos> wumpus: re my comment on flushing undo files.  i think i didn't realize that a reindex could pick up from where it left off.  so it still reads through all the block files, but if the hash is in mapBlockIndex it skips processing?
2172015-11-06T14:41:23  <wumpus> yes
2182015-11-06T14:41:25  <morcos> i was assuming the corruption happened after a reindex finished, but before the flush
2192015-11-06T14:41:37  <morcos> ok, thx
2202015-11-06T14:42:16  <wumpus> it used to remember where it was, but this is complicated by block files being possibly out of order, so now it scans from the beginning again after restart (but this goes pretty fast)
2212015-11-06T14:43:21  *** Thireus has quit IRC
2222015-11-06T15:12:07  *** Thireus has joined #bitcoin-core-dev
2232015-11-06T15:37:03  *** dixson3 has joined #bitcoin-core-dev
2242015-11-06T16:00:12  *** danielsocials has quit IRC
2252015-11-06T16:24:43  *** jtimon has quit IRC
2262015-11-06T16:27:20  *** zooko has joined #bitcoin-core-dev
2272015-11-06T16:28:27  *** danielsocials has joined #bitcoin-core-dev
2282015-11-06T16:34:23  *** Naphex has quit IRC
2292015-11-06T16:46:53  <jonasschnelli> IBD up to 382324 with secp256k1 branch took 9 hours...
2302015-11-06T16:46:55  <jonasschnelli> standard parameters
2312015-11-06T16:47:12  <jonasschnelli> Quad Core Intel(R) Xeon(R) CPU E31245 @ 3.30GHz
2322015-11-06T16:47:47  <jonasschnelli> 16GB ram (but i did not change -dbcache)
2332015-11-06T16:50:02  *** guest234234 has quit IRC
2342015-11-06T16:50:10  <jonasschnelli> (now doing the same with current master [openssl], just curios how the performance benefit are with standard args on a standard system)
2352015-11-06T16:54:09  *** danielsocials has quit IRC
2362015-11-06T17:03:03  *** rubensayshi has quit IRC
2372015-11-06T17:05:40  <gmaxwell> sdaftuar: a verify with libsecp256k1 as it's used in bitcoin core is 280 times slower than a single run of the sha256 compression function.  That PR ends up calling the sha256 compression function twice.
2382015-11-06T17:08:47  <gmaxwell> sdaftuar: I think at some point we should add a faster cryptgraphic hash to the source code base for these internal non-normative uses. (E.g. blake2 is a bit over 5x faster than sha2-256; though if the user has a not-yet-released cpu with a sha256 instruction, sha256 will be faster again).
2392015-11-06T17:10:09  <sdaftuar> gmaxwell: thanks.  i was investigating whether the use of sha256 in the PR might cause an unexpected slowdown...  after some more investigation i think my concern was misplaced, it looks like any "slowdown" is pretty minuscule, and i think i wasn't hitting the conditions under which the PR would provide expected speedups.  will keep testing...
2402015-11-06T17:10:52  <gmaxwell> sdaftuar: figuring out the if there was any tradeoff there is part of why pieter was measuring the cache performance with many sizes (because a small amount of hitrate increase offsets any constant cost).  Also, the remove operation in the current code ends up being pretty slow compared to the hashing, in fact.
2412015-11-06T17:11:17  <sdaftuar> because of map versus unordered_map?
2422015-11-06T17:13:15  <gmaxwell> Yes.
2432015-11-06T17:16:07  <gmaxwell> sipa: 6954 results--- so reindex with libsecp256k1,  without 6954 was 3hr 16 minutes, reindex with it was _2 hours_ 7 minutes.  Going to benchmark without signature validation.
2442015-11-06T17:17:47  <gmaxwell> UTXO in memory at the end was 4012 MB.
2452015-11-06T17:35:19  <sipa> gmaxwell: so from 5486 MB to 4012 MB, because of 6914?
2462015-11-06T17:35:37  <sipa> gmaxwell: 6954 is libsecp validation, so i guess you mean 6914 instead?
2472015-11-06T17:38:11  <morcos> sipa: is 6914 something you think should be considered for 0.12.  i haven't really looked at it much, but i'd happily test it out if you think its a near term consideration?
2482015-11-06T17:38:24  <gmaxwell> sipa: oops yes.
2492015-11-06T17:39:58  <sipa> morcos: i'm not sure what i can do more for it than get review
2502015-11-06T17:40:12  <morcos> gmaxwell's test seems to indicate there is a HUGE speed improvment, thats from allocation overhead? i think that would be something nice to test in my Connect Block benching
2512015-11-06T17:40:43  <morcos> sipa: understood, but it wasn't clear to me from your previous conversation with gmaxwell whether you even wanted to do 6914 or wanted to wait and do a bigger change
2522015-11-06T17:40:45  <sipa> morcos: as far as my reading of the specification is concerned, i think it's fully compatible with std::vector
2532015-11-06T17:40:57  <sipa> morcos: i don't think a bigger change is in scope
2542015-11-06T17:41:19  <sipa> i mean... i'd like to change allocation of scripts to some pooled storage per block and tx
2552015-11-06T17:41:22  <gmaxwell> One third less sync time and 25% less memory consumption is pretty nice.  These results don't surprise me -- well, so I'm a little surprised that _just_ fixing the scripts did this, I would be less surprised if the whole cache entry were made a single allocation.
2562015-11-06T17:42:11  <morcos> sipa: yes thats what i was referring to, but that doesnt mean you don't want 6914 for now is what i'm clarifying, you still do want 6914
2572015-11-06T17:43:22  <morcos> gmaxwell, i was seeing a significant chunk of ConnectBlock time being just destroying the temporary cache at the end.
2582015-11-06T17:44:23  <morcos> while i have you both here.  do you have any thoughts on how we could get rid of the 1 remaining database lookup for the coinbase in 6932
2592015-11-06T17:44:49  <morcos> i hate that you have to access the DB for every block just in case you're processing the 2 historical violations of BIP 30
2602015-11-06T17:46:18  <sipa> looking
2612015-11-06T17:46:27  *** BashCo has quit IRC
2622015-11-06T17:46:56  <jgarzik> morcos, agreed
2632015-11-06T17:47:19  *** fkhan has quit IRC
2642015-11-06T17:47:20  <jgarzik> that wants fixing (as you've proposed)
2652015-11-06T17:48:27  <morcos> maybe i'm trying to overoptimize, but i wasn't able to eliminate checking the database for preexisting coins when you are creating the new outputs for a new coinbase
2662015-11-06T17:53:05  <sipa> morcos: how about writing them and just not marking them as fresh?
2672015-11-06T17:53:47  <sipa> make ModifyNewCoins take a boolean fPossiblePreexisting or something
2682015-11-06T17:53:51  <morcos> that was my first plan, but it assert fails
2692015-11-06T17:54:15  <morcos> the code assumes that if you're not fresh you must exist in the parent
2702015-11-06T17:54:21  <sipa> i see
2712015-11-06T17:54:26  <sipa> maybe that can be relaxed
2722015-11-06T17:54:27  <morcos> i think you coudl remove that assert, but i didn't like making that change
2732015-11-06T17:57:26  <morcos> so the idea i've been hesitant to suggest is to actually check if its one of those two hashes
2742015-11-06T17:57:42  <sipa> yuck
2752015-11-06T17:57:59  <morcos> hence the hesitancy, but its really more clear as to why you're doing what you're doing
2762015-11-06T17:58:14  <morcos> otherwise you have to have a long winded explanation of why you treat a coinbase differently anyway
2772015-11-06T17:58:33  <morcos> also i assume way faster than having to not mark all coinbases as fresh or first check the database for them
2782015-11-06T17:59:43  <morcos> i don't know maybe not faster that not FRESH marking i guess
2792015-11-06T18:00:08  <morcos> anyway, lunch..  i'm happy to do whatever you guys are most comfortable with
2802015-11-06T18:00:47  <sipa> morcos: the only advantage of fresh is that it's removed from memory altogether before hitting disk, if it's spent before flush
2812015-11-06T18:01:37  *** fkhan has joined #bitcoin-core-dev
2822015-11-06T18:02:16  <morcos> sipa: right. so i guess that doesn't happen unless you have a big cache, with coinbases...   but even if it happens a small fraction of the time, a DB write has to be orders of magnitude more expensive than checking 2 hashes on every coinbase
2832015-11-06T18:03:31  *** CodeShark has joined #bitcoin-core-dev
2842015-11-06T18:05:54  *** Guest57961 has quit IRC
2852015-11-06T18:06:07  *** PaulCape_ has quit IRC
2862015-11-06T18:08:00  *** PaulCapestany has joined #bitcoin-core-dev
2872015-11-06T18:09:06  <sipa> morcos: i think removing the assertion is fine
2882015-11-06T18:11:22  <sipa> gmaxwell: my sigcache performance at 320 MB (with validating everything) keeps improving (it's an exponentially decaying average with 0.99 factor per block, so ~half a day halflife), down to 6% now
2892015-11-06T18:12:16  *** pigeons has joined #bitcoin-core-dev
2902015-11-06T18:12:40  *** pigeons is now known as Guest91590
2912015-11-06T18:14:08  <sipa> gmaxwell: feel like commenting on 6914?
2922015-11-06T18:16:50  <sipa> morcos: i guess in 6914 i can add more comments that refer to the specification and unit tests for the iterators
2932015-11-06T18:23:27  *** BashCo has joined #bitcoin-core-dev
2942015-11-06T18:36:37  <morcos> sipa: what size in MB would be equivalent to a 1M sigcache of the old version
2952015-11-06T18:37:06  <morcos> i tried 80MB which was my guess from the pull, and verification seemed to get a little faster, but overall run time slowed down
2962015-11-06T18:37:20  <sipa> morcos: it should be around 3 times smaller
2972015-11-06T18:37:38  <sipa> between 70 and 85 bytes per entry now
2982015-11-06T18:37:42  <sipa> around 220 before
2992015-11-06T18:44:46  <gmaxwell> sipa: will comment when I've through testing things with it.
3002015-11-06T18:45:21  <gmaxwell> reindex with 6914 and no signature checks, 1h 17m.
3012015-11-06T18:48:10  *** NLNico has quit IRC
3022015-11-06T18:50:50  *** danielsocials has joined #bitcoin-core-dev
3032015-11-06T18:51:31  *** paveljanik has joined #bitcoin-core-dev
3042015-11-06T18:52:34  <morcos> sipa: did you benchmark adding things to the sigcache, or only looking them up?  I'm wondering if calling GetRand() that often is too expensive.  something slowed down for me for sure...
3052015-11-06T18:52:37  *** JackH has quit IRC
3062015-11-06T18:55:03  *** danielsocials has quit IRC
3072015-11-06T18:55:33  <gmaxwell> morcos: it GetRand's on evicition (and the initial initilization) but not otherwise.
3082015-11-06T18:56:19  <morcos> yeah i guess in the steady state it shouldn't be more than a couple evictions per new tx..   hmm.
3092015-11-06T19:01:12  *** zooko has quit IRC
3102015-11-06T19:01:22  <sipa> morcos: the eviction takes like a microsecond
3112015-11-06T19:02:27  <morcos> yeah i misremembered GetRand speed, but its a tad under a micro, i thought it was a few micros.  still maybe better to use insecure_rand(), but doesn't explain my performance issue
3122015-11-06T19:02:54  <sipa> morcos: in any case, the getrand is certainly the largest cpu time offender in the sigcache behaviour
3132015-11-06T19:02:58  <sipa> more than the hash
3142015-11-06T19:03:07  <morcos> i'm checking to see if i didn't match cache sizes, but i don't think thats the problem, since verification did speed up
3152015-11-06T19:03:19  <morcos> its the rest of the run time that slowed down...
3162015-11-06T19:03:31  <sipa> do you measure this by running two nodes in parallel?
3172015-11-06T19:03:37  <sipa> or just long period of time averaging?
3182015-11-06T19:04:36  *** zooko has joined #bitcoin-core-dev
3192015-11-06T19:05:44  <sipa> morcos: where exactly are you seeing slowdown?
3202015-11-06T19:05:51  <morcos> total time to run a simulation over 7 days, increased by about 5%
3212015-11-06T19:06:10  <sipa> interesting!
3222015-11-06T19:06:48  <morcos> time spent in Connect Block had about a 10% decrease (but this number i've found has a lot of noise, i think due to the vagaries of the level db cache and/or whatever else is happening with my hard disk)
3232015-11-06T19:07:39  <morcos> btw, have you thought about the effect on reorgs
3242015-11-06T19:07:56  <morcos> reorgs are already incredibly slow, and now you've just lost the signature cache for every tx in the block
3252015-11-06T19:08:44  *** zooko` has joined #bitcoin-core-dev
3262015-11-06T19:09:22  <morcos> maybe we can skip signature checking on blocks added back from a reorg?
3272015-11-06T19:09:54  <morcos> txs from the disconnected block added to the mempool that is
3282015-11-06T19:10:19  <morcos> eh, that won't help since a bunch of them will need to be checked if they are included inthe replacement block
3292015-11-06T19:10:26  *** zooko has quit IRC
3302015-11-06T19:12:44  *** zooko has joined #bitcoin-core-dev
3312015-11-06T19:15:11  *** zooko` has quit IRC
3322015-11-06T19:15:59  <morcos> sipa: looks like it might be GetRand().  just ran with insecure_rand and it was a 5% overall speedup instead.  i'm going to try again removing a bunch of extraneous debugging i have.
3332015-11-06T19:18:07  <sipa> morcos: oh, that's an optimization i've thought about before
3342015-11-06T19:18:22  <sipa> morcos: we actually store in the block index whether a block was verified already
3352015-11-06T19:19:05  <morcos> sipa: are you talking about something different?  a re-re-org?
3362015-11-06T19:19:10  <sipa> right
3372015-11-06T19:19:53  <sipa> morcos: it's strange that getrand makes things slower... the earlier version used way more entropy
3382015-11-06T19:19:55  <morcos> i'm just talking about regular reorg, where you add back all the txs from the disconnected block.   so now you have to process on the order of a few thousand txs with literally none of their sigs in the cahce
3392015-11-06T19:20:01  <sipa> by using GetRandBytes()
3402015-11-06T19:20:39  <sipa> morcos: one way to mitigate that would be to not remove sigcache entries during validation, but instead keep a list of txids added in each block in the past, and remove entries a few blocks deep
3412015-11-06T19:21:18  <sipa> eh, nevermind
3422015-11-06T19:21:27  <sipa> that would mean keeping a list of all signature checks
3432015-11-06T19:21:58  <morcos> wait why wouldnt' that work
3442015-11-06T19:22:08  <morcos> just keep a list of entries to remove
3452015-11-06T19:22:17  <sipa> it would work, but it's ugly, large, and a layer violation :)
3462015-11-06T19:22:18  <morcos> every new block add to the back of the list and remove from the front
3472015-11-06T19:22:29  <sipa> we could use greg's idea of annotating the sigcache entries with a sequence number
3482015-11-06T19:22:44  <sipa> and then after every block wipe things with a sequence number a few before
3492015-11-06T19:22:45  <morcos> as for GetRand().  you're right, that doesn't make sense that its slower
3502015-11-06T19:23:10  <morcos> so maybe the speedup from insecure_rand is real, but that still doesn't explain why it was slower in the first place
3512015-11-06T19:23:25  <sipa> can you try with a 5 MB cache?
3522015-11-06T19:23:54  <sipa> maybe the slowdown is just from larger working set
3532015-11-06T19:24:14  <cfields> morcos: something you might try, i discovered this last week when profiling new network code...
3542015-11-06T19:24:21  <morcos> the base case i was comparing against was a 1M entry cache
3552015-11-06T19:24:41  <sipa> 1M entries with old code?
3562015-11-06T19:24:43  <morcos> yes
3572015-11-06T19:25:01  <sipa> does it get filled?
3582015-11-06T19:25:05  <cfields> morcos: the secure_delete openssl allocator was taking ~25% of the time to process a transaction. Commenting that out made a massive difference in the profile
3592015-11-06T19:25:05  <cfields> not sure if that's artificial or if it actually surfaces in the real-world
3602015-11-06T19:25:17  <cfields> s/transaction/message/
3612015-11-06T19:25:26  <morcos> i didn't check, but over 7 days it accepts 1.3M txs to the mempool, so i would assume thats several million sigs
3622015-11-06T19:25:27  <sipa> cfields: wut
3632015-11-06T19:25:53  <cfields> sipa: take that with a grain of salt, it was a very artificial benchmark
3642015-11-06T19:26:14  <sipa> morcos: so an interesting thing i saw is that with master and a 300 M mempool, and delete-on-use-in-block sigcache, i never got higher than 944k sigcache entries
3652015-11-06T19:26:29  <sipa> cfields: what kind of profiling?
3662015-11-06T19:27:02  <morcos> sipa: that seems plausible, but the old code doesn't have delete on use
3672015-11-06T19:27:08  <cfields> sipa: gprof of echoing messages back/forth between nodes
3682015-11-06T19:27:13  <sipa> morcos: in fact, it stayed between 936k and 944k very consistently
3692015-11-06T19:27:42  <cfields> sipa: CDataStream is backed by the zeroafterfree alloc, so i suspect it'd be an issue in many other places
3702015-11-06T19:28:10  <sipa> cfields: yeah, that may be unnecessarily much
3712015-11-06T19:28:58  <cfields> i'd prefer if we were more conservative with where that was used. for non-sensitive data, it just seems wasteful
3722015-11-06T19:29:06  <sipa> indeed
3732015-11-06T19:29:15  <sipa> and network data should never be sensitive
3742015-11-06T19:30:19  <cfields> sipa: right. very likely that that particular case was highly inflated though. It was a worst-case of bouncing thousands of tiny messages around very quickly. Still goes to show the trend, though.
3752015-11-06T19:39:44  *** PaulCape_ has joined #bitcoin-core-dev
3762015-11-06T19:41:59  *** PaulCapestany has quit IRC
3772015-11-06T19:50:33  *** BashCo has quit IRC
3782015-11-06T19:53:29  *** BashCo has joined #bitcoin-core-dev
3792015-11-06T19:56:39  *** zooko has quit IRC
3802015-11-06T20:05:10  *** PaulCapestany has joined #bitcoin-core-dev
3812015-11-06T20:07:54  *** PaulCape_ has quit IRC
3822015-11-06T20:29:37  *** CodeShark_ has joined #bitcoin-core-dev
3832015-11-06T20:29:38  *** CodeShark has quit IRC
3842015-11-06T20:30:04  <morcos> sipa: apologies. that slowdown was an artifact.  i've run it a couple times now, and it is faster over all with 6918.  i'm still seeing 10% speedup in ConnectBlock, but then also an an overall speedup
3852015-11-06T20:30:17  *** zooko has joined #bitcoin-core-dev
3862015-11-06T20:30:38  <morcos> switching to insecure_rand offered incremental improvement over that, but nothing like what i thought i saw before
3872015-11-06T20:35:17  *** CodeShark_ has quit IRC
3882015-11-06T20:39:13  <sipa> morcos: ok, good to know!
3892015-11-06T20:50:35  *** paveljanik has quit IRC
3902015-11-06T20:52:21  *** danielsocials has joined #bitcoin-core-dev
3912015-11-06T21:01:39  *** danielsocials has quit IRC
3922015-11-06T21:06:53  *** ParadoxSpiral has quit IRC
3932015-11-06T21:09:18  *** CodeShark_ has joined #bitcoin-core-dev
3942015-11-06T21:14:35  *** Guyver2 has quit IRC
3952015-11-06T21:15:27  *** CodeShark_ has quit IRC
3962015-11-06T21:22:51  *** CodeShark_ has joined #bitcoin-core-dev
3972015-11-06T21:36:32  *** treehug88 has quit IRC
3982015-11-06T21:59:32  *** danielsocials has joined #bitcoin-core-dev
3992015-11-06T22:21:54  *** CodeShark_ has quit IRC
4002015-11-06T22:32:50  *** CodeShark_ has joined #bitcoin-core-dev
4012015-11-06T22:38:54  *** zooko has quit IRC
4022015-11-06T22:47:50  *** danielsocials has quit IRC
4032015-11-06T23:13:11  <gmaxwell> 18 of the last 101 blocks have been v4.
4042015-11-06T23:14:59  <phantomcircuit> gmaxwell, oon mainnet?
4052015-11-06T23:15:02  <phantomcircuit> that was fast
4062015-11-06T23:16:29  *** belcher has joined #bitcoin-core-dev
4072015-11-06T23:18:05  <gmaxwell> cfields: the zeroafterfree allocator has the benefit of killing making some use after free or use-of-uninitilized memory vulnerabilities unexploitable.
4082015-11-06T23:19:46  <cfields> gmaxwell: mmm
4092015-11-06T23:21:02  <GitHub14> [bitcoin] TheBlueMatt opened pull request #6964: Benchmark sanity checks and fork checks in ConnectBlock (master...verify-commits-fixes) https://github.com/bitcoin/bitcoin/pull/6964
4102015-11-06T23:23:54  <gmaxwell> I dunno if its worth the cost, of course-- but it's not totally pointless.
4112015-11-06T23:24:29  <jgarzik> perhaps makes valgrind happier
4122015-11-06T23:25:26  <GitHub124> [bitcoin] TheBlueMatt opened pull request #6965: Benchmark sanity checks and fork checks in ConnectBlock (master...bench) https://github.com/bitcoin/bitcoin/pull/6965
4132015-11-06T23:28:21  <gmaxwell> nah. it's on free, won't make valgrind happier.
4142015-11-06T23:37:16  <phantomcircuit> gmaxwell, what's the cost?
4152015-11-06T23:52:04  *** danielsocials has joined #bitcoin-core-dev
4162015-11-06T23:55:52  <gmaxwell> phantomcircuit: memory bandwidth.
4172015-11-06T23:57:30  *** danielsocials has quit IRC