19:00:08 <wumpus> #startmeeting
19:00:08 <lightningbot> Meeting started Thu Jun  7 19:00:08 2018 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:08 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:56 <achow101> hi
19:01:03 <sipa> hi
19:01:05 <wumpus> PSA: v0.16.1rc2 has been tagged, please start your gitian builders if you haven't yet, hopefully this can be a short rc, the only change is translations
19:01:22 <sipa> reasonable chance that rc2 will be final?
19:01:29 <sipa> (sorry, i haven't followed 0.16.1 much)
19:01:34 <achow101> yes
19:01:46 <wumpus> well at least I haven't seen any other reports to it, except the Russian translation issue
19:01:48 <promag> hi
19:02:26 <wumpus> so yes I hope this can be final very quickly
19:02:58 <jimpo_> hi
19:03:02 <wumpus> topic proposals?
19:03:44 <wumpus> #topic High priority for review
19:03:56 <wumpus> #link https://github.com/bitcoin/bitcoin/projects/8
19:04:54 <wumpus> I don't understand why I added #13059 there last week, it's an issue, not a PR
19:04:55 <gribble> https://github.com/bitcoin/bitcoin/issues/13059 | Dynamic wallet load / create / unload · Issue #13059 · bitcoin/bitcoin · GitHub
19:05:59 <wumpus> at least we merged #13243
19:06:02 <gribble> https://github.com/bitcoin/bitcoin/issues/13243 | Make reusable base class for auxiliary indices by jimpo · Pull Request #13243 · bitcoin/bitcoin · GitHub
19:06:06 <sipa> yay
19:06:10 <promag> =)
19:06:27 <wumpus> so 6 PRs left, I'll remove the issue, surprised no one notified me about that
19:06:49 <promag> I saw that, didn't mind :P
19:08:16 <wumpus> I had an attempt at reviewing and testing #12196, but seems I found a bug
19:08:21 <gribble> https://github.com/bitcoin/bitcoin/issues/12196 | Add scantxoutset RPC method by jonasschnelli · Pull Request #12196 · bitcoin/bitcoin · GitHub
19:08:29 <wumpus> it also needs rebase
19:08:33 <wumpus> @jonasschnelli
19:08:55 <jonasschnelli> hi
19:09:18 <sipa> i was going to follow up with some ideas for writing sets of scripts/addresses
19:09:28 <jonasschnelli> yes. will take care. had some busy days but will work on it next week
19:09:40 <wumpus> ok, thanks for letting me know
19:09:44 <jonasschnelli> please do sipa
19:09:51 <sipa> yeah, it's on my todo list
19:11:09 <wumpus> #13062 I already reviewed a while ago, though it's been needed to rebase since
19:11:11 <gribble> https://github.com/bitcoin/bitcoin/issues/13062 | Make script interpreter independent from storage type CScript by sipa · Pull Request #13062 · bitcoin/bitcoin · GitHub
19:11:24 <sipa> i'll happily keep rebasing it :)
19:11:58 <wumpus> luke-jr: #11082 needs rebase too
19:12:00 <gribble> https://github.com/bitcoin/bitcoin/issues/11082 | Add new bitcoin_rw.conf file that is used for settings modified by this software itself by luke-jr · Pull Request #11082 · bitcoin/bitcoin · GitHub
19:12:26 <luke-jr> is there some reason we went from single-value args + multi-value args to override-args + config-args? the former seems a lot better..
19:12:36 <luke-jr> (this is blocking me on 11082 rebasing)
19:12:49 <wumpus> I'll take that as a topic suggestion
19:12:51 <sipa> i'm not sure what you mean by that
19:13:05 <wumpus> (for later)
19:13:20 <wumpus> #12136 still has lots of active discussion
19:13:25 <gribble> https://github.com/bitcoin/bitcoin/issues/12136 | Implement BIP 174 Partially Signed Bitcoin Transactions by achow101 · Pull Request #12136 · bitcoin/bitcoin · GitHub
19:13:57 <wumpus> achow101: any idea what's needed to move forward there?
19:14:06 <gmaxwell> Is 13191 waiting on my review and testing?
19:14:08 <achow101> wumpus: more review
19:14:17 <wumpus> achow101: just review? ok
19:14:20 <achow101> wumpus: perhaps on the bip itself too
19:14:28 <sipa> i've also been discussion some ideas for splitting part of it up
19:15:04 <wumpus> gmaxwell: #13191 is already merged, but you're very welcome to test and review it anyhow
19:15:07 <gribble> https://github.com/bitcoin/bitcoin/issues/13191 | Specialized double-SHA256 with 64 byte inputs with SSE4.1 and AVX2 by sipa · Pull Request #13191 · bitcoin/bitcoin · GitHub
19:15:41 <wumpus> sipa: that's a good idea, I think, just to have progress
19:16:09 <wumpus> better to make sure as many as possible non-controversial things already merged
19:16:30 <gmaxwell> wumpus: lol. sorry, I had an old page up and on reload the post merge discussion caused me to miss that it was merged. :P
19:16:45 <gmaxwell> So I was confused as to why it wasn't merged yet.
19:16:45 <wumpus> gmaxwell: hehe
19:17:21 <sipa> there are a few more specialized-instructions-optimized-code PRs open
19:17:49 <sipa> we discovered yesterday that compiling the sse4 intrinsics code with -mavx gives a 30% speedup
19:18:02 <wumpus> #13111 should be pretty close, I guess it's just a boring thing to manually test because it unloads
19:18:05 <gribble> https://github.com/bitcoin/bitcoin/issues/13111 | Add unloadwallet RPC by promag · Pull Request #13111 · bitcoin/bitcoin · GitHub
19:18:24 <cfields> sipa: how much work do you think it would be to switch the sse41 to intrinsics?
19:18:26 <promag> reviews are welcome
19:18:36 <wumpus> promag: I will
19:18:46 <promag> i think it's almost there
19:18:54 <sipa> cfields: that would be neat
19:19:01 <sipa> cfields: i can try, but it's low priority for me
19:19:08 <wumpus> sipa: yes, that was really neat
19:19:08 <gmaxwell> I'll put money on it being a boatload slower. :P
19:19:32 <sipa> gmaxwell: define "boatload" please
19:19:37 <gmaxwell> (based on the mavx result suggests to me that the compiler is failing to achieve a good register allocation that doesn't kill performance)
19:19:40 <gmaxwell> 30%.
19:19:42 <sipa> (so i know whether to take the bet or not)
19:19:54 <sipa> nah, no way it's 30% slower with intrinsics :)
19:20:13 <wumpus> I supose it's only boatloads slower on my stupid AMD system :)
19:20:41 <cfields> sipa: ok, I'll look around for an impl on the net to copy from. I doubt I'd fare well myself, though :(
19:21:02 <sipa> cfields: the asm code we have is derived from nasm code that's more readable
19:21:05 <sipa> that may help
19:21:13 <wumpus> but anyhow if anyone needs benchmarking of anything on AMD, just let me know
19:21:21 <cfields> ok
19:21:30 <cfields> wumpus: I'd prefer to merge before getting the AMD numbers :p
19:21:51 <wumpus> cfields: I understand :p
19:22:42 <gmaxwell> wumpus: have you seen if the mavx compiled SSE4 code is slower on your system? I wouldn't expect it to be.
19:22:43 <wumpus> too bad I seem to collect them
19:23:18 <cfields> gmaxwell: iirc he benched those and it was ~the same.
19:23:34 <wumpus> gmaxwell: there was almost no difference with avx here
19:23:44 <wumpus> it was very slightly faster
19:23:48 <gmaxwell> odd.
19:24:03 <wumpus> looks like they go in to some emulation mode
19:24:05 <cfields> right: https://0bin.net/paste/ReThQTAAWhKYfH7x#K99wDsZBBbtqEnc1N44e9UWz2E-t1y2jDhByhD8BBZe
19:24:20 <gmaxwell> (odd because my understanding was that the 30% speedup from getting better register space, not from using any AVX instructions...)
19:24:52 <sipa> gmaxwell: in AVX mode there literally is 2x more register space
19:25:10 <sipa> but maybe that's not the reason for the speedup, i haven't analysed
19:25:28 <sipa> still, i would be very surprised if equivalent code with intrinsics rather than asm is more than a few % slower
19:25:37 <gmaxwell> We can take this discussion offline, but I don't completely agree. :)
19:25:42 <sipa> ok!
19:25:45 <wumpus> intrinsics should do very well, with modern SIMD instruction sets
19:26:08 <wumpus> (they're pretty much designed to work well with them)
19:26:21 <wumpus> but yes, it's getting a bit off topic
19:26:42 <wumpus> #topic single-value args + multi-value args to override-args + config-args? (luke-jr)
19:26:55 <sipa> luke-jr: elaborate
19:27:08 <gmaxwell> I have cached skeptcisism mostly because compilers used to be VERY bad with register allocations for SIMD, resulting in a lot of code that is more complex than 'run this code 4x in parallel' not getting a speedup when hand asm hapily got a 3x speedup. I know they're much better now. So perhaps ignore me. :)
19:27:21 <luke-jr> we used to have a map of arg name -> single value and arg name -> multiple values
19:28:04 <luke-jr> it's been changed to arg name-> multiple values, but with two maps, one for config file, and one for command line options
19:28:36 <luke-jr> it seems better to have config/commandline share maps
19:28:37 <wumpus> gmaxwell: compilers seem to have become better with that, but I understand your skepticism
19:29:16 <wumpus> gmaxwell: there's also the *optimizing for a specific CPU type* versus *optimizing it to be, on average, fast, for many families of CPUs* thing
19:29:33 <luke-jr> this comes up because the change complicates how to implement rwconf
19:30:40 <achow101> luke-jr: I think that change may be for qt settings interactions
19:30:50 <wumpus> luke-jr: I'm not sure I know enough about this to understand the implications of this in practice
19:30:59 <jnewbery> luke-jr: it was changed in #11862
19:31:02 <gribble> https://github.com/bitcoin/bitcoin/issues/11862 | Network specific conf sections by ajtowns · Pull Request #11862 · bitcoin/bitcoin · GitHub
19:31:38 <wumpus> whatever you do, please don't mess up the bitcoin-qt initialization order, the rest is fine with me :p
19:31:56 <sipa> i think it would also be useful to make the arg information know whether a particular argument is single-value or multi-value
19:32:16 <sipa> rather than have that information be at query time
19:32:39 <sipa> but i also don't understand the interactions well enough
19:32:42 <sipa> aj: you here?
19:32:43 <luke-jr> sipa: yes, I was thinking about that too
19:32:48 <wumpus> #13389 kind of scared me
19:32:50 <gribble> https://github.com/bitcoin/bitcoin/issues/13389 | Utils and libraries: Fix #13371 - move umask operation earlier in AppInit() by n2yen · Pull Request #13389 · bitcoin/bitcoin · GitHub
19:33:25 <luke-jr> the reason they're split one way or the other, is because the last command line option overrides the former ones, and the earlier config file overrides the later ones
19:33:45 <wumpus> we don't have good tests for half of this stuff
19:33:57 <luke-jr> so now when we do Get*Arg, the code is going for the end of the command line list, then the start of the config file list
19:34:22 <luke-jr> (the altnerative being, to just parse into a single value at startup, and just access that map at runtime)
19:35:48 <sipa> i feel like the right people aren't here to discuss that
19:35:57 <wumpus> yes
19:35:59 <luke-jr> maybe
19:36:06 <sipa> it sounds reasonable what you're saying, but i don't know enough to really comment
19:36:32 <wumpus> I agree
19:37:11 <jnewbery> There's a lot of discussion and review in both #12878 and #11862. AJ also added really good code coverage in those PRs, so it should be pretty straightforward to follow
19:37:14 <wumpus> other topics?
19:37:14 <gribble> https://github.com/bitcoin/bitcoin/issues/12878 | [refactor] Config handling refactoring in preparation for network-specific sections by ajtowns · Pull Request #12878 · bitcoin/bitcoin · GitHub
19:37:17 <gribble> https://github.com/bitcoin/bitcoin/issues/11862 | Network specific conf sections by ajtowns · Pull Request #11862 · bitcoin/bitcoin · GitHub
19:37:20 <promag> it also sounds reasonable to encapsulate the arg chaining in a map lookup lookalike
19:38:49 <wumpus> meh
19:39:07 <luke-jr> sounds like either way I should start with tests
19:39:15 <wumpus> personally I prefer method calls to just look like method calls, not operator overrides, in the common case
19:40:05 <wumpus> luke-jr: yes! tests are always good
19:41:54 <wumpus> other topics?
19:42:34 <promag> actually I have a question
19:43:10 <promag> #13374 and #13375
19:43:12 <gribble> https://github.com/bitcoin/bitcoin/issues/13374 | utils and libraries: checking for bitcoin address in translations by kaplanmaxe · Pull Request #13374 · bitcoin/bitcoin · GitHub
19:43:14 <gribble> https://github.com/bitcoin/bitcoin/issues/13375 | utils and libraries: address check in update-translations.py by undercoverGod · Pull Request #13375 · bitcoin/bitcoin · GitHub
19:43:17 <wumpus> if you have a topic, just propose it, there's no need to tell that you have a question
19:43:46 <promag> they are the same, why isn't one closed?
19:44:18 <wumpus> because no one told me (or fanquake, or anyone) to?
19:44:38 <wumpus> I do not have the capacity to pay attention to all PRs in parallel
19:44:54 <promag> really? :P
19:44:54 <sipa> we need to have an AVX2 wumpus
19:45:00 <wumpus> sipa: +1
19:45:04 <sipa> +8
19:45:21 <sipa> end of meeting, i assume?
19:45:26 <wumpus> yes
19:45:28 <wumpus> #endmeeting