< cfields>
i think it's pretty universal, the struct padding is the only thing i can see practically changing...
< cfields>
but I guess you're right, it doesn't save much
< fanquake>
pierre_rochard nice rupdates to bitcoinacks so far.
< pierre_rochard>
Thanks fanquake! ICYMI: https://bitcoinacks.com/ I have a few ideas for the next big iteration ( https://github.com/PierreRochard/bitcoin-acks/blob/acks/README.md ), happy to add more if you have anything that is dragging you down as a contributor/reviewer/maintainer. Something I was thinking about during today’s meeting was surfacing High Priority for review PRs in the UI.
< fanquake>
pierre_rochard Cool, I'll take a look. Even something as simple as a different background colour on the High Priority PRs might work.
< bitcoin-git>
[bitcoin] fanquake opened pull request #13165: doc: Mention good first issue list in CONTRIBUTING.md (master...good-first-issue) https://github.com/bitcoin/bitcoin/pull/13165
< luke-jr>
pierre_rochard: not forcing the horizontal scrollbar off would be nice
< luke-jr>
and some way to hide closed PRs
< luke-jr>
not sure why adding a filter for Closed insists on the value being a date O.o
<@wumpus>
jonasschnelli: is that with or without my patch at the bitcoind side?
< jonasschnelli>
without... will compare performance soon
< jonasschnelli>
I'm working on a test setup where block 450'000 to 500'000 will be requested probably 100-1000 times... then print the median result
< jonasschnelli>
the client just loads the block but does not deserialize it
<@wumpus>
right, you have block request logic in bitcoincore-indexd, so might be an idea to start from that and make a benchmarker that just pulls blocks and drops them
<@wumpus>
I thought about making something based on the P2P functional test framework, but python is likely going to be a bottleneck then
< jonasschnelli>
yes. the current poc uses libbtc which is a straight forward p2p client using libevent2
<@wumpus>
jonasschnelli: going to try to build it
< jonasschnelli>
wumpus: great. with python?
<@wumpus>
jonasschnelli: I mean your indexd project
<@wumpus>
and try to do some benchmarking with it
< jonasschnelli>
wumpus: it's very immature and sketchy
<@wumpus>
yes but I only need a very basic node framework for this, the less overhead the better
< jonasschnelli>
also... you probably need a manual -lpthread for linking... :)
< jonasschnelli>
yes... give it a try
<@wumpus>
I first have to make it find my customized libevent
<@wumpus>
hopefully PKG_CONFIG_PATH does just work :)
<@wumpus>
jonasschnelli: a shame, looks like the libbtc libevent2 check doesn't use pkgconfig to check where the library is
<@wumpus>
I'll see if I can add that or maybe I'll just whack in CPPFLAGS=... for now
< jonasschnelli>
wumpus: Good point. happy if you open a PR at github.com/libbtc/libbtc ;)
<@wumpus>
jonasschnelli: also need to pass in -fPIC I suposse /usr/bin/ld: bitcoincore_indexd-btcnode.o: relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
<@wumpus>
jonasschnelli: where does it get the node configuration (where to connect to) from?
< jonasschnelli>
wumpus: btcnode.cpp
< jonasschnelli>
search after 127.0.0,1
<@wumpus>
ok I see,t hanks
<@wumpus>
so simply 127.0.0.1:8333, but it wasn't connecting because my node was not listening
< jonasschnelli>
wumpus: What do you do in handshake_done()?
< jonasschnelli>
This is where you should ask for blocks
< jonasschnelli>
the current bitcoincore-indexd does sync headers first
<@wumpus>
jonasschnelli: haven't changed the function
< jonasschnelli>
wumpus: it looks like it doesn request headers then
< jonasschnelli>
maybe set a breakpoint in void handshake_done(struct btc_node_ *node)
< jonasschnelli>
wumpus: wait..
< jonasschnelli>
your client-peer is sending a getheaders()
< jonasschnelli>
-()
< jonasschnelli>
but seems like your remote node does not answer
<@wumpus>
will do - the node syncing from is not up to date with the chain, could that be a problem?
< jonasschnelli>
Are they on the same network? Mainnet/etc.?
<@wumpus>
yes both on mainnet
<@wumpus>
I'll look into it
< jonasschnelli>
wumpus: eventually... the client node will ask for headers from genesis... could be that your remote node is refusing that during IBD
<@wumpus>
jonasschnelli: indeed - seems like the old "Ignoring getheaders from peer=%d because node is in initial block download" plague again, adding whitelist=127.0.0.1 solved it
<@wumpus>
it's rolling
< jonasschnelli>
wumpus: why did you use a custom built libevent? Did you modify the sources for performance measuring?
<@wumpus>
I want to have a newer one that is installed on this version of ubuntu by default, I've also made some http changes to make it work with UNIX domain sockets, though that part isn't necessary here it's the only version of libevent I have installed
<@wumpus>
(and I always install manuall-compiled things into /opt/<libname>, not in /usr)
< jonasschnelli>
I see
<@wumpus>
jonasschnelli: looks like it doesn't hit the fast path, I suppose your indexer doesn't request MSG_WITNESS_BLOCK?
< jonasschnelli>
wumpus: the indexer does send a inv with blockhashes... shouldn't this result in a MSG_WITNESS_BLOCK?
<@wumpus>
jonasschnelli: ser_u32(inv_msg_cstr, 2); <- that's MSG_BLOCK, not MSG_WITNESS_BLOCK
<@wumpus>
needs 1 << 30 ORed in
< jonasschnelli>
Right. Yes. I guess just changing to MSG_BLOCK | MSG_WITNESS_FLAG should do the job.
<@wumpus>
yes
<@wumpus>
eh no, those constants are not defined anywhere
<@wumpus>
would be a good idea though
< jonasschnelli>
yes
<@wumpus>
ok, hitting the fast path now on the server side, I commented out the transaction processing code on the client side - and make it quit when current block height is 473600 (arbitrary)... just a matter of waiting now
< jonasschnelli>
wumpus: I would probably start from 450000 up to 500000 for performance tests.
<@wumpus>
yes maybe - I don't have any more recennt blocks on this node, so this will have to do for now
<@wumpus>
including the small blocks in the beginning is not very useful, probably, but should not take a lot of time
< jonasschnelli>
Indeed.
< jonasschnelli>
wumpus: for performance benchmarks, what do you think in requesting 450k-500k, measure time, loop this for 1000 and calculate avg/med?
< jonasschnelli>
450-500k should take seconds
<@wumpus>
requesting the same blocks over again will make them end up in the disk cache, though it's easy to average out especially when you disregard the first run
< bitcoin-git>
bitcoin/master a508091 fanquake: doc: Mention good first issue list in CONTRIBUTING.md
< bitcoin-git>
bitcoin/master 627c376 MarcoFalke: Merge #13165: doc: Mention good first issue list in CONTRIBUTING.md...
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #13165: doc: Mention good first issue list in CONTRIBUTING.md (master...good-first-issue) https://github.com/bitcoin/bitcoin/pull/13165
< sdaftuar>
BlueMatt: i think there might be another edge case bug in ABC that we didn't talk about yet
< BlueMatt>
oh? ugh
< sdaftuar>
it seems like it might be possible for pruning to run in between cs_main lock acquisitions?
< sdaftuar>
and it's FindMostWorkChain's job to deal with that
< sdaftuar>
probably it couldn't really happen in any "normal" scenario
< sdaftuar>
anyway i think this may not be so different than other modes where pruning might fail, so i don't know that we should worry about it, except maybe add some comments to remind ourselves
< ramzo>
hello
< ken2812221>
Is there a way to update-alternatives in gitian builder? I'm trying to bump my gitian build to 18.04, but stuck at Windows update-alternatives to posix version.
<@wumpus>
ken2812221: yes, you can sudo afaik
< ken2812221>
no, sudo or su does not work.
<@wumpus>
I think it's possible to enable that in the descriptor
< ken2812221>
Ok, I would find it, thanks.
<@wumpus>
gbuild has to be run with --alow-sudo in that case, too
< ken2812221>
Ok. I'm trying.
< ken2812221>
Thanks, it does work!!
<@wumpus>
good!
< ken2812221>
Gitian-build fails due to no 'rename' program. it's a pain after it has been compiled for a long time.
< MarcoFalke>
How hard would it be to transcribe the gitian-descriptors into a docker file?
< ken2812221>
I'm not using docker, I'm using lxc.
< ken2812221>
I've created a PR to gitian-builder, so we can use lxc 3.0.0 to do gitian-build
<@wumpus>
ken2812221: great!
< TheCharlatan>
Are there existing benchmarking results for the bench tests available between armv7 and armv8 ?
< skeees>
sdaftuar: i'm unable to reproduce that genesis block failure - what are you doing to achieve?
< sdaftuar>
while src/test/test_bitcoin -t validation_block_tests; do :; done
< sdaftuar>
that was with a patch to try to get rid of that other error (i got rid of the BOOST_CHECK() call at line 160)
< skeees>
hah - yeah i've been doing that a bunch too - no dice. maybe platform specific
< sdaftuar>
oh i see what you're saying about the genesis block -- yes that does seem like a test bug
< sdaftuar>
and i guess the callbacks are all happening in a single thread anyway, so the BOOST_CHECK calls there should be safe
< skeees>
even though unable to reproduce - i've updated pr with something that i *think* will fix the error you're seeing related to that
< sdaftuar>
skeees: i don't know how the threading works in the boost testing framework, but i did notice that you don't acquire cs_main before accessing chainActive.Tip() when you construct the TestSubscriber()
< sdaftuar>
is it possible that there's a race where you're able to pull the value of the tip before the callback fires (very rarely?)
< skeees>
hmmm you still seeing this issue even after 04176fc?
< skeees>
didn't acquire because I assumed I'd be the only thread running in the test framework at that point
< bitcoin-git>
bitcoin/master c3f34d0 practicalswift: Make it clear which functions that are intended to be translation unit local...
< bitcoin-git>
bitcoin/master f82e1c9 MarcoFalke: Merge #13163: Make it clear which functions that are intended to be translation unit local...
< sdaftuar>
i was still trying to reproduce, haven't taken your fix yet
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #13163: Make it clear which functions that are intended to be translation unit local (master...internal-linkage) https://github.com/bitcoin/bitcoin/pull/13163
< sdaftuar>
hmm that hypothesis doesn't seem to be panning out (i tried adding a sleep before the BlockConnected callbacks after we connect the genesis block, to see if that would trigger it -- no dice)
< sdaftuar>
skeees: oh, could it just be an issue of the scheduler getting to run the genesis block BlockConnected callbacks after the TestSubscriber is created? i don't know what the callback semantics are--
< sdaftuar>
do we callback only the callbacks that were subscribed at the time the event was generated, or do we callback everyone who is subscribed at the time the callback is actually fired off by the scheduler?
< skeees>
i think it could be that
< sdaftuar>
i can't figure out where to stick a sleep to verify that this is the issue...
< skeees>
its not well defined right now because we push a callback to the scheduler that invokes the signal
< skeees>
so you could subscribe after you push that callback but before signal is invoked i think
< sdaftuar>
that's pretty weird behavior imo
< sdaftuar>
but sort of exactly why we need these tests!
< bitcoin-git>
[bitcoin] jamesob opened pull request #13168: Thread names in logs and deadlock debug tools (take 2) (master...2018-05-threadnames-take-2) https://github.com/bitcoin/bitcoin/pull/13168
< skeees>
yeah - its strange - that behavior in particular starts to become a problem if you ever get into a state where you have a lot of dynamic subscription / unsubscription (e.g. may actually pose some challenges getting rid of mempool signals)
< skeees>
but in current use case where you just subscribe on startup and those remain fixed - its not a huge deal
< sdaftuar>
right
< sdaftuar>
aha! i think that's it
< sdaftuar>
i managed to reproduce it with a sleep added to the scheduler
< sdaftuar>
so we should decide if we want to stick with that behavior or not... seems kind of shitty to me. perhaps registering with the validationinterface should require the queue to be drained first?
< sdaftuar>
not sure if that idea works
< sdaftuar>
BlueMatt: ^^^
< skeees>
i think that's a separate pr - i'll put something together
< skeees>
for this one - i think it'll suffice to change some of the subscription ordering