< bitcoin-git>
[bitcoin] practicalswift opened pull request #10839: Don't use pass by reference to const for cheaply-copied types (bool, char, etc.). (master...dont-pass-by-reference-for-cheaply-copied-types) https://github.com/bitcoin/bitcoin/pull/10839
< fanquake>
sipa same issue as before. Assertion failed: (consensus.hashGenesisBlock etc
< AsadSalman>
I am able to get a value out (as you can see in the screenshot)
< AsadSalman>
but that is a sum of both CTxOut
< AsadSalman>
I only want the amount I spent (which was 10BTC, i.e. 1,000,000,000 satoshi)
< AsadSalman>
I don't know what the other CTxOut that is getting summed in is, and how do I get rid of it in the final sum?
< sipa>
it's change
< AsadSalman>
change? I'm not sure what you mean, could you elaborate?
< sipa>
not here, try bitcoin.stackexchange.com
< sipa>
or #bitcoin
< AsadSalman>
okay! (sorry, I thought this would be an appropriate place to ask about core)
< sipa>
your question is about how bitcoin transactions work
< gmaxwell>
BlueMatt: on 10831 please review the rebase carefully, I'm sick and wouldn't even have done the rebase today but for merge window urgency.
< cfields>
sipa: still crashes :(
< cfields>
sipa: i managed to get lldb to step into the asm, but it doesn't show me where I am when i single-step instructions, only that I'm in the asm block. So i have to step 1 instruction at a time and dump registers
< cfields>
the explicit assignments will help though, since now I'll know where I'm looking
< sipa>
cfields: so you do reach the asm block?
< sipa>
can you count how many instructions inside you execute?
< cfields>
sipa: yea, and past the zero-size check
< cfields>
sipa: yea, i can do that. the recent changes should make it much easier, i'll poke some more
< sipa>
cfields: thanks for all the time you're spending on thid
< cfields>
sipa: np. it's a big boost.
< sipa>
cfields: for IBD it seems a 200-300 second reduction to block 450k
< sipa>
at the tip the impact is probably larger, but harder to benchmark
< cfields>
wow
< cfields>
net-side will benefit at the tip too, though probably not enough to be relevant
< cfields>
sipa: gcc docs says the value of inputs isn't allowed to change. i suppose values of pointers passed in are ok though?
< sipa>
cfields: the only inputs are constants
< sipa>
everything else is marked as an output
< cfields>
oh right, nm
< gmaxwell>
sha-ni is a bigger boost.
< sipa>
yes, far larger
< sipa>
close to 1000s reduction
< sipa>
more like 800s
< sipa>
(compared to master)
< gmaxwell>
cfields: I expect even just sse4 to make a very measurable improvement in compact block relay... that is mostly bottlenecked on sha2...
< cfields>
whoa
< cfields>
have you benchmarked any benefit to adding a dedicated sha2d?
< sipa>
cfields: can you show another disasm?
< gmaxwell>
haven't though if we want further benefit I think what we really want is a hash tree that hashes multiple things at once, parallel sse4 sha2 is significantly faster than one at a time... and it even looks like for sha-ni for max throughput you want to do at least two at a time.
< cfields>
gmaxwell: yea, i was reading that paper (one of many?) yesterday
< cfields>
hah! well that was some easy googling for once :)
< cfields>
sipa: do you see that documented anywhere?
< sipa>
nope
< bitcoin-git>
[bitcoin] jnewbery opened pull request #10841: [rpc] Give users one final warning before depracating getinfo (master...deprecate_getinfo) https://github.com/bitcoin/bitcoin/pull/10841
< cfields>
sipa: maybe safer to use digits instead, then? Reading the docs like a pedant, I assume some future linker could break win32 the same way.
< cfields>
nope, that's crashy too
< sipa>
cfields: so, latest PR version works?
< cfields>
sipa: i don't think so, i think that matches what i tried
< sipa>
?
< cfields>
sipa: i misread the docs, look at their example
< cfields>
"So for example, the first 1: may be named .L1C-B1, and the 44th 3: may be named .L3C-B44."
< sipa>
A local symbol is any symbol beginning with certain local label prefixes. By default, the local label prefix is `.L' for ELF systems or `L' for traditional a.out systems, but each target may have its own set of local label prefixes.
< cfields>
apparently the numbers aren't a replacement for the L prefix
< sipa>
you're looking at automatically generated names
< sipa>
we don't need that; the asm syntax's %= guaranrees uniqueness already
< cfields>
sipa: sorry, misread the diff
< sipa>
it won't work, i forgot to rename the jumps
< cfields>
yep, with those renamed, all good
< phantomcircuit>
gmaxwell, btw when i did the walletdb stuff i was doing CWalletDB *pwalletdb=NULL and then initializing pwalletdb only if it was null instead of creating a new function
< bitcoin-git>
[bitcoin] practicalswift opened pull request #10842: Fix incorrect Doxygen tag (@ince → @since) (master...doxygen-since) https://github.com/bitcoin/bitcoin/pull/10842
< bitcoin-git>
[bitcoin] practicalswift opened pull request #10843: Add attribute [[noreturn]] (C++11) to functions that will not return (master...noreturn) https://github.com/bitcoin/bitcoin/pull/10843
< terjegun>
Hello! I was wondering, is there any reward for contributing translations to the project?
< Lauda>
No.
< instagibbs>
terjegun, satisfaction in knowing you helped others?
< BlueMatt>
that seems to indicate it doesnt support "seg -e"???
< BlueMatt>
that sounds like an ubuntu-on-windows issue
< sipa>
I vaguely remember that there were known issues when building on Ubuntu-in-Windows, but I don't remember whether it was this.
< BlueMatt>
not this, no
< BlueMatt>
well, afair
< cfields>
IIRC the last round of problems was caused by an update that borked the default path
< sipa>
cfields: what is the option to make gcc emit every function in its own section?
< cfields>
sipa: ffunction-section
< cfields>
er, there's another s in there somewhere though
< cfields>
-ffunction-sections
< sipa>
do we enable -Wl,--gc-sections by default?
< cfields>
sipa: and you want to link with -gc-sections
< cfields>
no
< sipa>
any reason not to?
< cfields>
sipa: i think default-hidden-visibility accomplishes the same thing without the section bloat?
< sipa>
cfields: is that always set for us?
< sipa>
cfields: a build with CFLAGS/CXXFLAGS/LDFLAGS all set to "-O2 -g0", results in a 6MB stripped bitcoind
< sipa>
cfields: a build with CFLAGS/CXXFLAGS/LDFLAGS all set to "-O2 -g0 -ffunction-sections -fdata-sections -Wl,--gc-sections", results in a 5.4MB stripped bitcoind
< cfields>
sipa: it's not set auto, --enable-reduced-exports
< cfields>
mind comparing with that?
< gmaxwell>
why don't we do that by default
< cfields>
gmaxwell: there's a visibility issue with some boost versions
< gmaxwell>
ah
< sipa>
cfields: that option makes no difference; 6.0MB again
< cfields>
huh
< sipa>
make clean && ./configure --with-incompatible-bdb --without-gui --enable-reduced-exports && make -j9 src/bitcoind && strip src/bitcoind && wc -c src/bitcoind
< sipa>
6008256 src/bitcoind
< cfields>
sipa: sounds promising then, though I'm kinda confused as to why
< bitcoin-git>
bitcoin/master 5618b7d Pavel Janík: Do not shadow upper local variable `state`.
< bitcoin-git>
bitcoin/master 1fc783f MarcoFalke: Merge #10739: test: Move variable `state` down where it is used...
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #10739: test: Move variable `state` down where it is used (master...20170704_Wshadow_txvalidationcache_tests) https://github.com/bitcoin/bitcoin/pull/10739
< cfields>
sipa: if we're using ffunction-sections, we can use -Wl,--icf=safe too
< cfields>
shaves off another ~70k for me
< sipa>
cfields: i don't see that in man ld
< cfields>
sipa: man ld.gold
< sipa>
ah
< bincap>
given two gitian.signs asserts (from two people) is there a quick command to compare if their out_manifest is identical?
< bitcoin-git>
[bitcoin] practicalswift opened pull request #10847: Enable devirtualization opportunities by using the final specifier (C++11) (master...devirtualization) https://github.com/bitcoin/bitcoin/pull/10847
< bitcoin-git>
bitcoin/master 2c2e90d practicalswift: Fix incorrect Doxygen tag (@ince → @since). Make Doxygen parameter names match actual parameter names.
< bitcoin-git>
bitcoin/master 3895e25 MarcoFalke: Merge #10842: Fix incorrect Doxygen tag (@ince → @since). Doxygen parameter name matching....
< sipa>
(it seems to forget progress when continuing a reindex)
< BlueMatt>
oh, yes, thats next on my list
< sipa>
actually, i may be an idiot there
< BlueMatt>
hmm, did that not used to happen? I have no idea
< sipa>
it is supposed to start over from scratch, but go faster through the parts it has already done
< sipa>
i guess the thing to make sure is that it does not wipe the block index when contonuing a reindex
< BlueMatt>
it shouldnt, though
< BlueMatt>
hmm
< BlueMatt>
(thats the section where you commented on the comment you found confusing
< BlueMatt>
(it says explicitly that we dont want to clear the db even if we're setting fReindex unless its a new reindex...)
< sipa>
well i failed to understand what it was trying to say :)
< BlueMatt>
sipa: try now?
< BlueMatt>
(comment, that is)
< BlueMatt>
didnt change the code
< sipa>
ok, will have a look soon
< BlueMatt>
so, wait, do you think there is a bug that i should go hunt or do you think you mis-read?
< sipa>
i thought there was a bug, but i'm less sure now
< sipa>
i'll verify
< BlueMatt>
hmmm...this client-side getinfo thing scares me
< BlueMatt>
our rpcs are always atomic elements, but if you start calling getinfo, then getwalletinfo, then getmininginfo separately, they could return results based on different views of the current chain
< BlueMatt>
which would be astoundingly confusing for users
< sipa>
i'm totally fine with deprecating and then deleting
< BlueMatt>
well I'm curious what the previously-mentioned "IRC discussion" was that resulted in the pr getting reopened
< BlueMatt>
i dont want to show up late to the party and shit on it and say it shouldnt happen, but I'm really uneasy about the idea
< BlueMatt>
sipa: do you want the asm sha in for 15?
< BlueMatt>
should i bother reviewing it?
< BlueMatt>
(yet)
< sipa>
BlueMatt: i think it's a great and easy performance win, and i think it's ready
< sipa>
whether we want it in 0.15 depends on reviewer interest
< BlueMatt>
k
< BlueMatt>
i'll try to get to it
< sipa>
awesome
< cfields>
BlueMatt: yea, will do in a bit. I'm frantically trying to bump qt atm.
< cfields>
+1 for sha asm in 0.15
< cfields>
sipa: any theories as to what slows down the lto build?
< sipa>
cfields: over-eager inlining across objects?
< cfields>
makes sense
< gmaxwell>
sipa: wumpus had a comment about where the detection was being triggered for the asm.
< cfields>
i could see how it might make a mess of our serialization
< sipa>
gmaxwell: i've long since addressed that and commented about it
< gmaxwell>
I only haven't acked it because I didn't think the patch was in its final state yet... I've tried it on a few hosts, works as expected.
< gmaxwell>
sipa: oh
< gmaxwell>
oh I see there are more commits now
< sipa>
gmaxwell: where 'long' means 15 hours ago
< sipa>
it also prints which implementation is being used
< sipa>
the only unaddressed comment is BlueMatt's concern about printing copyright information for binary-only distributions
< BlueMatt>
we do it for openssl for qt - we have it in the about dialog or smth iirc
< gmaxwell>
I don't believe matt is correct in any case; we don't have a "binary only distribution" -- sure you don't have to download the source, but thats up to you.
< sipa>
easy enough to add something there, if needed
< gmaxwell>
BlueMatt: openssl is 4-clause BSD
< cfields>
\o/ a self-test
< sipa>
cfields: actually, you should test whether reintroducing the foldgers crystals bug in the sse asm causes it to select the basic sha256 implementation at run time
< sipa>
it should
< gmaxwell>
sipa: I was thinking of suggesting logging that failure, when cpu id and selftest disagree...
< cfields>
sipa: hmm, i wonder if we should still fail in that case though, and require a --software-sha256 flag or something
< sipa>
gmaxwell: the crypto code doesn't have access to logging
< cfields>
if that's happening in the wild, we really want to know about it
< sipa>
we could assert in case the selftest doesn't go as expected
< sipa>
i agree that this may hide bugs
< sipa>
though in a pretty safe way... i don't think it's easy to screw up SHA256 in a way that wouldn't be detectable with a single test
< gmaxwell>
sipa: one thing the selftest doesn't extensively test is sensitivity to alignent.
< sipa>
ok, i can fix that
< sipa>
i can also extend it to test 0 and/or 2 blocks rather than just a single block
< BlueMatt>
gmaxwell: hmm? bitcoin.org/bin has binary distributions
< BlueMatt>
we ship binary-only stuff, at least on windows
< BlueMatt>
iirc
< sipa>
BlueMatt: i think gmaxwell's point is that the source code is still available for those
< sipa>
i have no idea whether that matters, IANAL
< BlueMatt>
ah, ok
< BlueMatt>
wellt hen I dont care
< BlueMatt>
as long as his point isnt "we dont ship binary-only" and is instead "but source is freely available", then he would know :)
< gmaxwell>
BlueMatt: if it meant what you thought, it wouldn't be GPL compatible. You're interperting the license in the same way that 4-clause bsd is written.
< BlueMatt>
k :)
< gmaxwell>
BlueMatt: my point was we always make the source available.
< BlueMatt>
gmaxwell: we dont always *ship* the source along with the binary
< BlueMatt>
was my point
< BlueMatt>
as long as I'm misreading the liscense, I withdraw my concern :)
< BlueMatt>
we just make it available
< gmaxwell>
sipa: not something to fix now, but I see that the multiblock code doesn't really make a heroic effort to transform multiple blocks at a time. This may disavantage the avx2-rorx code, which really wants 2 blocks at a time (as it vector processes the initial expander in both blocks)
< sipa>
gmaxwell: that's not too hard to fix
< gmaxwell>
ya, I know. e.g. change the buffering to buffer two blocks.
< sipa>
yeah
< gmaxwell>
similar, the 8m rorx code is setup for 8 blocks.