π maflcko opened a pull request: " ci: Retry image building once on failure "
(https://github.com/bitcoin/bitcoin/pull/33677)
This should fix https://github.com/bitcoin/bitcoin/issues/33640.
It also contains a few refactor cleanups, which are explained in the corresponding commits.
(https://github.com/bitcoin/bitcoin/pull/33677)
This should fix https://github.com/bitcoin/bitcoin/issues/33640.
It also contains a few refactor cleanups, which are explained in the corresponding commits.
π¬ l0rinc commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432095621)
I don't really see any speedup for `AssembleBlock`
<img width="679" height="476" alt="image" src="https://github.com/user-attachments/assets/fc804b80-241b-446f-bfc1-28b222d3190e" />
Change: gcc=-0.71%, clang=-0.82%
------
`CCoinsCaching` does seem to be improved a bit
<img width="664" height="477" alt="image" src="https://github.com/user-attachments/assets/fc248746-1716-41b1-ae0c-ac08b6c3dbd5" />
Change: gcc=+18.87%, clang=+11.79%
------
<details>
<summary>Benchmark</sum
...
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432095621)
I don't really see any speedup for `AssembleBlock`
<img width="679" height="476" alt="image" src="https://github.com/user-attachments/assets/fc804b80-241b-446f-bfc1-28b222d3190e" />
Change: gcc=-0.71%, clang=-0.82%
------
`CCoinsCaching` does seem to be improved a bit
<img width="664" height="477" alt="image" src="https://github.com/user-attachments/assets/fc248746-1716-41b1-ae0c-ac08b6c3dbd5" />
Change: gcc=+18.87%, clang=+11.79%
------
<details>
<summary>Benchmark</sum
...
π¬ laanwj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3432096233)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3432096233)
Concept ACK
π ismaelsadeeq's pull request is ready for review: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676)
(https://github.com/bitcoin/bitcoin/pull/33676)
π¬ cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432164685)
> > Not so sure about the `Assemble Block` bench though. Doesn't it measure block assembly **_after_** admission, therefore not timing the policy checks you changed?
>
> Thanks for the review & benchmarks. I'm positive about the fact that the `AssembleBlock` bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.
It _does_ call code on the policy path, but only during setup, not during the timed region.
In
...
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432164685)
> > Not so sure about the `Assemble Block` bench though. Doesn't it measure block assembly **_after_** admission, therefore not timing the policy checks you changed?
>
> Thanks for the review & benchmarks. I'm positive about the fact that the `AssembleBlock` bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.
It _does_ call code on the policy path, but only during setup, not during the timed region.
In
...
π¬ maflcko commented on issue "ci: short read: expected xxxxxxxxx bytes but got xxxxxxxxx: unexpected EOF":
(https://github.com/bitcoin/bitcoin/issues/33640#issuecomment-3432178761)
Similar variation: https://github.com/bitcoin/bitcoin/actions/runs/18713760603/job/53368943173#step:9:165:
```
Building ci_native_msan image tag to run in
+ docker buildx build --file=/home/admin/actions-runner/_work/bitcoin/bitcoin/ci/test_imagefile --build-arg=CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --build-arg=FILE_ENV=./ci/test/00_setup_env_native_msan.sh --build-arg=BASE_ROOT_DIR=/home/admin/actions-runner/_work/_temp --platform=linux --label=bitcoin-ci-test --tag=ci_native_msan --cac
...
(https://github.com/bitcoin/bitcoin/issues/33640#issuecomment-3432178761)
Similar variation: https://github.com/bitcoin/bitcoin/actions/runs/18713760603/job/53368943173#step:9:165:
```
Building ci_native_msan image tag to run in
+ docker buildx build --file=/home/admin/actions-runner/_work/bitcoin/bitcoin/ci/test_imagefile --build-arg=CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --build-arg=FILE_ENV=./ci/test/00_setup_env_native_msan.sh --build-arg=BASE_ROOT_DIR=/home/admin/actions-runner/_work/_temp --platform=linux --label=bitcoin-ci-test --tag=ci_native_msan --cac
...
π¬ Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432193066)
@l0rinc @cedwies the bench is not presented as evidence. it is presented as a transparent way of saying performance hasn't degraded. the PR body clearly states that it remained the same. I included it for the sake of transparency.
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432193066)
@l0rinc @cedwies the bench is not presented as evidence. it is presented as a transparent way of saying performance hasn't degraded. the PR body clearly states that it remained the same. I included it for the sake of transparency.
π¬ waketraindev commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#issuecomment-3432260201)
* updated PR description and commit message to highlight P2p getaddr responses
* corrected formatting
(https://github.com/bitcoin/bitcoin/pull/33663#issuecomment-3432260201)
* updated PR description and commit message to highlight P2p getaddr responses
* corrected formatting
π¬ maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3432270963)
Looks like the CI re-run passed after one month, so it seems reasonable stable
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3432270963)
Looks like the CI re-run passed after one month, so it seems reasonable stable
π¬ cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432352303)
> But the code seems more complicated than before - is there a way to retain the speedup without complicating the code (i.e. minimizing the diff)?
It is more complicated. I would:
Keep:
- change from index-based to range-based loops.
- In `AreInputsStandard(...)`: Move `vSolutions` outside the for loop (this brings the performance gain in `CCoinsCaching`)
- In `IsWitnessStandard(...)`: Keep the variable name change from `stack` to `script_stack`
Discard (to minimize diff):
- In `Ar
...
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432352303)
> But the code seems more complicated than before - is there a way to retain the speedup without complicating the code (i.e. minimizing the diff)?
It is more complicated. I would:
Keep:
- change from index-based to range-based loops.
- In `AreInputsStandard(...)`: Move `vSolutions` outside the for loop (this brings the performance gain in `CCoinsCaching`)
- In `IsWitnessStandard(...)`: Keep the variable name change from `stack` to `script_stack`
Discard (to minimize diff):
- In `Ar
...
π¬ furszy commented on pull request "net: introduce block tracker to retry to download blocks after failure":
(https://github.com/bitcoin/bitcoin/pull/27837#issuecomment-3432381897)
closing for now, will re-open after rebasing it.
(https://github.com/bitcoin/bitcoin/pull/27837#issuecomment-3432381897)
closing for now, will re-open after rebasing it.
β
furszy closed a pull request: "net: introduce block tracker to retry to download blocks after failure"
(https://github.com/bitcoin/bitcoin/pull/27837)
(https://github.com/bitcoin/bitcoin/pull/27837)
β
furszy closed a pull request: "wallet: fix crash during migration due to invalid multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31378)
(https://github.com/bitcoin/bitcoin/pull/31378)
π¬ furszy commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-3432386506)
closing for now. Not sure if I will ever back to work on this PR.
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-3432386506)
closing for now. Not sure if I will ever back to work on this PR.
π¬ furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#issuecomment-3432388684)
Probably still useful but closing for now due to lack of availability.
(https://github.com/bitcoin/bitcoin/pull/31404#issuecomment-3432388684)
Probably still useful but closing for now due to lack of availability.
β
furszy closed a pull request: "descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404)
(https://github.com/bitcoin/bitcoin/pull/31404)
π furszy opened a pull request: "http: limit RPC server threads to available cores"
(https://github.com/bitcoin/bitcoin/pull/33678)
The HTTP server currently spawns 16 threads by default, regardless of the systemβs available ones.
This change limits the number of threads in the RPC server to the number of available CPUs,
preventing excessive context switching and improving overall responsiveness on systems with
fewer cores. If `-rpcthreads` exceeds that limit, it is adjusted to num_cores - 1 and a warning is
logged.
(https://github.com/bitcoin/bitcoin/pull/33678)
The HTTP server currently spawns 16 threads by default, regardless of the systemβs available ones.
This change limits the number of threads in the RPC server to the number of available CPUs,
preventing excessive context switching and improving overall responsiveness on systems with
fewer cores. If `-rpcthreads` exceeds that limit, it is adjusted to num_cores - 1 and a warning is
logged.
π furszy opened a pull request: "test: set number of RPC server threads to 2"
(https://github.com/bitcoin/bitcoin/pull/33679)
The default `-rpcthreads` value spawns 16 HTTP server threads for each node.
Running the functional test suite with default `rpcthreads` can exhaust file
descriptors or hit other resource limits very easily (more when tests are run
in parallel).
Furthermore, having 16 threads is unnecessary since they are mostly idle. We
run RPC calls on a single RPC connection and wait for it result synchronously.
There is (almost) never two RPC calls occurring concurrently.
Because of this, the threads
...
(https://github.com/bitcoin/bitcoin/pull/33679)
The default `-rpcthreads` value spawns 16 HTTP server threads for each node.
Running the functional test suite with default `rpcthreads` can exhaust file
descriptors or hit other resource limits very easily (more when tests are run
in parallel).
Furthermore, having 16 threads is unnecessary since they are mostly idle. We
run RPC calls on a single RPC connection and wait for it result synchronously.
There is (almost) never two RPC calls occurring concurrently.
Because of this, the threads
...
π¬ maflcko commented on pull request "http: limit RPC server threads to available cores":
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432429726)
Not sure. This seems to be partially reverting e56fc7ce6a92eae7e80657d9f57a148cc002358d. See also https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828025349
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432429726)
Not sure. This seems to be partially reverting e56fc7ce6a92eae7e80657d9f57a148cc002358d. See also https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828025349
π¬ maflcko commented on pull request "test: set number of RPC server threads to 2":
(https://github.com/bitcoin/bitcoin/pull/33679#issuecomment-3432447263)
lgtm ACK e9cd45e3d3c7592265ebf67387090b3df1501df4
Any number larger than `1` should be fine, so that it is still possible to accidentally or intentionally cause races when several test threads call the rpc threads.
(https://github.com/bitcoin/bitcoin/pull/33679#issuecomment-3432447263)
lgtm ACK e9cd45e3d3c7592265ebf67387090b3df1501df4
Any number larger than `1` should be fine, so that it is still possible to accidentally or intentionally cause races when several test threads call the rpc threads.