1 2017-08-18 00:00:06 0|gmaxwell|thats a lot, time for a reback.
2 2017-08-18 00:00:10 0|gmaxwell|repack.
3 2017-08-18 00:00:25 0|kanzure|also other unmerged branches though
4 2017-08-18 00:00:37 0|kanzure|sipa might even have elements.git branches in there
5 2017-08-18 00:02:19 0|sipa|no
6 2017-08-18 00:03:10 0|kanzure|welp.
7 2017-08-18 00:10:58 0|luke-jr|cfields: it occurs to me the reason the dir is dirty, is because it's missing files; so if we want to defer doing a real fix, we can alternatively fix the missing-files issue instead, by generating the tarball from git-archive
8 2017-08-18 00:29:52 0|cfields|luke-jr: I believe the dir is dirty because we extract the tarball into it
9 2017-08-18 00:31:11 0|luke-jr|that doesn't make sense..
10 2017-08-18 00:56:31 0|praxeology|https://medium.com/@DCGco/bitcoin-scaling-agreement-at-consensus-2017-133521fe9a77
11 2017-08-18 00:56:54 0|praxeology|sorry my 2yo grabbed my mouse
12 2017-08-18 01:08:44 0|cfields|luke-jr: I see what you mean
13 2017-08-18 01:08:52 0|promag|praxeology: I feel you too
14 2017-08-18 01:16:10 0|Cryptocide|Abra|BitClub Network|Bitcoin.com|BitFury|BitGo|Bitmain|BitPay Blockchain|Bloq|BTCC|Circle|Ledger|RSK Labs|Xapo, no thanks
15 2017-08-18 01:17:43 0|kanzure|why XORing them? i don't get it.
16 2017-08-18 01:17:51 0|kanzure|er, ORing
17 2017-08-18 01:18:33 0|kanzure|yes. anyway.
18 2017-08-18 01:42:37 0|kallewoof|I'm running a modified Bitcoin Core node to do some profiling on where resources are spent (CPU cycles and bandwidth in particular) and am seeing some really weird stuff. E.g:
19 2017-08-18 01:42:42 0|kallewoof|0.77877 27892 282511326 24083459 0 131703419 SendMessages.inventory.trickle (tx-relay).LOCK(cs)
20 2017-08-18 01:43:34 0|kallewoof|The columns are "portion of CPU used", "min CPU cycles", "max CPU cycles", "medium CPU cycles per call", "bandwidth per call", "# of calls", "code path"
21 2017-08-18 01:44:08 0|kallewoof|So the LOCK(cs) in SendMessages for inventory for trickle for the tx relay part is taking 78% of all CPU cycles for my node. Does that seem normal?
22 2017-08-18 01:45:07 0|kallewoof|Also baffled by the number of calls. 131 million. I started profiling yesterday.
23 2017-08-18 01:49:40 0|kallewoof|This is probably from the mempool.info(hash) call in the while loop btw. That probably explains the high # of calls, but 131 million in 24 hours is 1500/second. Maybe my profiling code is broken.
24 2017-08-18 01:51:55 0|kallewoof|... Actually, it rose by 38k from 01:48:49 to 01:50:00, so doesn't seem improbable.
25 2017-08-18 02:23:08 0|gmaxwell|sometimes profiling is confused by locks. (and/or moderately condented locks in an otherwise inactive process can be a high percentage due to spinning).
26 2017-08-18 02:23:13 0|gmaxwell|Sounds like it might be interesting.
27 2017-08-18 02:23:21 0|praxeology|Re: https://docs.google.com/document/d/1y6Hsqdg1xBrJY4dFeKP6y05XCceJoVMs0_M_VwKFReM/edit Maybe it would be a good idea to communicate with the exchanges and check and see who will continue to support Bitcoin Core's chain
28 2017-08-18 02:23:48 0|gmaxwell|Everyone okay with me doing a presentation on 0.15 improvements to a local group in 1.5 weeks?
29 2017-08-18 02:26:03 0|praxeology|Like... I know that BitPay and Coinbase are declaring that they will support Segwit2x... has any exchange declared that they will continue to support Bitcoin (Bitcoin Core's rules)?
30 2017-08-18 02:27:39 0|kallewoof|gmaxwell: there is some amount of overhead as I am doing the profiling on my own. Maybe that's the cause for the high portion of time spent there, but it still seems like a lot of LOCK calls, regardless of actual CPU cycle count. Would be cool if the mempool could be copied once and then not lock cs at all. Code is here btw: https://github.com/kallewoof/bitcoin/tree/profile-resources
31 2017-08-18 02:28:06 0|sipa|copying the whole mempool?
32 2017-08-18 02:28:15 0|kallewoof|Only the hashes.
33 2017-08-18 02:28:20 0|sipa|oh
34 2017-08-18 02:29:10 0|kallewoof|Actually that wouldn't work. It uses txinfo for feeRate and tx etc.
35 2017-08-18 02:29:33 0|praxeology|kallewoof: does your bitcoin process have enough memory to hold the entire chainstate?
36 2017-08-18 02:30:07 0|praxeology|err, what -dbcache are you using?
37 2017-08-18 02:30:09 0|kallewoof|it has 16 GB of RAM. 223 MB free atm.
38 2017-08-18 02:30:32 0|kallewoof|Default
39 2017-08-18 02:30:40 0|kallewoof|(dbcache)
40 2017-08-18 02:31:12 0|praxeology|are you profiling while synching from genesis, or from the latest tip, or what?
41 2017-08-18 02:31:25 0|sipa|praxeology: he's talking about inv relay
42 2017-08-18 02:31:29 0|sipa|not about sync
43 2017-08-18 02:31:30 0|kallewoof|The node is fully synced up. I restarted it with profiling enabled so it's from tip
44 2017-08-18 02:33:49 0|bitcoin-git|[13bitcoin] 15jonasnick opened pull request #11083: Fix combinerawtransaction RPC help result section (06master...06fix-combinerawtransaction-help) 02https://github.com/bitcoin/bitcoin/pull/11083
45 2017-08-18 02:35:45 0|praxeology|kallewoof: is your profiler sampling instruction pointer positions, or is it timing each function call?
46 2017-08-18 02:37:06 0|kallewoof|It's using rdtsc. Not sure which that falls under.
47 2017-08-18 02:37:19 0|praxeology|the latter can really mess up measurements
48 2017-08-18 02:38:54 0|praxeology|and potentially the profile is not actually measuring cpu usage... that could be misleading, instead it could just be saying where a thread is sleeping
49 2017-08-18 02:40:21 0|kallewoof|Right. It definitely keeps ticking even while waiting for locks. I was more intereted in the # of LOCK() calls/second rather than the actual CPU usage in this case, though.
50 2017-08-18 02:41:59 0|kallewoof|Since the code is auto-profiling all locks, I can simply reduct the lock times from the parent to get "time spent excluding lock wait times", if that seemed useful.
51 2017-08-18 02:43:23 0|praxeology|is it measuring the number of lock calls? or the number of times it sampled with the thread being at the Lock() call?
52 2017-08-18 02:43:36 0|kallewoof|# of calls
53 2017-08-18 02:47:03 0|praxeology|Does bitcoin's networking code operate on polling or interrupt?
54 2017-08-18 02:47:40 0|kallewoof|I logged every entry into the tx relay loop and logged the size of vInvTx. I don't have a lot of connections yet (14 or so) but I'm seeing 4-5 per second. Example of vInvTx sizes: 5, 0, 5, 0, 4, 4, 5, 1, 0, 1, 5, 1, 2, 0, 26, 6, 5, 0, 5, 5, 0, 5, 0, 2, 2, 13, 52, 0, 0, 0, 4, 4, 11, 21
55 2017-08-18 02:48:51 0|Fibonacci|o/
56 2017-08-18 02:49:48 0|kallewoof|That doesn't match up with 1500/second at all, but maybe with more connections. Or maybe there was a bunch of txs in the last 24 hours.
57 2017-08-18 02:53:46 0|kallewoof|Actually, for comparison, the path into "trickle (tx-relay)" only has 543732 instances, which would mean there are on average 133867681/543732=246 calls to LOCK(cs) per entry. Huh.
58 2017-08-18 02:56:58 0|cfields|kallewoof: you can use my lock dumper to profile: https://github.com/theuni/bitcoin/commit/be49a294a240ec81a901af1aaabbba2172d38dc1
59 2017-08-18 02:57:04 0|cfields|it should cherry-pick cleanly
60 2017-08-18 02:57:11 0|cfields|lock stats are dumped at shutdown
61 2017-08-18 02:57:20 0|cfields|i should really clean that up and PR it
62 2017-08-18 02:57:28 0|kallewoof|Nice :)
63 2017-08-18 02:58:02 0|cfields|should tell you how long it's locked, and what percentage of the thread time it spent in it
64 2017-08-18 02:58:28 0|kallewoof|I sort of already know that. What is confusing me is the # of times it is locking.
65 2017-08-18 02:58:45 0|kallewoof|350 million in <24h is a lot of LOCK()s.
66 2017-08-18 02:58:51 0|cfields|heh
67 2017-08-18 03:27:33 0|Fibonacci|Bitcointalk will soon be irrelevant in the cryptoverse. Alternative methods for coin legitimization are popping up that will more carefully scrutinize developers intentions and identities, while at the same time allowing for an influx of new blood not associated with the corrupt financial institutions. Keep your eyes open for this shifting to a new paradigm
68 2017-08-18 03:28:17 0|luke-jr|!ops spammer
69 2017-08-18 03:28:50 0|Fibonacci|That wasn't spam Luke-jr
70 2017-08-18 03:28:56 0|luke-jr|sure looks like it.
71 2017-08-18 03:28:57 0|Fibonacci|I typed that myself
72 2017-08-18 03:29:17 0|luke-jr|considering this channel has nothing to do with bitcointalk, altcoins, etc
73 2017-08-18 03:29:20 0|Fibonacci|well I think it's time
74 2017-08-18 03:35:55 0|Cryptocide|People can code whatever they want wherever, and if they have great coding skills they will shine, what is the issue we all agree on this
75 2017-08-18 03:48:35 0|kallewoof|Found part of why there are so many locks. It's locking cs twice for every entry in vInvTx, which is anything from a few up to 100+. With 4-5 per sec that adds up fast. The first lock is for the CompareInvMempoolOrder (std::pop_heap call at start of loop), and the second is for the actual mempool.info(hash) call.
76 2017-08-18 03:55:26 0|kallewoof|Actually, since the CompareInvMempoolOrder is a sorter, and each sort calls CompareDepthAndScore which locks cs, the number of lock calls are dependent on the size of the vector. That definitely explains things...
77 2017-08-18 03:57:58 0|kallewoof|I think this could all be solved by making a sub-mempool which takes a list of hashes and simply pulls them out of the mempool once. These operations could then be done on the sub-mempool without locking anything or at least without locking cs.
78 2017-08-18 05:02:31 0|kallewoof|Woah.. size of vInvTx after running for a bit (grabbed latest bunch in order): 4596, 4, 0, 7, 2, 8, 3836, 4370, 21, 2, 3, 3806, 1, 5, 4340, 16, 16, 0, 4121, 7, 4478, 11, 16, 0, 4095, 4452, 4264, 5, 9, 6, 1, 4061, 11, 11, 4579, 4544, 7, 10, 1, 1, 0, 4318, 18, 54, 3, 3, 1, 1, 0, 0, 4421, 13. 4+k entries would definitely result in a lot of locking.
79 2017-08-18 05:03:29 0|sipa|kallewoof: wow
80 2017-08-18 07:01:41 0|bitcoin-git|13bitcoin/06master 1464fb0ac 15practicalswift: Declare single-argument (non-converting) constructors "explicit"...
81 2017-08-18 07:01:41 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/22e301a3d56d...4afb5aa9e173
82 2017-08-18 07:01:42 0|bitcoin-git|13bitcoin/06master 144afb5aa 15Wladimir J. van der Laan: Merge #10969: Declare single-argument (non-converting) constructors "explicit"...
83 2017-08-18 07:02:21 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10969: Declare single-argument (non-converting) constructors "explicit" (06master...06explicit) 02https://github.com/bitcoin/bitcoin/pull/10969
84 2017-08-18 07:07:24 0|bitcoin-git|[13bitcoin] 15kallewoof opened pull request #11084: [mempool] Mempool snapshots to avoid lots of locking (06master...06mempool-snapshot) 02https://github.com/bitcoin/bitcoin/pull/11084
85 2017-08-18 07:12:52 0|fanquake|wumpus are you merging a few PRs now, or reviewing
86 2017-08-18 07:16:16 0|fanquake|I think 11071, 11066, 11083, 10878 are ready to go. Fairly trivial.
87 2017-08-18 07:29:05 0|wumpus|fanquake: thanks! will take a look
88 2017-08-18 07:42:29 0|fanquake|Also 11076 now that it's squashed and the commit message is fixed.
89 2017-08-18 07:46:51 0|bitcoin-git|13bitcoin/06master 14d1e6f91 15practicalswift: Prefer compile-time checking over run-time checking
90 2017-08-18 07:46:51 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/4afb5aa9e173...dbf6bd6ea05f
91 2017-08-18 07:46:52 0|bitcoin-git|13bitcoin/06master 14dbf6bd6 15Wladimir J. van der Laan: Merge #11071: Use static_assert(ââ¬Â¦, ââ¬Â¦) (C++11) instead of assert(ââ¬Â¦) where appropriate...
92 2017-08-18 07:47:29 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11071: Use static_assert(ââ¬Â¦, ââ¬Â¦) (C++11) instead of assert(ââ¬Â¦) where appropriate (06master...06static_assert) 02https://github.com/bitcoin/bitcoin/pull/11071
93 2017-08-18 07:48:01 0|gmaxwell|kallewoof: copying the mempool ids worries me, what happens when it has a million tiny transactions in it... that will be pretty slow and blow out all the caches. Would it have been possible to use a closure like structure to carry the required operations under a single grab of the lock. (maybe thats worse, haven't benchmarked)
94 2017-08-18 07:49:46 0|wumpus|sounds like something that definitely needs benchmarks
95 2017-08-18 07:51:02 0|kallewoof|Not sure what you mean by ids. Right now the PR I submitted will make a snapshot copying over the txs in the list as CTxMempoolEntry's. That is one single iteration over mapTx in the mempool, comparing each to the given std::set. I think this is faster than iterating mapTx once per hash, but not sure. Yes, proper benchmarks are probably needed...
96 2017-08-18 07:53:00 0|gmaxwell|kallewoof: it's just that there can be a million-ish entries in mapTx. So I think that in extreme cases your snapshot could be quite slow. Though I certantly believe it's faster with the typically small mempools.
97 2017-08-18 07:53:14 0|bitcoin-git|13bitcoin/06master 14f9ca0fe 15Jonas Nick: Fix combinerawtransaction RPC help result section
98 2017-08-18 07:53:14 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/dbf6bd6ea05f...f3558834db4d
99 2017-08-18 07:53:15 0|bitcoin-git|13bitcoin/06master 14f355883 15Wladimir J. van der Laan: Merge #11083: Fix combinerawtransaction RPC help result section...
100 2017-08-18 07:53:54 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11083: Fix combinerawtransaction RPC help result section (06master...06fix-combinerawtransaction-help) 02https://github.com/bitcoin/bitcoin/pull/11083
101 2017-08-18 07:56:59 0|kallewoof|gmaxwell: I realized I am not stopping iteration ever, so I switched the code a bit (1. it will now erase found entries from set, and 2. it will break when set is empty). I will do benchmarks to see how mempool size impacts speed there.
102 2017-08-18 07:57:15 0|bitcoin-git|13bitcoin/06master 1472a184a 15Carl Dong: Update init.md: Fix line breaks in section 3b.
103 2017-08-18 07:57:15 0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f3558834db4d...c58128f18992
104 2017-08-18 07:57:16 0|bitcoin-git|13bitcoin/06master 14c58128f 15Wladimir J. van der Laan: Merge #10878: Docs: Fix Markdown formatting issues in init.md...
105 2017-08-18 07:57:16 0|bitcoin-git|13bitcoin/06master 14d201e40 15Carl Dong: Update init.md: Fix section numbering.
106 2017-08-18 07:57:45 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10878: Docs: Fix Markdown formatting issues in init.md (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/10878
107 2017-08-18 07:59:22 0|kallewoof|... I *am* assuming that iterating over mapTx once is better than simply doing mapTx.find() for each hash. I probably shouldn't assume that.
108 2017-08-18 08:00:27 0|sipa|iterating is O(1) per ste
109 2017-08-18 08:00:33 0|sipa|find is O(log n)
110 2017-08-18 08:01:26 0|kallewoof|sipa: Thanks. So with big mempool and small set, my approach is bad. Will fix.
111 2017-08-18 08:01:37 0|bitcoin-git|13bitcoin/060.15 1430c246b 15practicalswift: Updating the release notes (minor stylistic changes)
112 2017-08-18 08:01:37 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 060.15: 02https://github.com/bitcoin/bitcoin/compare/252ca9c5d8d7...1c4b9b31355f
113 2017-08-18 08:01:38 0|bitcoin-git|13bitcoin/060.15 141c4b9b3 15Wladimir J. van der Laan: Merge #11076: 0.15 release-notes nits: fix redundancy, remove accidental parenthesis & fix range style...
114 2017-08-18 08:10:24 0|kallewoof|gmaxwell: Updated. With k=vInvTx.size(), n=mapTx.size(), pre: O(k log n); post: O(k log n). This is mapTx stuff only. That's the part you were concerned about, right?
115 2017-08-18 08:13:08 0|gmaxwell|okay, thats more interesting.
116 2017-08-18 08:20:26 0|gmaxwell|its late, I must be confused... because I don't understand how this doesn't violate the locking on the multiindex. But I never did understand all the multiindex details all that well.
117 2017-08-18 08:21:17 0|kallewoof|The snapshot is created under lock. Then it's used single-thread-like. How would that violate multiindex locking?
118 2017-08-18 08:23:42 0|gmaxwell|Though.. couple extra points: when it's going to access all the vInvTx it would probably be a lot faster to pull all the things that get compared out at once instead of building a CTxMemPool::indexed_transaction_set.
119 2017-08-18 08:24:02 0|gmaxwell|kallewoof: because there are references being copied, who owns the underlying data?
120 2017-08-18 08:24:43 0|gmaxwell|e.g. what happens when a mempool entry is deleted while you've dropped the lock but are accessing the snapshot.
121 2017-08-18 08:25:32 0|kallewoof|I am pretty sure the CTxMemPoolEntry is copied, not referenced.
122 2017-08-18 08:26:29 0|gmaxwell|the mempool entry contains references.
123 2017-08-18 08:26:37 0|kallewoof|if deleted, the inv may contain the hash of a deleted tx but that would have been the case if you had arrived a fraction of a second earlier, too, so doesn't seem unacceptable
124 2017-08-18 08:26:40 0|gmaxwell|(and it better, you sure as heck don't want to copy all the txn! :) )
125 2017-08-18 08:27:30 0|kallewoof|Oh, yes. But these are accessed without locking already.
126 2017-08-18 08:27:42 0|kallewoof|Or at the least, they are made available after lock is released.
127 2017-08-18 08:27:53 0|sipa|what is accessed?
128 2017-08-18 08:28:00 0|kallewoof|tx, for example.
129 2017-08-18 08:28:05 0|sipa|the mempool contains shared pointers to constant transactions
130 2017-08-18 08:28:17 0|sipa|you can copy the shared pointers under lock
131 2017-08-18 08:28:22 0|sipa|and then lose the lock
132 2017-08-18 08:28:35 0|sipa|and then deref the shared pointers to txn
133 2017-08-18 08:28:38 0|sipa|but nothing else
134 2017-08-18 08:28:46 0|kallewoof|Yes. And if I have a separate structure with pointers to those txs, I can do whatever I want with it, right?
135 2017-08-18 08:28:53 0|sipa|yes
136 2017-08-18 08:28:58 0|kallewoof|Then I think I'm good.
137 2017-08-18 08:28:58 0|sipa|txn are immutable
138 2017-08-18 08:40:23 0|kallewoof|gmaxwell: forgot to reply regarding the vInvTx optimization you mentioned; I'm not entirely sure what you mean. Do you mean I should pull out the TxMempoolInfo objects directly instead of doing the snapshot middle step?
139 2017-08-18 08:41:27 0|kallewoof|I kind of intended for the snapshot class to be useful in other places but if this is the only one that might be better.
140 2017-08-18 08:41:33 0|gmaxwell|The only things that get accessed in the objects IIRC are their depth (and if they're in there or not), which means that you can just fetch all the data that the sort will later access in advance, instead of copying the whole CTxMemPoolEntry object.
141 2017-08-18 08:42:01 0|gmaxwell|Which will probably be very time and memory inefficient (e.g. copying shared pointers)
142 2017-08-18 08:42:09 0|kallewoof|Ahh.. that makes sense.
143 2017-08-18 08:43:36 0|gmaxwell|(also their feerate, perhaps. vague recolletion here, in any case, it's just a couple narrow things. We do something like that for the node sorts for eviction)
144 2017-08-18 08:49:47 0|bitcoin-git|[13bitcoin] 15dooglus opened pull request #11085: Add 'sethdseed' RPC to initialize or replace HD seed. (06master...06set_hd_seed) 02https://github.com/bitcoin/bitcoin/pull/11085
145 2017-08-18 09:28:50 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/c58128f18992...9f60b3707d1e
146 2017-08-18 09:28:51 0|bitcoin-git|13bitcoin/06master 1407685d1 15Jonas Schnelli: Add length check for CExtKey deserialization
147 2017-08-18 09:28:51 0|bitcoin-git|13bitcoin/06master 149f60b37 15Wladimir J. van der Laan: Merge #11081: Add length check for CExtKey deserialization (jonasschnelli, guidovranken)...
148 2017-08-18 09:29:35 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11081: Add length check for CExtKey deserialization (jonasschnelli, guidovranken) (06master...062017/08/fix_cextkey) 02https://github.com/bitcoin/bitcoin/pull/11081
149 2017-08-18 13:22:01 0|bitcoin-git|13bitcoin/06master 145be6e9b 15Wladimir J. van der Laan: doc: Update build-openbsd for 6.1...
150 2017-08-18 13:22:01 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9f60b3707d1e...aeec8b4b6882
151 2017-08-18 13:22:02 0|bitcoin-git|13bitcoin/06master 14aeec8b4 15Wladimir J. van der Laan: Merge #11080: doc: Update build-openbsd for 6.1...
152 2017-08-18 13:22:40 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11080: doc: Update build-openbsd for 6.1 (06master...062017_08_openbsd_bump) 02https://github.com/bitcoin/bitcoin/pull/11080
153 2017-08-18 13:24:15 0|bitcoin-git|[13bitcoin] 15BitonicEelis opened pull request #11087: Diagnose unsuitable outputs in lockunspent(). (06master...06lockunspent) 02https://github.com/bitcoin/bitcoin/pull/11087
154 2017-08-18 13:24:36 0|bitcoin-git|13bitcoin/06master 14bea8e9e 15practicalswift: Document the preference of nullptr over NULL or (void*)0
155 2017-08-18 13:24:36 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/aeec8b4b6882...9e00a625b43c
156 2017-08-18 13:24:37 0|bitcoin-git|13bitcoin/06master 149e00a62 15Wladimir J. van der Laan: Merge #11066: Document the preference of nullptr over NULL or (void*)0...
157 2017-08-18 13:25:17 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11066: Document the preference of nullptr over NULL or (void*)0 (06master...06document-nullptr-preference) 02https://github.com/bitcoin/bitcoin/pull/11066
158 2017-08-18 13:50:51 0|promag|wumpus what about https://github.com/bitcoin/bitcoin/pull/11039 ?
159 2017-08-18 13:55:24 0|wumpus|promag: I'll have a look
160 2017-08-18 13:56:11 0|promag|np,
161 2017-08-18 13:56:17 0|wumpus|I like replacing .count() and [] with one find
162 2017-08-18 13:56:31 0|wumpus|incidentally, [] generates quite a lot of code
163 2017-08-18 13:57:13 0|wumpus|(when used with maps)
164 2017-08-18 13:57:31 0|promag|there is a PR to replace that with .at()
165 2017-08-18 13:57:59 0|promag|the idea there is just to avoid the 2nd lookup
166 2017-08-18 13:58:31 0|wumpus|yes
167 2017-08-18 14:01:56 0|wumpus|but the stanza of using count then [] (sometimes even multiple times) instead of using an iterator always annoyed me (but apparently not enough to patch it myself), happy to see it go
168 2017-08-18 14:06:58 0|luke-jr|might be nice to do a explicittype& varname = it->second; though - but this is definitely an improvement
169 2017-08-18 14:07:26 0|luke-jr|(it's hard to tell what it->second actually is here)
170 2017-08-18 14:09:59 0|wumpus|luke-jr: looks like that's what he does
171 2017-08-18 14:10:06 0|wumpus|luke-jr: only the iterator is assigned to an auto
172 2017-08-18 14:10:16 0|luke-jr|I'm not talking about the iterator ;p
173 2017-08-18 14:10:20 0|promag|yes, I almost did that, but I think the best is to switch those finds with GetWalletTx which is almost never used
174 2017-08-18 14:11:02 0|wumpus|luke-jr: I know, but he does "CWalletTx& wtx = it->second;" in many places
175 2017-08-18 14:11:13 0|wumpus|which matches "explicittype& varname = it->second; " right?
176 2017-08-18 14:11:18 0|luke-jr|yes, it should be in the other places too
177 2017-08-18 14:11:23 0|wumpus|meh
178 2017-08-18 14:11:47 0|promag|those "explicittype& varname = it->second; " were already there
179 2017-08-18 14:12:00 0|wumpus|yes. it's fine.
180 2017-08-18 14:12:34 0|promag|luke-jr: I didn't add new vars
181 2017-08-18 14:12:43 0|luke-jr|promag: I'm saying you should in this case
182 2017-08-18 14:12:57 0|luke-jr|directly using it->second is poor for readability
183 2017-08-18 14:13:39 0|promag|that will cause a bigger diff, out of scope IMO
184 2017-08-18 14:13:41 0|wumpus|please don't change it now, I've just reviewed it
185 2017-08-18 14:13:43 0|wumpus|yeah
186 2017-08-18 14:14:51 0|wumpus|the scope is "avoid second wallet lookup", which he did, The old code was "mapWallet[txin.prevout.hash].MarkDirty();" whichalso had no explicit type.
187 2017-08-18 14:15:05 0|luke-jr|sure
188 2017-08-18 14:15:45 0|promag|luke-jr: I almost did that, just thought those can be improved in a later PR
189 2017-08-18 14:15:49 0|promag|cool
190 2017-08-18 14:17:52 0|luke-jr|even with the nit, it's a utACK, not a NACK ;)
191 2017-08-18 14:17:59 0|luke-jr|still a clear improvement
192 2017-08-18 14:19:52 0|promag|ty
193 2017-08-18 14:27:02 0|bitcoin-git|13bitcoin/06master 148f2f1e0 15João Barbosa: wallet: Avoid second mapWallet lookup
194 2017-08-18 14:27:02 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9e00a625b43c...fc51565cbd4c
195 2017-08-18 14:27:03 0|bitcoin-git|13bitcoin/06master 14fc51565 15Wladimir J. van der Laan: Merge #11039: Avoid second mapWallet lookup...
196 2017-08-18 14:27:40 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11039: Avoid second mapWallet lookup (06master...062017-08-avoid-second-mapwallet-lookup) 02https://github.com/bitcoin/bitcoin/pull/11039
197 2017-08-18 14:36:15 0|promag|This is similar https://github.com/bitcoin/bitcoin/pull/11041/files but does what luke-jr mentioned
198 2017-08-18 14:54:19 0|promag|PR "tagged" with WIP should not be reviewed correct?
199 2017-08-18 15:03:54 0|wumpus|promag: only the general direction, but reviewing whether all i's are dotted is probably a waste of time and effort
200 2017-08-18 15:04:16 0|promag|right
201 2017-08-18 15:28:12 0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/fc51565cbd4c...0e5b7486cb7f
202 2017-08-18 15:28:13 0|bitcoin-git|13bitcoin/06master 141221f60 15John Newbery: [wallet] Remove keypool_topup_cleanups...
203 2017-08-18 15:28:13 0|bitcoin-git|13bitcoin/06master 1467ceff4 15John Newbery: [wallet] Add logging to MarkReserveKeysAsUsed
204 2017-08-18 15:28:14 0|bitcoin-git|13bitcoin/06master 140e5b748 15Wladimir J. van der Laan: Merge #11044: [wallet] Keypool topup cleanups...
205 2017-08-18 15:28:51 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #11044: [wallet] Keypool topup cleanups (06master...06keypool_topup_cleanups) 02https://github.com/bitcoin/bitcoin/pull/11044
206 2017-08-18 16:33:31 0|bitcoin-git|[13bitcoin] 15luke-jr opened pull request #11089: Enable various p2sh-p2wpkh functionality (06master...06p2shp2wpkhstuff) 02https://github.com/bitcoin/bitcoin/pull/11089
207 2017-08-18 16:56:01 0|bitcoin-git|[13bitcoin] 15instagibbs reopened pull request #9017: Enable various p2sh-p2wpkh functionality (06master...06p2shp2wpkhstuff) 02https://github.com/bitcoin/bitcoin/pull/9017
208 2017-08-18 16:57:42 0|bitcoin-git|13bitcoin/06master 14e53615b 15Andrew Chow: Remove vchDefaultKey and have better first run detection...
209 2017-08-18 16:57:42 0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/0e5b7486cb7f...262167393d05
210 2017-08-18 16:57:43 0|bitcoin-git|13bitcoin/06master 142621673 15Wladimir J. van der Laan: Merge #10952: [wallet] Remove vchDefaultKey and have better first run detection...
211 2017-08-18 16:58:15 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10952: [wallet] Remove vchDefaultKey and have better first run detection (06master...06remove-defaultkey) 02https://github.com/bitcoin/bitcoin/pull/10952
212 2017-08-18 16:59:33 0|luke-jr|instagibbs: see #11089
213 2017-08-18 17:13:16 0|instagibbs|luke-jr, ah sorry didn't notice that
214 2017-08-18 17:13:40 0|luke-jr|instagibbs: if you want to take it back, I can close
215 2017-08-18 17:13:49 0|instagibbs|nah, you rebased for me
216 2017-08-18 17:13:51 0|instagibbs|thanks :P
217 2017-08-18 17:13:52 0|luke-jr|although I cut out some of it
218 2017-08-18 17:15:41 0|instagibbs|sure, doesn't have to be done all at once
219 2017-08-18 17:17:42 0|bitcoin-git|[13bitcoin] 15instagibbs closed pull request #9017: Enable various p2sh-p2wpkh functionality (06master...06p2shp2wpkhstuff) 02https://github.com/bitcoin/bitcoin/pull/9017
220 2017-08-18 17:29:22 0|luke-jr|instagibbs: well, I disagree with extending sign/verify ;)
221 2017-08-18 17:34:09 0|bitcoin-git|[13bitcoin] 15Derek701 opened pull request #11090: Update release-notes.md (060.15...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/11090
222 2017-08-18 17:40:03 0|instagibbs|luke-jr, no strong opinions :)
223 2017-08-18 19:42:34 0|warren|https://bitcoincore.org/en/2016/01/26/segwit-benefits/ "(this effectively results in an effective limit closer to 1.6 to 2 MB)." <--- This can be improved, asking for fact checks.
224 2017-08-18 19:43:33 0|warren|1) I don't think 'effective limit" is accurate in describing what is really a practical size compared to pre-segwit transaction uses, which these estimates were really referring to?
225 2017-08-18 19:44:09 0|warren|2) And isn't 1.6MB-2MB the very old estimates, later found in a Bitfury study (?) to be more like 2.1MB? I wonder what it would be now.
226 2017-08-18 19:59:17 0|gmaxwell|warren: luke posted an alaysis patch on reddit, it's about 1.9x IIRC.
227 2017-08-18 20:07:22 0|warren|"effective limit" -> "effective size"?
228 2017-08-18 20:12:52 0|bitcoin-git|[13bitcoin] 15laanwj opened pull request #11091: test: Increase initial RPC timeout to 60 seconds (06master...062017_08_test_wait_for_rpc) 02https://github.com/bitcoin/bitcoin/pull/11091
229 2017-08-18 20:33:31 0|luke-jr|does per-TXO store the outpoint in both the key and the value? O.o
230 2017-08-18 20:36:29 0|sipa|luke-jr: no, only in the key
231 2017-08-18 20:37:32 0|sipa|COutPoint in the key, CTxOut in the value
232 2017-08-18 21:23:11 0|cfields|luke-jr: grr, using git archive won't work either. Ironically, because the substitution from 'git archive' creates a diff.
233 2017-08-18 21:37:24 0|luke-jr|>_<
234 2017-08-18 21:37:38 0|luke-jr|well, we want that anyway I think, so maybe PR it for master at least
235 2017-08-18 21:37:45 0|luke-jr|or at least don't discard it yet
236 2017-08-18 21:38:56 0|luke-jr|cfields: so that leaves either fixing the tarball to build with the desired string, or making src/obj/build.h manually, right?
237 2017-08-18 21:39:20 0|cfields|luke-jr: i don't think we can make it manually, it'll just get overwritten
238 2017-08-18 21:41:08 0|BlueMatt|cfields: can you confirm there is nothing stopping switching sync.h primitives to std?
239 2017-08-18 21:41:44 0|cfields|BlueMatt: sure, let me finish this thing up first though
240 2017-08-18 21:41:52 0|BlueMatt|no rush
241 2017-08-18 21:42:07 0|BlueMatt|cfields: just comment on 10866
242 2017-08-18 21:42:34 0|luke-jr|echo '#!/bin/true' >share/genbuild.sh
243 2017-08-18 21:42:35 0|luke-jr|mkdir -p src/obj
244 2017-08-18 21:42:37 0|luke-jr|echo "#define BUILD_SUFFIX gentoo${PVR#${PV}}" >src/obj/build.h
245 2017-08-18 21:42:39 0|luke-jr|cfields: ^ what I do for Gentoo
246 2017-08-18 21:43:56 0|cfields|BlueMatt: oh. at the very least, CSemaphore/CScheduler/CCheckqueue non-interruptible
247 2017-08-18 21:44:21 0|BlueMatt|hmm?
248 2017-08-18 21:44:34 0|BlueMatt|I dont think CSemaphore needs to be interruptible
249 2017-08-18 21:44:41 0|BlueMatt|CScheduler and CCheckqueue use boost:: directiyl
250 2017-08-18 21:44:43 0|BlueMatt|directly
251 2017-08-18 21:46:25 0|cfields|BlueMatt: ok right, those were preventing the switch to std::thread.
252 2017-08-18 21:46:44 0|BlueMatt|yes
253 2017-08-18 21:46:46 0|cfields|luke-jr: heh
254 2017-08-18 21:48:21 0|cfields|BlueMatt: yea, i guess CSemaphore is ok too. though didn't we decide at one point that the interruptions were saving us?
255 2017-08-18 21:50:37 0|BlueMatt|on CSemaphore? no?
256 2017-08-18 21:50:52 0|cfields|nm, was thinking of e007b24
257 2017-08-18 21:51:21 0|cfields|i guess that commit shows we rely on posting :)
258 2017-08-18 21:51:30 0|BlueMatt|yes, we rely on posting
259 2017-08-18 21:54:04 0|cfields|ok neat, i guess the boost::mutex dep slowly dissolved
260 2017-08-18 21:55:13 0|BlueMatt|=D
261 2017-08-18 21:55:21 0|cfields|luke-jr: ok, want to just do that for gitian?
262 2017-08-18 21:55:32 0|BlueMatt|well, no, not neat, really, we have locking primitives that have useful debug things in them, and a bunch of code is blindly using boost:: directly :(
263 2017-08-18 21:55:34 0|BlueMatt|but oh well
264 2017-08-18 21:55:43 0|cfields|that's really gross, as it completely defeats the purpose of all of the version stuff
265 2017-08-18 21:56:08 0|cfields|but i guess it works for 0.15
266 2017-08-18 21:57:01 0|luke-jr|cfields: personally, I prefer the "just get it fixed" approach, but if we don't have time for that, okay..
267 2017-08-18 21:57:49 0|cfields|luke-jr: well the "just get it fixed" approach involves nuking most of our version handling, no?
268 2017-08-18 21:58:31 0|cfields|i think we'll just take the complexity down to "the tarball injected the version" or "check git" ?
269 2017-08-18 22:00:47 0|luke-jr|cfields: more like using genbuild on the git to generate the stuff used by the tar
270 2017-08-18 22:01:13 0|luke-jr|what should it say, btw?
271 2017-08-18 22:01:22 0|cfields|sure, that works
272 2017-08-18 22:01:34 0|cfields|hmm?
273 2017-08-18 22:01:47 0|luke-jr|I've never seen it without the git hash in the version ;)
274 2017-08-18 22:02:00 0|luke-jr|Bitcoin Core RPC client version v0.15.0
275 2017-08-18 22:02:01 0|luke-jr|?
276 2017-08-18 22:03:44 0|cfields|oh, heh
277 2017-08-18 22:03:54 0|cfields|let me check 0.14.2
278 2017-08-18 22:09:09 0|cfields|splash says "v0.14.2"
279 2017-08-18 22:09:13 0|luke-jr|cfields: working on another branch atm, can you try http://codepad.org/TexBr9BW ?
280 2017-08-18 22:09:31 0|cfields|i think the rc's usually have a git commit tacked on though. trying to find one
281 2017-08-18 22:10:57 0|cfields|heh, good idea
282 2017-08-18 22:12:35 0|cfields|testing
283 2017-08-18 22:17:34 0|cfields|we should nuke the GIT_DIR export, then
284 2017-08-18 22:22:46 0|luke-jr|cfields: yes, probably
285 2017-08-18 22:45:30 0|cfields|luke-jr: it works as-is
286 2017-08-18 22:46:05 0|cfields|luke-jr: i have to run out. you're welcome to PR, or I'll do it tonight when I get back
287 2017-08-18 22:46:17 0|cfields|thanks for the fix :)
288 2017-08-18 22:48:28 0|luke-jr|cfields: np, although it's probably not a good long-term fix..
289 2017-08-18 23:35:05 0|bitcoin-git|[13bitcoin] 15MojaPochi opened pull request #11092: Litecoin ver 0.10.4 for macOS (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/11092
290 2017-08-18 23:36:00 0|bitcoin-git|[13bitcoin] 15fanquake closed pull request #11092: Litecoin ver 0.10.4 for macOS (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/11092