💬 tdb3 commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2033997944)
> > is the desire here to have the unit tests run by default when running individual tests
>
> No
>
> > (and also by default when running all tests)?
>
> Yes
>
> They are just another test now, so they don't need special treatment via `--skipunit`. Conversely, they don't need to be run more often than other tests.
>
> > The user can specify --failfast to end testing early if any test fails (unit tests are no exception)
>
> Yes, that seems good enough.
>
Ah, thank you for clarifying.
@mzu
...
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2033997944)
> > is the desire here to have the unit tests run by default when running individual tests
>
> No
>
> > (and also by default when running all tests)?
>
> Yes
>
> They are just another test now, so they don't need special treatment via `--skipunit`. Conversely, they don't need to be run more often than other tests.
>
> > The user can specify --failfast to end testing early if any test fails (unit tests are no exception)
>
> Yes, that seems good enough.
>
Ah, thank you for clarifying.
@mzu
...
💬 hebasto commented on pull request "ci: Temporarily disable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2034000240)
> Is it an option to drop the USDT tests from the `Cirrus CI / ASan + LSan + UBSan + integer, no depends, USDT` job and move them to a GH actions 23.04 VM if "lifting the requirements from the self-hosted runners" is the general direction we want to go? (probably makes sense to merge this sooner than later before adding a GH actions VM job to have CI green again?).
I'll look into it.
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2034000240)
> Is it an option to drop the USDT tests from the `Cirrus CI / ASan + LSan + UBSan + integer, no depends, USDT` job and move them to a GH actions 23.04 VM if "lifting the requirements from the self-hosted runners" is the general direction we want to go? (probably makes sense to merge this sooner than later before adding a GH actions VM job to have CI green again?).
I'll look into it.
💬 mzumsande commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2034005144)
> utxo set snapshots are useful for more than just snapshot loading.
Do you know of any examples of the utxo dumps already being used for something else?
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2034005144)
> utxo set snapshots are useful for more than just snapshot loading.
Do you know of any examples of the utxo dumps already being used for something else?
💬 maflcko commented on pull request "ci: Temporarily disable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2034006509)
> Ok, I did a bit more digging: I was missing the context that #29765 upgraded the CI images to a newer Ubuntu version and that the `Cirrus CI / ASan + LSan + UBSan + integer, no depends, USDT` job [is now failing](https://github.com/bitcoin/bitcoin/pull/29786/checks?check_run_id=23346486009).
29765 didn't do the switch, it was done in fa83b65ef8934b44fbac02da8dbc27fc0bc230e6 last year.
> Is it an option to drop the USDT tests from the `Cirrus CI / ASan + LSan + UBSan + integer, no dep
...
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2034006509)
> Ok, I did a bit more digging: I was missing the context that #29765 upgraded the CI images to a newer Ubuntu version and that the `Cirrus CI / ASan + LSan + UBSan + integer, no depends, USDT` job [is now failing](https://github.com/bitcoin/bitcoin/pull/29786/checks?check_run_id=23346486009).
29765 didn't do the switch, it was done in fa83b65ef8934b44fbac02da8dbc27fc0bc230e6 last year.
> Is it an option to drop the USDT tests from the `Cirrus CI / ASan + LSan + UBSan + integer, no dep
...
💬 0xB10C commented on pull request "ci: Temporarily disable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2034008362)
> GH actions 23.04 VM
GitHub currently only supports: `ubuntu-latest` (22.04), `ubuntu-22.04`, and `ubuntu-20.04`
https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2034008362)
> GH actions 23.04 VM
GitHub currently only supports: `ubuntu-latest` (22.04), `ubuntu-22.04`, and `ubuntu-20.04`
https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
👍 fanquake approved a pull request: "build, depends: Fix `libmultiprocess` cross-compilation"
(https://github.com/bitcoin/bitcoin/pull/29665#pullrequestreview-1976129823)
ACK 2de2ea2ff63b97eacb23234932c6e1f1f65e4494 - I checked that this fixes the macOS cross-compilation issue. I'm assuming these packages are also likely to change further in the (near) future, given the changes going in upstream: https://github.com/chaincodelabs/libmultiprocess/pulls?q=is%3Apr+is%3Aclosed.
> I don't actually understand why the fix is needed, when this was broken, or why mpgen linking should fail when it is cross compiled.
I agree with Russ that it'd be better to actually f
...
(https://github.com/bitcoin/bitcoin/pull/29665#pullrequestreview-1976129823)
ACK 2de2ea2ff63b97eacb23234932c6e1f1f65e4494 - I checked that this fixes the macOS cross-compilation issue. I'm assuming these packages are also likely to change further in the (near) future, given the changes going in upstream: https://github.com/chaincodelabs/libmultiprocess/pulls?q=is%3Apr+is%3Aclosed.
> I don't actually understand why the fix is needed, when this was broken, or why mpgen linking should fail when it is cross compiled.
I agree with Russ that it'd be better to actually f
...
💬 mzumsande commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2034027583)
> @mzumsande, now that unit tests are being run in parallel, we could treat them like any other test (i.e. no special treatment with `--skipunit`, so this option could now be removed). Do you see this aligning with the original goals of `skipunit` or is there something else to consider?
Yes, if they are treated like other tests, `-skipunit` is no longer necessary and should be removed. That's even better than the status quo because it removes the need to remember that `-skipunit` exists when
...
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2034027583)
> @mzumsande, now that unit tests are being run in parallel, we could treat them like any other test (i.e. no special treatment with `--skipunit`, so this option could now be removed). Do you see this aligning with the original goals of `skipunit` or is there something else to consider?
Yes, if they are treated like other tests, `-skipunit` is no longer necessary and should be removed. That's even better than the status quo because it removes the need to remember that `-skipunit` exists when
...
🚀 fanquake merged a pull request: "build, depends: Fix `libmultiprocess` cross-compilation"
(https://github.com/bitcoin/bitcoin/pull/29665)
(https://github.com/bitcoin/bitcoin/pull/29665)
👍 fanquake approved a pull request: "[25.x] Finalize 25.2"
(https://github.com/bitcoin/bitcoin/pull/29794#pullrequestreview-1976147858)
ACK e95c484f7de7fe30d5a2e87c0e7e2a5faf9c1a5f
(https://github.com/bitcoin/bitcoin/pull/29794#pullrequestreview-1976147858)
ACK e95c484f7de7fe30d5a2e87c0e7e2a5faf9c1a5f
🚀 fanquake merged a pull request: "[25.x] Finalize 25.2"
(https://github.com/bitcoin/bitcoin/pull/29794)
(https://github.com/bitcoin/bitcoin/pull/29794)
🚀 fanquake merged a pull request: "depends: add -g to DEBUG=1 flags"
(https://github.com/bitcoin/bitcoin/pull/29527)
(https://github.com/bitcoin/bitcoin/pull/29527)
💬 fanquake commented on pull request "depends: add -g to DEBUG=1 flags":
(https://github.com/bitcoin/bitcoin/pull/29527#issuecomment-2034083503)
Going to followup with an optimization flags alingment RFC.
(https://github.com/bitcoin/bitcoin/pull/29527#issuecomment-2034083503)
Going to followup with an optimization flags alingment RFC.
💬 glozow commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#issuecomment-2034094652)
(Yay v26.1) I have a couple more things for the last section, and then will take out of draft.
(https://github.com/bitcoin/bitcoin/pull/29645#issuecomment-2034094652)
(Yay v26.1) I have a couple more things for the last section, and then will take out of draft.
💬 fanquake commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2034145974)
> IIRC usage was turned into a compile error (maybe only intermittently, will check again).
I think I was thinking of something else here. Have changed this to just adding the new macro.
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2034145974)
> IIRC usage was turned into a compile error (maybe only intermittently, will check again).
I think I was thinking of something else here. Have changed this to just adding the new macro.
👋 fanquake's pull request is ready for review: "depends: add new LLVM debug macro"
(https://github.com/bitcoin/bitcoin/pull/29781)
(https://github.com/bitcoin/bitcoin/pull/29781)
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-2034186584)
> Dealing with a read-only tmp file is probably outside the scope of this PR, but maybe we should reconsider if +rw might actually be the correct mode to use.
@luke-jr that seems like an unlikely situation (unless bitcoind is killed in the moment between writing the temporary cookie file and renaming it?), nonetheless, it could be addressed by changing where we apply the new more restrictive `400` permissions, from the temp file to the renamed `.cookie`.
I don't think the temporary cookie
...
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-2034186584)
> Dealing with a read-only tmp file is probably outside the scope of this PR, but maybe we should reconsider if +rw might actually be the correct mode to use.
@luke-jr that seems like an unlikely situation (unless bitcoind is killed in the moment between writing the temporary cookie file and renaming it?), nonetheless, it could be addressed by changing where we apply the new more restrictive `400` permissions, from the temp file to the renamed `.cookie`.
I don't think the temporary cookie
...
💬 tdb3 commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2034218395)
Rebased and pushed. `skipunit` now removed.
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2034218395)
Rebased and pushed. `skipunit` now removed.
💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034241283)
Tested self-built MSVC binaries for 26.0 and 27.0rc1, definitely no regression.
26.0:

27.0rc1:

Note that I made corrections to my previous graphs because of a bug that caused some runs to leak from one graph to the other. The regression is even more prominent now.
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034241283)
Tested self-built MSVC binaries for 26.0 and 27.0rc1, definitely no regression.
26.0:

27.0rc1:

Note that I made corrections to my previous graphs because of a bug that caused some runs to leak from one graph to the other. The regression is even more prominent now.
💬 maflcko commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034250821)
> Tested self-built MSVC binaries for 26.0 and 27.0rc1, definitely no regression.
I forgot to mention that optimization was enabled in commit 41e378a0a1f0729b423034f47c28a4e83287a1e8. So for a fair comparison, one would have to backport 41e378a0a1f0729b423034f47c28a4e83287a1e8 to 26.0.
I presume that benchmarking IBD is expensive. So maybe comparing the micro-benchmarks can provide a hint at which code is slower?
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034250821)
> Tested self-built MSVC binaries for 26.0 and 27.0rc1, definitely no regression.
I forgot to mention that optimization was enabled in commit 41e378a0a1f0729b423034f47c28a4e83287a1e8. So for a fair comparison, one would have to backport 41e378a0a1f0729b423034f47c28a4e83287a1e8 to 26.0.
I presume that benchmarking IBD is expensive. So maybe comparing the micro-benchmarks can provide a hint at which code is slower?
💬 fanquake commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034264315)
> Tested pre-built binaries on Debian 12 WSL on the same system, measured 45 runs of each. No performance regression,
> Tested self-built MSVC binaries for 26.0 and 27.0rc1, definitely no regression.
It's not clear to me if there is still an issue here or not. Or is there still a regression when comparing our 26.x and 27.x Windows release binaries?
If the only issue is in relation to self-compiled MSVC binaries, then this isn't a blocker for 27.x.
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034264315)
> Tested pre-built binaries on Debian 12 WSL on the same system, measured 45 runs of each. No performance regression,
> Tested self-built MSVC binaries for 26.0 and 27.0rc1, definitely no regression.
It's not clear to me if there is still an issue here or not. Or is there still a regression when comparing our 26.x and 27.x Windows release binaries?
If the only issue is in relation to self-compiled MSVC binaries, then this isn't a blocker for 27.x.