19:00:10 <wumpus> #startmeeting
19:00:10 <lightningbot> Meeting started Thu May  3 19:00:10 2018 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:10 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:13 <promag> hi
19:00:26 <jamesob> hi
19:00:29 <wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator
19:00:32 <jnewbery> hello
19:00:37 <kanzure> hi.
19:00:42 <achow101> hi
19:00:43 <cfields> hi
19:00:56 <jonasschnelli> hi
19:01:05 <jimpo> hi
19:01:07 <jcorgan> lurking as usual
19:01:23 <wumpus> any other proposed topics?
19:01:51 <jamesob> logging at some point?
19:02:04 <wumpus> what about logging?
19:02:16 <jamesob> I'd like to talk about moving it into a separate thread
19:02:25 <wumpus> ok
19:02:37 <BlueMatt> #12934
19:02:38 <jnewbery> +1 to moving logging to a separate thread
19:02:39 <gribble> https://github.com/bitcoin/bitcoin/issues/12934 | [WIP] [net] [validation] Call ProcessNewBlock() asynchronously by skeees · Pull Request #12934 · bitcoin/bitcoin · GitHub
19:02:41 <wumpus> let's start with the usual topic
19:03:06 <wumpus> even though it will piss off BlueMatt :)
19:03:07 <wumpus> #topic High prioriity for review
19:03:14 <wumpus> https://github.com/bitcoin/bitcoin/projects/8
19:03:52 <wumpus> only #12979 #12560 #10757 left now
19:03:55 <gribble> https://github.com/bitcoin/bitcoin/issues/12979 | Split validationinterface into paralell validation/mempool interfaces by TheBlueMatt · Pull Request #12979 · bitcoin/bitcoin · GitHub
19:03:59 <gribble> https://github.com/bitcoin/bitcoin/issues/12560 | [wallet] Upgrade path for non-HD wallets to HD by achow101 · Pull Request #12560 · bitcoin/bitcoin · GitHub
19:04:05 <gribble> https://github.com/bitcoin/bitcoin/issues/10757 | RPC: Introduce getblockstats to plot things by jtimon · Pull Request #10757 · bitcoin/bitcoin · GitHub
19:04:16 <wumpus> anything to discuss about those?
19:04:25 <wumpus> anything new to add? or remove?
19:04:49 <jimpo> I'd like #12254 to get some review :-)
19:04:52 <gribble> https://github.com/bitcoin/bitcoin/issues/12254 | BIP 158: Compact Block Filters for Light Clients by jimpo · Pull Request #12254 · bitcoin/bitcoin · GitHub
19:05:22 <jnewbery> There's a whole series of sequential PRs on wallet load/create/unload. It'd be good to start moving through those, but I don't know how high priority they are for others
19:05:42 <wumpus> jnewbery: if it's blocking anyone, the first one on the list should be on there at least
19:05:48 <jtimon> perhaps put the load one in high priority?
19:05:52 <jnewbery> #10740
19:05:56 <gribble> https://github.com/bitcoin/bitcoin/issues/10740 | [wallet] `loadwallet` RPC - load wallet at runtime by jnewbery · Pull Request #10740 · bitcoin/bitcoin · GitHub
19:06:14 <jnewbery> It's only blocking because there are other ones queued behind it
19:06:25 <BlueMatt> jnewbery: that is the most common form of blocking
19:06:26 <wumpus> so it's blocking the continuation ones
19:06:33 <jimpo> That's basically the definition of blocking...
19:06:35 <wumpus> right, it's pretty much the definition of blocking
19:06:37 <wumpus> lol
19:06:39 <jonasschnelli> I agree that we should add 10740 to the list
19:06:46 <wumpus> #10740 given me an unicorn though
19:06:51 <gribble> https://github.com/bitcoin/bitcoin/issues/10740 | [wallet] `loadwallet` RPC - load wallet at runtime by jnewbery · Pull Request #10740 · bitcoin/bitcoin · GitHub
19:07:01 <jonasschnelli> has also a unicorn, reload solved
19:07:15 <wumpus> yes
19:07:16 <LukeJr> unicorns probably have a high street value
19:07:37 <wumpus> added
19:07:39 <jnewbery> not any more. The market's been flooded
19:07:49 <BlueMatt> topic: 0.15.1
19:07:50 <LukeJr> shows what I know of unicorn markets
19:07:52 <BlueMatt> err 16
19:08:12 <wumpus> LukeJr: yes, I"m trying to farm them and sell them, but I have more now than atoms in the knows universe so you could say the supply is more than the demand...
19:08:20 <promag> wumpus: could also add #13097 too?
19:08:22 <gribble> https://github.com/bitcoin/bitcoin/issues/13097 | ui: Support wallets loaded dynamically by promag · Pull Request #13097 · bitcoin/bitcoin · GitHub
19:08:57 <wumpus> promag: ok
19:09:11 <promag> ty
19:09:34 <wumpus> #topic Delete 0.8, 0.9 and 0.10 git branches
19:09:43 <MarcoFalke> Background: Those last tagged versions on those branches are EOL for more than a year now. The tags can be kept for archival reasons, but the branches are no longer required. See https://bitcoincore.org/en/lifecycle/#schedule
19:09:49 <wumpus> yes
19:09:53 <BlueMatt> ack
19:09:57 <wumpus> bye
19:10:00 <achow101> ack
19:10:02 <jonasschnelli> ack
19:10:17 <jtimon> I didn't know we still had those
19:10:18 <LukeJr> is the latest commit of each tagged?
19:10:34 <LukeJr> If not, maybe a 0.n_final tag is appropriate
19:10:35 <jonasschnelli> LukeJr: i hope so
19:10:48 <wumpus> LukeJr: agree, will check that first
19:10:58 <cfields> er, are they not tagged from their respective branches?
19:11:06 <LukeJr> jonasschnelli: it wouldn't be the first time we have backported fixes and never released with them
19:11:06 <MarcoFalke> LukeJr: For the 0.8 one not
19:11:23 <MarcoFalke> The others are last tag == tip of branch, last time I checked
19:11:42 <wumpus> will create a v0.8_final for that
19:11:49 <wumpus> +tag
19:12:02 <LukeJr> maybe it can be a non-object tag, to avoid confusing users with a recent date
19:12:33 <wumpus> yes fine with me, just to avoid losing history
19:12:48 <MarcoFalke> Oh, and the 0.9 one is missing one commit (upnp)
19:13:06 <sipa> back
19:13:27 <wumpus> MarcoFalke: so same for 0.9 then
19:14:03 <wumpus> #topic Moving logging to a separate thread (jamesob)
19:14:44 <jamesob> after working on #13099, I think it may be worthwhile to move logging into a separate thread
19:14:46 <gribble> https://github.com/bitcoin/bitcoin/issues/13099 | Use thread names in logs and deadlock diagnostics by jamesob · Pull Request #13099 · bitcoin/bitcoin · GitHub
19:15:03 <jamesob> esp. given instances like #12970
19:15:04 <BlueMatt> ACKACKACKACKACKACKACKACKACKACK (this is a surprisingly high lag-creator for miners, at least for those with spinning-disk-backed-or-cloud-hosted machines
19:15:06 <gribble> https://github.com/bitcoin/bitcoin/issues/12970 | logging: bypass timestamp formatting when not logging by theuni · Pull Request #12970 · bitcoin/bitcoin · GitHub
19:15:17 <wumpus> queue log messages to a ring buffer ?
19:15:26 <jamesob> wumpus: sure, something along those lines
19:15:32 <wumpus> gmaxwell has been proposing that for ages...
19:15:35 <BlueMatt> see-also commit which does this at https://github.com/bitcoinfibre/bitcoinfibre/commit/6b6a3aef0663775b63bac7d0aa07ec5fc4eb9fc9
19:15:45 <jnewbery> I started working on a branch for this last year, but didn't get very far
19:15:51 <jnewbery> definite concept ACK
19:15:56 <wumpus> yes, concept ACK
19:16:16 <BlueMatt> I did not propose it upstream as it means if we LogPrintf(); assert(false) we'll likely miss it
19:16:18 <jnewbery> means the message processing thread can't be blocked on i/o hangs
19:16:21 <BlueMatt> and I figured folks wouldnt want that
19:16:30 <skeees> ACK - should point out that it could make crash debugging substantially more complicated
19:16:39 <BlueMatt> jnewbery: it still will be a ton cause it ReadBlockFromDisk()s
19:16:40 <wumpus> BlueMatt: we could have special priority log calls that always make it to disk?
19:16:46 <jamesob> skeees: right, I think that's the only caveat I can think of
19:16:50 <LukeJr> BlueMatt: surely there's a way to run log flushing at assert
19:16:53 <wumpus> BlueMatt: CriticalLogPrintf?
19:16:53 <promag> +1 BlueMatt point
19:16:56 <cfields> are there any possible interactions where someone might be tailing the log and assume that it's synchronous?
19:17:04 <jonasschnelli> use gdb
19:17:15 <BlueMatt> wumpus: I mean the number of times its been helpful just to see which lines actually made it to debug.log when someone submitted a crash is super helpful
19:17:21 <LukeJr> cfields: it couldn't be?
19:17:25 <BlueMatt> and gdb isnt really an option when debugging remotely over github issues :/
19:17:32 <jimpo> Does anyone know the relative performance of logging to console vs debug log file?
19:17:50 <BlueMatt> jimpo: on non-serial-consoles, console logging should be fast, serial consoles can block
19:17:54 <LukeJr> in theory, we could fork() a logging-only process, but that feels ugly
19:17:56 <BlueMatt> (or vtt on a slow-refresh-monitor)
19:17:59 <sipa> We should fork a separate process for logging, and then open a FIFO with it and pipe the log data there. If bitcoind crashes, that process hopefully survives :)
19:18:02 <sipa> !hi5 LukeJr
19:18:03 <gribble> Error: "hi5" is not a valid command.
19:18:17 <wumpus> it's mostly a win for low priority debugging messages, I guess
19:18:21 <BlueMatt> or we could just make it optional
19:18:26 <wumpus> if logging is low volume it's never a bottleneck
19:18:35 <BlueMatt> miners can enable it, everyone else probably doesnt need to
19:18:47 <wumpus> only miners that have debug=net enabled?
19:18:51 <skeees> just throwing out some alternatives with a properly tuned buffer cache - logging shouldn't hit disk that often? if its still flushing frequently - could consider a more compact binary format for logs - though that means you'd need a special tool to read them
19:19:18 <jimpo> I feel like disabling logging to file and piping stdout through tee or something should work fine
19:19:38 <wumpus> skeees: well the logging is unbuffered at this moment so can't do much worse than that...
19:19:38 <BlueMatt> wumpus: I mean -debug should always mean non-async loggin, I think
19:19:50 <cfields> skeees: it's not just hitting disk, it's serialization as well. Though I guess we couldn't defer serialization if references are passed in.
19:19:51 <wumpus> even line-buffered would be better for performance
19:20:12 <jamesob> wumpus: I thought fwrite was buffered?
19:20:12 <wumpus> BlueMatt: right
19:20:15 <skeees> another option is to have debug logging compiled out
19:20:25 <skeees> instead of just disabled via flag as it is currently
19:20:30 <wumpus> jamesob: yes, but the buffer is disabled after opening
19:20:40 <BlueMatt> I mean I dont think people who want performance dont want a debug.log
19:20:48 <BlueMatt> they just dont want it to result in 8-ms pauses
19:21:00 <wumpus> skeees: that's not the issue; debug logging is already not executed if it's disabled
19:21:03 * LukeJr wonders if we skip serialization when the debug log level is such that it will not be logged
19:21:08 <wumpus> skeees: even formatting is bypassed in that case
19:21:19 <wumpus> LukeJr: yes
19:21:24 <skeees> LukeJr: yeah that's exactly the question i was trying to ask
19:21:26 <skeees> ah ok
19:21:58 <skeees> i dunno actually
19:22:02 <wumpus> tbh I don't think there's an actual issue here
19:22:09 <skeees> because theres a lot of uint256.ToString() stuff
19:22:15 <cfields> operations on incoming vars aren't skipped though, I believe. Like LogPrint("foo: %s", foo.get())
19:22:18 <BlueMatt> the only issue here is for folks who care a shitload about small disk pauses
19:22:22 <wumpus> although the ring buffer would be nice because of gmaxwell's argument (log at higher debug message, but don't log the messages to disk unless a crash)
19:22:22 <BlueMatt> hence why I use it in fibre
19:22:58 <BlueMatt> when we get ReadBlockFromDisk to be async for peer handling, then it may make more sense to revisit this
19:23:11 <wumpus> so a crash would log the last low-priority debug messages even if debugging is disabled.. then agian, it's not possible to bypass formatting anymore in that case
19:23:25 <BlueMatt> we'd have to swap assert() for dump_log_and_crash()
19:23:36 <wumpus> not sure that even requires logging to be in a sepaerate thread
19:23:41 <LukeJr> wumpus: not sure if formatting is a big deal tbh
19:23:50 <LukeJr> BlueMatt: or handle SIGABRT?
19:23:51 <BlueMatt> no, but then I dont get to upstream my lockless ring buffer logger :p
19:23:54 <wumpus> LukeJr: well some messages are extremely high volume
19:24:01 <BlueMatt> luke-jr: I dont think we want to do that much work in a signal handler
19:24:03 <wumpus> LukeJr: try enabling *all* debugging for fun
19:24:31 <BlueMatt> matt@bitcoin-seednode:~$ ls -lha ~/.bitcoin/debug.log
19:24:31 <BlueMatt> -rw------- 1 matt matt 8.6T May  3 19:24 /home/matt/.bitcoin/debug.log
19:24:41 <wumpus> heh
19:24:51 <sipa> how long?
19:24:54 <LukeJr> BlueMatt: if it's plain old C code, we can probably get away with it
19:25:04 <wumpus> hence my proposal at some point to split up net logging into low and high volume messages
19:25:14 * jamesob buys BlueMatt a pair of socks with the logrotate manpage on them
19:25:23 <BlueMatt> sipa: 2014-11-19 00:06:55
19:25:26 <wumpus> #12219
19:25:28 <gribble> https://github.com/bitcoin/bitcoin/issues/12219 | More granular net logging · Issue #12219 · bitcoin/bitcoin · GitHub
19:25:40 <cfields> wumpus: +1. Will have a look.
19:26:02 <BlueMatt> I do like the flush-debug-ring-to-disk-on-crash idea
19:26:09 <wumpus> yes
19:26:09 <jamesob> +1
19:26:24 <BlueMatt> (would mean serializing all debug messages, though)
19:26:38 <LukeJr> maybe even on non-crash critical errors
19:26:57 <BlueMatt> non-crash critical errors should crash :p
19:27:09 <wumpus> it's in the word 'critical'
19:27:55 <jamesob> okay so unless anyone has any objections, I'll start working on a thread-consumes-from-ring-buffer implementation in the near future
19:28:04 <wumpus> I think I killed the topic
19:28:05 <wumpus> oh
19:28:10 <jnewbery> jamesob: ACK
19:28:21 <BlueMatt> jamesob: I think only as optional except for disabled-debug-messages
19:28:37 <jamesob> i.e. async logging should be opt-in?
19:28:38 <BlueMatt> jamesob: but if upstream wants to maintain by fibre patches, fine by me :p
19:28:41 <BlueMatt> yes
19:28:48 <BlueMatt> also, you may want to start with https://github.com/bitcoinfibre/bitcoinfibre/commit/6b6a3aef0663775b63bac7d0aa07ec5fc4eb9fc9
19:29:07 <jamesob> I'll give it a look, thanks
19:29:42 <wumpus> #topic 0.16.1 (BlueMatt)
19:30:06 <BlueMatt> so for those who weren't paying attention, skeees found some particularly novel races in block handling in #13092
19:30:07 <gribble> https://github.com/bitcoin/bitcoin/issues/13092 | ActivateBestChain concurrency issues · Issue #13092 · bitcoin/bitcoin · GitHub
19:30:21 <BlueMatt> cause they're threading issues they almost certainly wont effect anyone except submitblock users
19:30:29 <BlueMatt> ie miners, and only rare races
19:30:42 <BlueMatt> but, still, I think given that and some of the other various fixes we've had, it may be worth backporting
19:31:11 <wumpus> 13092 is an issue, not a PR, can't label it for backport
19:31:14 <BlueMatt> further, at least personally, I'd kinda like to see #11423 make some movement, and making that kind of policy change in a non-minor-version strikes me as weird
19:31:17 <gribble> https://github.com/bitcoin/bitcoin/issues/11423 | [Policy] Make OP_CODESEPARATOR and FindAndDelete in non-segwit scripts non-std by jl2012 · Pull Request #11423 · bitcoin/bitcoin · GitHub
19:31:25 <BlueMatt> yes, it needs a pr, sdaftuar has a branch, I believe, but not open
19:31:35 <sdaftuar> skeees has a pr, one sec
19:31:46 <skeees> #13023
19:31:46 <sdaftuar> #13023
19:31:48 <BlueMatt> was the skeees pr updated for the new method?
19:31:48 <gribble> https://github.com/bitcoin/bitcoin/issues/13023 | Always refresh most work chain when reacquiring cs_main in ActivateBestChain() by skeees · Pull Request #13023 · bitcoin/bitcoin · GitHub
19:31:50 <gribble> https://github.com/bitcoin/bitcoin/issues/13023 | Always refresh most work chain when reacquiring cs_main in ActivateBestChain() by skeees · Pull Request #13023 · bitcoin/bitcoin · GitHub
19:31:55 <sdaftuar> no we need to settle on the right fix
19:32:01 <skeees> still needs a bit of work - i need to pull in some fixes @sdaftuar made
19:32:09 <sdaftuar> i think the fix i proposed works, but maybe someone has a better idea
19:32:26 <wumpus> ok added tags
19:32:27 <BlueMatt> ok, yes, I think that is doable, however, and I think between that and making progress on the standardness changes from jl2012 probably are worth a minor bump
19:32:37 <skeees> there's also #12988 - which is a similar type of bug
19:32:40 <gribble> https://github.com/bitcoin/bitcoin/issues/12988 | Hold cs_main while calling UpdatedBlockTip() signal by skeees · Pull Request #12988 · bitcoin/bitcoin · GitHub
19:33:07 <BlueMatt> yes, indeed, that too
19:33:18 <sdaftuar> agreed
19:33:30 <wumpus> ok
19:33:55 <BlueMatt> (so, obviously, for those following along at home, next-milestone usually gets auto-high-priority-for-review status :p)
19:34:19 <wumpus> re: #11423, I rebased that one for jl2012, not sure if there's further things to be done there
19:34:22 <gribble> https://github.com/bitcoin/bitcoin/issues/11423 | [Policy] Make OP_CODESEPARATOR and FindAndDelete in non-segwit scripts non-std by jl2012 · Pull Request #11423 · bitcoin/bitcoin · GitHub
19:35:12 <wumpus> added 0.16.1 tag there too
19:35:18 <BlueMatt> I believe jl2012 wanted to add one more policy rule there, which should be done, there was a branch floating around but I cant find it atm
19:35:25 <BlueMatt> anyway, thats for discussion on that pr
19:35:47 <BlueMatt> I'll ping jl2012 since he's probably asleep
19:36:04 <BlueMatt> anyway, seems no disagreement, so next topic?
19:36:06 <wumpus> yes, but it is the open PR with the most (ut)ACKs so I thought it'd be almost ready for merge
19:36:34 <wumpus> that's why I rebased it with the idea to merge it after final review
19:36:38 <wumpus> but ok
19:36:54 <MarcoFalke> Yeah, would need some re-ACKs first, though
19:37:19 <wumpus> right, but if the goalposts keep moving
19:37:41 <wumpus> I think we've exhausted all proposed topics
19:37:52 <MarcoFalke> If there are minor fix ups, they should probably happen soon
19:38:21 <BlueMatt> oh, no, wait i had another topic
19:38:34 <BlueMatt> topic: activate segwit
19:38:38 <BlueMatt> topic 2: <BlueMatt>  #12934
19:38:40 <gribble> https://github.com/bitcoin/bitcoin/issues/12934 | [WIP] [net] [validation] Call ProcessNewBlock() asynchronously by skeees · Pull Request #12934 · bitcoin/bitcoin · GitHub
19:39:23 <BlueMatt> its certainly not ready for review, but we should maybe have a discussion about what concurrency across peers is gonna look like
19:39:24 <wumpus> #topic Call ProcessNewBlock() asynchronously  (skeees)
19:39:40 <BlueMatt> there are two main approaches, but both end up requiring similar refactors for the majority of their work
19:39:51 <BlueMatt> in the past I've looked at doing ProcessMessages() in parallel
19:40:07 <BlueMatt> in this pr skeees moves the validation processing of txn/blocks into a queue and does that in a separate thread
19:40:31 <sipa> how does that interact with eg sending a ping after a block, and expecting it to be processed after the pong returns?
19:40:34 <BlueMatt> in both cases, we end up building logic to "pause" processing of a peer until whatever message it just generated has been processed
19:40:36 <wumpus> that sounds sensible, though I'm really afraid of c++ threading
19:40:43 <BlueMatt> sipa: ^
19:40:51 <gmaxwell> Seems like it would add more locking overhead.
19:40:55 <BlueMatt> (which would also likely be useful for eg ReadBlockFromDisk moving to be more async)
19:41:31 <sipa> sound reasonable to me
19:41:32 <BlueMatt> and a ton of logic to move CNodeState out of cs_main (likely by creating a CNodeState2 during transision)
19:41:43 <BlueMatt> cfields: may have more thoughts
19:41:43 <sipa> that sounds even better
19:41:48 <skeees> the other side benefit of this approach is that ultimately it may simplify the concurrency model inside validation
19:41:53 <sipa> though everything depends on the details...
19:42:06 <BlueMatt> gmaxwell: indeed, thats a major difference between the skeees approach and the parallel ProcessMessages approach
19:42:09 <skeees> having a single thread processing from an ordered queue would have also solved the concurrency issues discussed before
19:42:21 <BlueMatt> ProcessMessages will still do the work on the processing thread, which should have a bit less locking overhead
19:42:21 <cfields> BlueMatt: just reading now. at a glance, this approach sounds similar to what I had in mind as well
19:42:28 <BlueMatt> but is definitely more complicated than the skeees approach
19:42:38 <BlueMatt> which draws a much cleaner boundary between validation and net_processing
19:42:41 <gmaxwell> The biggest gain I'm aware of from concurrency is that right now when connecting multiple blocks at a time (due to out of order fetching in IBD) we stop downloading new blocks, ... fixing that would be a big gain; so it might be worth prioritizing improvements that would let that happen.
19:42:54 <cfields> BlueMatt: I don't see why they'd be mutually exclusive?
19:43:07 <BlueMatt> gmaxwell: that and relaying compact block gettxn during block connection
19:43:25 <BlueMatt> which is the one big cheapish win left for block-relay-latency
19:43:29 <gmaxwell> (e.g. esp when fetching from slow peers, watching network traffic during IBD is comical... transfering at only a couple mbps for a while then nothing for seconds at a time while it connects)
19:43:34 <BlueMatt> cfields: well doing both is maybe not so useful
19:43:54 <BlueMatt> since they'd accomplish largely the same thing
19:44:13 <gmaxwell> BlueMatt: Fair enough for gettxn, though right now my own node makes almost no gettxn requests... and orphaning rates appear to be exceptionally low.
19:44:30 <gmaxwell> (so what I'm saying is that at the moment I don't think of latency improvements as that critical)
19:44:33 <BlueMatt> gmaxwell: the skeees approach is certainly better for doing things like deserialization of blocks coming in from other peers while calling ProcessNewBlock/ActivateBestChain
19:44:52 <BlueMatt> indeed, right now (with mempool ~empty) compact block performance is ~perfect
19:44:55 <BlueMatt> its not always so, however
19:45:16 <gmaxwell> hm. interesting point that making deserialization concurrent would potentially be a multi-core gain.
19:45:17 <BlueMatt> also, fibre nodes see more getblocktxn than miners
19:45:17 <sipa> by "mempool ~empty" you mean "mempool is a perfect match" ?
19:45:33 <BlueMatt> which would be equally solved by miners delaying 1 second before including new txn in blocks....
19:45:54 <gmaxwell> sipa: if each block mostly clears the mempool, miner-specific prioritizaition of transactions doesn't cause as much surprise txn.
19:46:13 <gmaxwell> BlueMatt: and also making use of the CB facility for transmitting extra txn proactively.
19:46:24 <BlueMatt> yea, that too
19:46:52 <BlueMatt> anyway, so tl;dr: I was a bigger fan of the skeees approach and wanted a little bit of concept discussion on the idea of putting a queue in front of the entire validation stuff
19:46:59 <BlueMatt> since thats a huge departure from current operation
19:47:10 <BlueMatt> but I think the potential gain is nice
19:47:17 <BlueMatt> plus all our blocks are already passed in as shared_ptrs anyway.....
19:47:21 <gmaxwell> so right my point was that speading up IBD is probably a big win, .. improving latency and what not would be nice too but I think it's less important.
19:47:42 <LukeJr> +1
19:47:43 <BlueMatt> yes, right now that is definitely true
19:47:51 <BlueMatt> certainly this has the potential to address both, however
19:47:56 <gmaxwell> making use of more cores during sync would be nice too, e.g. if deserilization and hashing can happen on seperate threads async with connection.
19:48:13 <BlueMatt> we can do CheckBlock on the net_processing thread
19:48:27 <BlueMatt> (since, and sipa is gonna die a little bit inside, here, the result is cached in the CBlock structure)
19:48:44 <sdaftuar> that is kinda gross :)
19:48:52 <BlueMatt> its incredibly gross
19:48:59 <gmaxwell> (e.g. multiple message handling threads, which do deserialization and maybe stateless checks)
19:49:21 <wumpus> oh no...
19:49:24 <bitcoin-git> [13bitcoin] 15practicalswift opened pull request #13163: Make it clear which functions that are intended to be translation unit local (06master...06internal-linkage) 02https://github.com/bitcoin/bitcoin/pull/13163
19:49:51 <gmaxwell> though I did a dumb hack a while back that reordered some of the block validation steps to interleave checks that operate per transaction and got a few percent sync speed improvement... we should be mindful of memory locality and not introduce even more passes over blocks.
19:50:05 <BlueMatt> gmaxwell: dunno about multiple message handling threads being doable in the immediate future, but doing CheckBlock on the message processing thread (which does merkle root verification, which is a good chunk of the work) may be worth a chunk
19:50:27 <skeees> cool - so if no strong objections or concerns with the approach i'll continue this and come back when its more ready for review
19:51:07 <gmaxwell> Funny, I would have thought handling seperate peers in seperate threads would almost just work now, vs stuff that handled messages from the same peer concurrently, which violates protocol assumptions about ordering without special work to add processing barriers.
19:51:25 <BlueMatt> gmaxwell: CNodeState requires cs_main, so....not even close :(
19:51:40 <gmaxwell> (almost just work, because the critical data structues have locks ... oh CNodeState)
19:51:45 <BlueMatt> I've got some old branches that do that, if you want to look
19:51:48 <gmaxwell> cs_main lite.
19:51:50 <BlueMatt> but they're....not small changes
19:52:03 <BlueMatt> and cfields hates them, though he's probably right
19:52:26 <BlueMatt> anyway, so </topic> </meeting> ["DIE", "XML"]?
19:52:29 <gmaxwell> in particular our most commons messages are transaction invs and get datas, and those should only need mempool stuff for much of their activity.
19:52:36 <gmaxwell> k.
19:52:44 <BlueMatt> gmaxwell: yes, I did all that in an old branch
19:52:51 <BlueMatt> that in particular is much closer than it used to be
19:53:00 <BlueMatt> cause now the HandleGetData stuff cant take cs_main at the top
19:53:10 <BlueMatt> IIRC that refactor landed for 0.16
19:54:08 <BlueMatt> https://github.com/TheBlueMatt/bitcoin/commits/2017-07-paralell-processmessages-redux
19:55:13 <gmaxwell> Thanks.
19:55:44 <BlueMatt> (it got a bunch of time in helgrind, too, and got all the issues fixed, so I'm at least kinda confident in it, but its essentially unreviewable, even by my standards)
19:57:40 <wumpus> #endmeeting