Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 hebasto commented on issue "cmake: GenerateHeaderFrom very slow":
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2346638003)
> Reverting #30842 fixes the issue for me.

Done in https://github.com/bitcoin/bitcoin/pull/30883.
💬 maflcko commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1757134512)
q: Is there a reason why PACKAGE_NAME is used above and below, but not here?
🤔 HumayunMhmadi requested changes to a pull request: "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"
(https://github.com/bitcoin/bitcoin/pull/30842#pullrequestreview-2300746284)
IMG_20240904_132132_468.jpg](https://github.com/user-attachments/assets/bf11748e-d475-4f4d-870c-6691730baba7)
💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757143565)
> `CCACHE_BASEDIR=${CMAKE_BINARY_DIR}` is effective when building in different build directories from the same source directory.
>
> `CCACHE_BASEDIR=${CMAKE_SOURCE_DIR}` works for building from different source directories.
>
> We need to choose one of these two options.

Thanks, that clarifies a lot. I only do the second not the first, because I use git worktrees, and I assumed this PR was trying to make git worktrees and multiple checkouts work better. Maybe people who work on build sy
...
💬 maflcko commented on pull request "Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"":
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346656947)
review ACK fdeb717e78fd55fbfda199c87002358bdc5895af
💬 fanquake commented on pull request "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"":
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346666009)
Going to merge this, given multiple developers have reported issues.
fanquake closed an issue: "cmake: GenerateHeaderFrom very slow"
(https://github.com/bitcoin/bitcoin/issues/30881)
🚀 fanquake merged a pull request: "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`""
(https://github.com/bitcoin/bitcoin/pull/30883)
💬 jonatack commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#discussion_r1757158064)
@martinus PR needs rebase/update to cmake. This makefile has been removed.
📝 sipa opened a pull request: "streams: cache file position within AutoFile"
(https://github.com/bitcoin/bitcoin/pull/30884)
This is intended to address #30833.
👍 instagibbs approved a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2300766803)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08

seems straight forward to me, and all usages appear to be concerned with wallet/mempool things, not consensus related activities which may require weight-level precision.

For weight-level consensus precision I suspect wallet-like functionality is less interesting.
💬 instagibbs commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1757153370)
without this multiplier was there an issue in this PR?
💬 maflcko commented on pull request "wallet: clarify FundTransaction signature":
(https://github.com/bitcoin/bitcoin/pull/29564#issuecomment-2346714791)
@S3RK Are you still working on this?
💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757184931)
> Also maybe I need to think about it more, but I'm not sure why choosing CMAKE_BINARY_DIR would be better than CMAKE_SOURCE_DIR in either case as long a CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR. Are we trying to optimize for cases where CMAKE_BINARY_DIR is not a subdirectory?

I don't think it is related to whether `CMAKE_BINARY_DIR` is a subdirectory of `CMAKE_SOURCE_DIR`.

Due to https://github.com/bitcoin/bitcoin/blob/07c7c96022dd325be1cd3b353d575eb6a5593f55/src/CMakeLists.
...
💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2346723974)
reACK a5ea6467b6a694facc1f5efe460bf30539167597

via `git range-diff master 59028de422bbf8ccf53da27f1153e343952e585f a5ea6467b6a694facc1f5efe460bf30539167597`

Just benchmark changes, with examples 11-14 added in 2fcfb4c0392a2f3114195e6e9979c1431798632c and 15-19 in 8fbc6f3c33c6a5b23314279f0856de0625cc6d3f

New benchmarks all run for each commit properly post-introduction.
my benchmark runs for comparison starting from introduction commit to tip: https://gist.github.com/instagibbs/058f5158
...
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2346729363)
I rebased the PR against latest `master`. Manually resolved a small code conflict in src/chainparams.cpp and a large one in contrib/signet/miner. In new OOP version of contrib/signet/miner my change is much smaller :)

Also I noticed that Makefile based testing was replaced with CMake based approach. I added new C++ file with tests (chainparams_tests.cpp) to src/test/CMakeLists.txt instead of src/Makefile.test.include.
💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757199833)
> I don't think it is related to whether `CMAKE_BINARY_DIR` is a subdirectory of `CMAKE_SOURCE_DIR` or not.

Ccache documentation describes behavior of base_dir: "ccache will rewrite absolute paths into paths relative to the current working directory, but only absolute paths that begin with **base_dir**"

So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across
...
💬 maflcko commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757200582)
> last commit, which was mostly motivated by [#30811 (comment)](https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2331096969).

Sorry, I was just testing, not trying to imply that this is the common case. Using worktrees (different source dirs) seems more common than different build dirs for the same cmake config.
🤔 murchandamus reviewed a pull request: "RPC: improve SFFO arg parsing, error catching and coverage"
(https://github.com/bitcoin/bitcoin/pull/30844#pullrequestreview-2300840916)
Awesome, thanks for picking this up.

crACK cddcbaf81e8e3d107cd321ee2c9a7fd786a8917e
davidgumberg closed a pull request: "Lint: improve subtree exclusion"
(https://github.com/bitcoin/bitcoin/pull/29965)