< bitcoin-git>
[bitcoin] ariard closed pull request #18797: Export standard Script flags in bitcoinconsensus (master...2020-04-export-standard-flags) https://github.com/bitcoin/bitcoin/pull/18797
< fjahr>
instagibbs: I noticed some lines in the original tests that seemed like dublicates/unnecessary to me and did not add to the test coverage and I removed those (mentioned in the commit message). I think that had nothing to do with the changes in the test framework because I did those pretty late and I moved those lines pretty early when I started working on tests for wtxid iirc.
< bitcoin-git>
[bitcoin] theStack opened pull request #19644: rpc: document returned error fields as optional if applicable (master...20200802-rpc-document_errors_fields_as_optional) https://github.com/bitcoin/bitcoin/pull/19644
< instagibbs>
fjahr, ahok
< instagibbs>
fjahr, it's not duplicate though, they were checking explicitly for a case we care about. Will continue in DM
< elichai2>
The new benchmark suite is cool, but I can't figure out which metric I should look at 😅
< elichai2>
should I look at ins/op? at cyc/op? at ns/op? something else? argh
< elichai2>
the only measure that actually stays consistent between each run is the instruction per ops, but I'm not sure when is it good enough (ie should I only look at that as long as I know there are no expensive SIMD/CRYPTO -like instructions? or is there also a big difference in the timing of ie MOV vs ADD)
< sipa>
elichai2: if your frequency scaling is off, they should all be proportional
< sipa>
and ultimately what you're interested in as absolutely numbers in ns/op; if you had to downscale your cpu speed to get consistent results, cyc/op may be better so you don't need to report "X ns/op @ frequency"
< sipa>
ins/op i don't know - perhaps that's just the best approximation to be had when your cpu is dynamically scaling frequency
< elichai2>
I'm locking the frequency using `sudo pyperf system tune` but I still get small changes in the cycles+ns
< sipa>
to actually lock the frequency on my cpu i need to boot with intel_pstate=disable on the kernel commandline, i think
< elichai2>
oh wow, ok
< sipa>
but some variation of cycles/ns is inevitable, due to random environment changes, like memory layout, other processing interrupting, ...
< elichai2>
sipa: I just realized there's somewhat of a bug in the siphash, you can't mix between calling `CSipHasher& Write(uint64_t data);` and `CSipHasher& Write(const unsigned char* data, size_t size);` because the former will ignore anything that the latter hasn't hashed yet
< sipa>
elichai2: that's documented
< elichai2>
"This function can only be used when a multiple of 8 bytes have been written so far" :D
< sipa>
/** Hash a 64-bit integer worth of data
< sipa>
* It is treated as if this was the little-endian interpretation of 8 bytes.
< elichai2>
You're right
< sipa>
* This function can only be used when a multiple of 8 bytes have been written so far.
< sipa>
*/
< elichai2>
I need to pay more attention to comments :P
< phantomcircuit>
sipa, im assuming the single parameter just blindly modifies the state assuming it's already on an 8 byte boundary right?
< phantomcircuit>
it's got an assert that would catch using it improperly actually
< sipa>
indeed
< phantomcircuit>
sipa, this is probably pedantic af, but what's the advantage of using an assert there instead of throwing an exception?
< phantomcircuit>
are asserts still always enabled?
< sipa>
yes
< elichai2>
bitcoin core is always compiled with asserts enabled
< phantomcircuit>
that's an instance where it's not concensus critical that it fail and the only way to catch a violation of the api is at runtime, so maybe it should be an exception not an assert?
< phantomcircuit>
anyways, pedantry
< sipa>
phantomcircuit: that's always a hard question; clearly some assumptions underlying the code are wrong, so it becomes very hard to reason about correctness
< phantomcircuit>
sipa, this is of course not in-scope for bitcoin dev but i've tried using some of the "library"ish things outside of core before and mostly run into random things like needing uint256 which then needs the serialization stuff
< phantomcircuit>
things like throwing an exception instead of calling assert would be useful for that, but like
< phantomcircuit>
yeah
< bitcoin-git>
[bitcoin] ariard opened pull request #19645: Allow wtxid-acceptance to the mempool (master...2020-08-wtxid-replacement) https://github.com/bitcoin/bitcoin/pull/19645