19:00:37 <wumpus> #startmeeting
19:00:37 <lightningbot> Meeting started Thu Jul  2 19:00:37 2020 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:37 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:42 <achow101> hi
19:00:45 <jnewbery> hi
19:00:45 <hebasto> hi
19:00:45 <jkczyz> hi
19:00:48 <jonasschnelli> Hi
19:00:56 <wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral ariard digi_james amiti fjahr
19:00:58 <wumpus> jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
19:01:06 <kanzure> hi
19:01:15 <provoostenator> hi
19:01:17 <meshcollider> hi
19:01:19 <jamesob> hi
19:01:25 <aj> hi
19:01:28 <fjahr> hi
19:01:35 <pinheadmz> hi
19:01:59 <jeremyrubin> hi
19:02:07 <wumpus> looks like there have been no proposed meeting topics this week in  http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt
19:02:29 <ariard> hi
19:02:54 <dongcarl> hi
19:02:58 <gzhao408> hi
19:02:59 <elichai2> Hi
19:03:43 <wumpus> any last minute topic proposals?
19:04:04 <dongcarl> would like to discuss the various places we put documentation a bit, but perhaps after the meeting
19:04:47 <wumpus> ok yes that's a bit vague, either you want to prpose it as a meeting topic or not?
19:05:27 <wumpus> #topic High priority for review
19:05:28 <dongcarl> sorry, I should be more clear, no, let's table that till after meeting
19:05:38 <achow101> #19324 for hi prio
19:05:42 <gribble> https://github.com/bitcoin/bitcoin/issues/19324 | wallet: Move BerkeleyBatch static functions to BerkeleyDatabase by achow101 · Pull Request #19324 · bitcoin/bitcoin · GitHub
19:05:52 <wumpus> https://github.com/bitcoin/bitcoin/projects/8 11 blockers, 1 bugfix, 3 chasing concept ACK pending
19:05:54 <wumpus> dongcarl: ok!
19:06:10 <wumpus> achow101: yup. I predicted that one :)
19:06:57 <wumpus> added
19:06:59 <achow101> heh
19:07:43 <dongcarl> #17919 please
19:07:45 <gribble> https://github.com/bitcoin/bitcoin/issues/17919 | [DO NOT MERGE] depends: Allow building with system clang by dongcarl · Pull Request #17919 · bitcoin/bitcoin · GitHub
19:08:08 <dongcarl> (the DO NOT MERGE is because of the last commit which is a demo, can be dropped easily)
19:08:20 <wumpus> DO NOT MERGE ;)
19:08:39 <wumpus> dongcarl: good to know!
19:09:04 <wumpus> added
19:09:05 <jeremyrubin> #proposedmeetingtopic I'd like to see the nanobench stuff get finished
19:10:38 <wumpus> anything in high priority for review that is mergable or close to being mergable?
19:11:02 <wumpus> we've merged a few smaller ones this week
19:11:34 <wumpus> but looks like most are still busy in mid-review
19:12:22 <jeremyrubin> I think #18191 is mergeable.
19:12:24 <wumpus> #18637 looks somewhat nearing readyness
19:12:24 <gribble> https://github.com/bitcoin/bitcoin/issues/18191 | Change UpdateForDescendants to use Epochs by JeremyRubin · Pull Request #18191 · bitcoin/bitcoin · GitHub
19:12:27 <gribble> https://github.com/bitcoin/bitcoin/issues/18637 | coins: allow cache resize after init by jamesob · Pull Request #18637 · bitcoin/bitcoin · GitHub
19:13:04 <jamesob> I'm +1 :)
19:14:04 <wumpus> jeremyrubin: ok will take a look after the meeting
19:14:29 <jnewbery> jeremyrubin: I can't see how 18191 is mergeable. I'd expect more than one ACK for changes to the mempool
19:15:12 <wumpus> #topic Nanobench (jeremyrubin)
19:15:15 <wumpus> jnewbery: yes
19:15:53 <wumpus> seems a bit premature
19:16:42 <jeremyrubin> fair
19:17:39 <jeremyrubin> I had thought some of the other reviewers had correctness acked the code, but there is disagreement about benchmarking. I don't think it requires further benching at this time, as it's primarily a memory improvement and has some benches. But can always do more.
19:17:53 <wumpus> I also wonder what sdaftuar_ thinks about it now, initially he didn't think it'd be worth the increase of complexity
19:19:10 <ariard> I'll try to review 18191 before next meeting, has been on my list for a while
19:19:28 <wumpus> ariard: thanks!
19:19:32 <jeremyrubin> Can send you some more details privately
19:19:44 <jeremyrubin> it's a non trivial memory improvement for reorgs
19:20:14 <jeremyrubin> ok onto nanobench
19:20:21 <jeremyrubin> I think it's in pretty good shape
19:20:55 <jeremyrubin> MarcoFalke needs to run with frequency lock enabled?
19:20:55 <dongcarl> jeremyrubin: can you link to prs?
19:20:58 <jeremyrubin> Uhh
19:21:18 <jeremyrubin> #18011
19:21:21 <gribble> https://github.com/bitcoin/bitcoin/issues/18011 | Replace current benchmarking framework with nanobench by martinus · Pull Request #18011 · bitcoin/bitcoin · GitHub
19:22:51 <wumpus> I have no opinion on replacing the benchmark framework, will leave it to people more involved with benchmarking
19:23:49 <jeremyrubin> I think we can just ship it and see what/if anything breaks. AFAIK it has been made output compatible with prior benching output format
19:24:01 <jeremyrubin> the only concrete harm is that the relative numbers won't be comparable anymore
19:24:18 <wumpus> so this doesn't add any dependencies?
19:24:30 <jeremyrubin> It adds a checked in header file
19:24:39 <jeremyrubin> Which martinus, the comitter, is the maintainer of
19:24:45 <wumpus> yeah that's okay
19:25:09 <jeremyrubin> So I'd just be happy to have someone who actively is adding benching features
19:25:27 <jeremyrubin> It has some nice new bells and whistles around helping auto categorize big o
19:25:31 <wumpus> that's a point
19:25:37 <sipa> jeremyrubin: why won't relative numbers be conparable?
19:26:03 <jeremyrubin> It does a better job I think of auto detecting how many runs are required or something
19:26:07 <jeremyrubin> so we do fewer runs
19:26:20 <jeremyrubin> and there is less measurment-in-benchmark overhead I think?
19:26:23 <sipa> ok, but the benchmark numbers should be the same?
19:26:31 <sipa> or at least, closer to reality
19:26:42 <dongcarl> "measure instructions, cycles, branches, instructions per cycle, branch misses" <- is this not done currently (w/o nanobench)?
19:26:48 <jeremyrubin> Well the hot-loop code I think is faster? But the benches themselves won't change
19:26:55 <jeremyrubin> No it is not dongcarl
19:27:07 <jeremyrubin> So we get a bunch of new and cool outputs
19:27:31 <jeremyrubin> But also a compatible output mode for tooling that can't be updated
19:27:34 <sipa> current master just tests min/max/avg time of some fixed number of iterations of a test
19:27:46 <wumpus> adding outputs is great
19:27:56 <dongcarl> that sounds like a worthy feature to me
19:28:13 <wumpus> does this still work on non-x86?
19:29:04 <wumpus> I added measuring instruction cycles at some point but this was also removed again
19:29:17 <jeremyrubin> Not sure wumpus
19:29:39 <dongcarl> I don't see anything in their CI for non-x86
19:29:53 <wumpus> jeremyrubin: I don't mind if it doesn't report all the numbers on non-x86, just that it compiles/works
19:30:13 <sipa> that seems reasonable
19:30:58 <jeremyrubin> I don'
19:31:15 <jeremyrubin> *don't see anything explicitly incompatible except the turbo detection?
19:31:17 <wumpus> the PR currently doesn't pass travis, not that that says everything :)
19:31:23 <jeremyrubin> Maybe I'm missing some syscall
19:31:59 <sipa> well how does it count instructions?
19:32:15 <sipa> because on ARM only privileged code can do that
19:32:29 <sipa> s/instructions/clock ticks/
19:32:32 <jeremyrubin> Failures on Travis are build systme related
19:32:43 <jeremyrubin> not inherent
19:32:53 <jeremyrubin> https://travis-ci.org/github/bitcoin/bitcoin/builds/697933849
19:33:07 <sipa> ok let's try to give it some.renewed review attention?
19:33:32 <sipa> seems useful to me to make progress on (i think the current benchmark system is pretty stupid...)
19:33:38 <wumpus> the ARM failure is really srange, just some timeout on package download
19:33:47 <wumpus> restarting those jobs
19:33:49 <dongcarl> classic Travis
19:34:01 <wumpus> dongcarl: yes :)
19:34:03 <dongcarl> *90's sitcom noises*
19:34:41 <jeremyrubin> martinus claims that it only does performance counters where available
19:34:44 <wumpus> I'm naive enough to think when ARM tests fails to think it's some real problem with non-x86 architectures
19:34:49 <jeremyrubin> So my guess is it's auto detected
19:34:59 <wumpus> anyhow, yes, let's continue review on it
19:35:26 <wumpus> any other topics?
19:37:48 * wumpus wonders why #18011 is in mempool improvements
19:37:50 <gribble> https://github.com/bitcoin/bitcoin/issues/18011 | Replace current benchmarking framework with nanobench by martinus · Pull Request #18011 · bitcoin/bitcoin · GitHub
19:38:14 <wumpus> let's add it to high prio for review as well at least
19:39:25 <dongcarl> Should the "need conceptual review" tag be removed or no?
19:40:13 <wumpus> dongcarl: do you think so?
19:40:40 <dongcarl> I haven't heard oppositions to the concept, just code kinks we might need to work out?
19:41:04 <wumpus> oh, you mean removing it from that particular PR, not in general, sorry, I agree :)
19:41:34 <wumpus> done
19:41:40 <dongcarl> :)
19:42:06 <wumpus> #endmeeting