💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2812322401)
`bb822a5ee1...ee16345af3`: address suggestions

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048596931)
Done.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048597720)
Done. This removes all build system changes from this PR, thanks!

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048598114)
Removed.

💬 hebasto commented on pull request "guix: Remove unused `file` package":
(https://github.com/bitcoin/bitcoin/pull/32242#issuecomment-2812325561)
> > My only guess is that it was used by autotools detection code indirectly, somehow.
>
> Yea, `file` was checked-for by Autotools, but maybe non of the related checks affected our build.
>
> If the conclusion is that this was never used, can the PR title, commit message, and PR description be fixed to reflect that.

Thanks! Done.

💬 fanquake commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048601918)
Same here, as the `clock_gettime` alternative comment.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048602251)
The linked article is long and convoluted and here I want to highlight only a short part of it that is relevant. Also having the text here is helpful if the web page disappears in the future.

💬 l0rinc commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#issuecomment-2812335126)
utACK 27f11217ca63e0f8f78f14db139150052dcd9962

💬 hebasto commented on pull request "build: Restore cross-compilation for Android":
(https://github.com/bitcoin/bitcoin/pull/32262#issuecomment-2812338837)
> I'd say a CI task config should exist. Otherwise, there is no way to test the build easily for non-android people. No opinion on adding it to the live CI matrix here in this repo.

I meant this ^ without adding to the live CI matrix in this repo.

💬 fanquake commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048607821)
Then I'd suggest atleast trying to consolidate what is here, given you already link to the same place above, with just "GetThreadTimes():" as a comment (that seems unecessary).

💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048612501)
But this introduces another inconsistency between Windows and other platforms. Initially, `run: ./bin/bench_bitcoin.exe -sanity-check -priority-level=high` was written to mimic the `bench_sanity_check_high_priority` test task behaviour.

💬 fanquake commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048614265)
Can you explain your issue with running more tests in this CI?

💬 hebasto commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#issuecomment-2812355350)
> ... that should probably be fixed _properly_, in some way.

I haven't found a way to do it that covers all use cases, is user-friendly, and doesn't introduce excessive complexity.

💬 hebasto commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#discussion_r2048621583)
> This says it applies to users with Homebrew installed, when _not_ building with homebrew-provided tools? What are the "tools" here?

For example, LLVM toolchain.

💬 ismaelsadeeq commented on issue "RPC: `getblockstats` might not return the *effective* percentile fee rate":
(https://github.com/bitcoin/bitcoin/issues/31140#issuecomment-2812361361)
> Because of this sorting, "effective" feerate isn't really meaningful.

When a child transaction (fee rate 100) is paying for parent transactions (fee rates 4 and 5), they should be considered as a single chunk with a combined fee rate (e.g., ~40).
So instead of sorting as [100, 5, 4, 3, 2, 2, 1, 1, 1], a more accurate representation would be [40, 3, 2, 2, 1, 1]. This will provide a more accurate view (different percentile fee rate when they are sorted individually) that represents what fee rat
...

💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048628558)
If it only takes a couple of extra seconds, why run a different set of tests for each platform?

Windows should, to the best of our ability, be treated the same as any other platform.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2812395790)
`ee16345af3...19c8336d97`: address suggestions

💬 fanquake commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048646279)
> If it only takes a couple of extra seconds,

I haven't measured, for all users, developers and CIs. The point of this change is to immediately start catching regressions in at least one CI.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048646883)
Reduced the text and removed the link to the doc since it is already a few lines earlier.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048648039)
Removed and added a note to the PR description about the alternatives.