💬 davidgumberg commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2336867846)
@andrewtoth
>> My bench logs indicate that the regression is coming from writing block undo data, the difference between the (reported) total block connection time between v27.1 and v28.0rc1 is +1,337.85 seconds for v28, and the difference in (reported) time spent writing undo block data is +1,579.31s for v28:
>
> This is saying that the total time for writing undo block data exceeds total block connection time. But, writing undo block data is a subsection of block connection time, so it sho
...
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2336867846)
@andrewtoth
>> My bench logs indicate that the regression is coming from writing block undo data, the difference between the (reported) total block connection time between v27.1 and v28.0rc1 is +1,337.85 seconds for v28, and the difference in (reported) time spent writing undo block data is +1,579.31s for v28:
>
> This is saying that the total time for writing undo block data exceeds total block connection time. But, writing undo block data is a subsection of block connection time, so it sho
...
💬 maflcko commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1749642423)
> made the test robust for me: [fjahr@ef00b8e](https://github.com/fjahr/bitcoin/commit/ef00b8eab48cf7ebcc8a0539f237175b8f5a0373)
Thanks for checking. Using a clean chain means that the cache won't be used, which normally lives on the storage unit where the build dir lives, so only the temp dir should be used. However, given the prior result that a non-tmp folder was generally fine, I would have expected the opposite result (Test would be less fragile by using the persisted cache on a non-tmp
...
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1749642423)
> made the test robust for me: [fjahr@ef00b8e](https://github.com/fjahr/bitcoin/commit/ef00b8eab48cf7ebcc8a0539f237175b8f5a0373)
Thanks for checking. Using a clean chain means that the cache won't be used, which normally lives on the storage unit where the build dir lives, so only the temp dir should be used. However, given the prior result that a non-tmp folder was generally fine, I would have expected the opposite result (Test would be less fragile by using the persisted cache on a non-tmp
...
⚠️ maflcko opened an issue: "CI timeouts"
(https://github.com/bitcoin/bitcoin/issues/30851)
Looks like some CI tasks are timing out, starting a few days ago.
It would be good to investigate and fix the problem.
A quick glance on the ccache stats may indicate that ccache stopped working for some tasks.
However, CI should also pass with a clean ccache, so the issue may be a different one, or there may be more than one issue.
(https://github.com/bitcoin/bitcoin/issues/30851)
Looks like some CI tasks are timing out, starting a few days ago.
It would be good to investigate and fix the problem.
A quick glance on the ccache stats may indicate that ccache stopped working for some tasks.
However, CI should also pass with a clean ccache, so the issue may be a different one, or there may be more than one issue.
💬 maflcko commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2337273254)
It could be a ccache issue, but another look is needed, see https://github.com/bitcoin/bitcoin/issues/30851
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2337273254)
It could be a ccache issue, but another look is needed, see https://github.com/bitcoin/bitcoin/issues/30851
💬 davidgumberg commented on issue "docs: Windows build intructions result in a large binary":
(https://github.com/bitcoin/bitcoin/issues/30593#issuecomment-2337273374)
+1 to adding a note in `docs/build-windows.md`
(https://github.com/bitcoin/bitcoin/issues/30593#issuecomment-2337273374)
+1 to adding a note in `docs/build-windows.md`
✅ maflcko closed a pull request: "test: use assert_greater_than util"
(https://github.com/bitcoin/bitcoin/pull/30019)
(https://github.com/bitcoin/bitcoin/pull/30019)
💬 maflcko commented on pull request "test: use assert_greater_than util":
(https://github.com/bitcoin/bitcoin/pull/30019#issuecomment-2337298742)
Closing for now due to inactivity on a test-only change for more than 6 months. Leave a comment below, if you want this re-opened.
(https://github.com/bitcoin/bitcoin/pull/30019#issuecomment-2337298742)
Closing for now due to inactivity on a test-only change for more than 6 months. Leave a comment below, if you want this re-opened.
💬 maflcko commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1749727270)
> And `UCharSpanCast` seems to work with `Span`, which I understood we're trying to get away from.
Correct. Sorry for the brevity. I wanted to say that my preference would be to find a way that is applicable to all places in the future that need to cast the inner type of a span from one byte type to another. (I don't think all code will ever be converted to `std::byte`, because some interfaces (to C code) really only work with `unsigned char`, so a conversion will be needed in the future for
...
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1749727270)
> And `UCharSpanCast` seems to work with `Span`, which I understood we're trying to get away from.
Correct. Sorry for the brevity. I wanted to say that my preference would be to find a way that is applicable to all places in the future that need to cast the inner type of a span from one byte type to another. (I don't think all code will ever be converted to `std::byte`, because some interfaces (to C code) really only work with `unsigned char`, so a conversion will be needed in the future for
...
👍 vasild approved a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-2289065969)
ACK f1704c021e615f740130fff7cd47093d708a3028
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-2289065969)
ACK f1704c021e615f740130fff7cd47093d708a3028
👍 vasild approved a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2289165721)
ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc
This code gives me a headache :exploding_head: (the old one, the new one - less)
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2289165721)
ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc
This code gives me a headache :exploding_head: (the old one, the new one - less)
👍 TheCharlatan approved a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2289201595)
ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2289201595)
ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc
👍 TheCharlatan approved a pull request: "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"
(https://github.com/bitcoin/bitcoin/pull/30842#pullrequestreview-2289228112)
ACK b07fe666f27e2b2515d2cb5a0339512045e64761
(https://github.com/bitcoin/bitcoin/pull/30842#pullrequestreview-2289228112)
ACK b07fe666f27e2b2515d2cb5a0339512045e64761
💬 fanquake commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2337495736)
> This PR aims to reduce the build time.
Can you update the PR description to clarify that this is only relevant for native windows builds. Can you also add the explanation for why this only makes a difference on Windows.
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2337495736)
> This PR aims to reduce the build time.
Can you update the PR description to clarify that this is only relevant for native windows builds. Can you also add the explanation for why this only makes a difference on Windows.
💬 fanquake commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1749844206)
Are you planning on updating this comment given the above? Seems like we can have something a bit less vauge, which makes it clear that this is something that is going to have to wait a long time, and isn't actually directly dependant on the distros. Although, it seems like we could just switch the code to try both now, given that is likely what we'll have to do (and keep) in any case.
(https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1749844206)
Are you planning on updating this comment given the above? Seems like we can have something a bit less vauge, which makes it clear that this is something that is going to have to wait a long time, and isn't actually directly dependant on the distros. Although, it seems like we could just switch the code to try both now, given that is likely what we'll have to do (and keep) in any case.
💬 fanquake commented on issue "Trying to run bitcoin qt on Windows and getting an AV":
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2337551159)
Not sure what to do here. Are there steps for debugging? @hebasto given this is Qt on Windows?
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2337551159)
Not sure what to do here. Are there steps for debugging? @hebasto given this is Qt on Windows?
💬 fanquake commented on pull request "cmake: add `USE_SOURCE_PERMISSIONS` to all `configure_file()` usage":
(https://github.com/bitcoin/bitcoin/pull/30823#issuecomment-2337575044)
> If https://github.com/bitcoin/bitcoin/pull/30838 happens to fix https://github.com/bitcoin/bitcoin/issues/30815, it seems reasonable to switch to NO_SOURCE_PERMISSIONS in all cases for consistency.
I'm going to merge this, and we can follow up in #30838, and decide if we to change all call sites, or just those needed for reproducibility. That PR will also need to be updated with an explantion of why this started happening, and the fix (I assume there should be no issue always using CMakes d
...
(https://github.com/bitcoin/bitcoin/pull/30823#issuecomment-2337575044)
> If https://github.com/bitcoin/bitcoin/pull/30838 happens to fix https://github.com/bitcoin/bitcoin/issues/30815, it seems reasonable to switch to NO_SOURCE_PERMISSIONS in all cases for consistency.
I'm going to merge this, and we can follow up in #30838, and decide if we to change all call sites, or just those needed for reproducibility. That PR will also need to be updated with an explantion of why this started happening, and the fix (I assume there should be no issue always using CMakes d
...
💬 fanquake commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2337609950)
> A quick glance on the ccache stats may indicate that ccache stopped working for some tasks.
Yea, seems like our ccache efficiency has tanked post-CMake. Is it possible this is somehow a stagnant cache or related issue (although given it's been 2 weeks since the switch, I'd think caches would have cycled)?.
ccache hits for some CI jobs of the last merge into master (https://github.com/bitcoin/bitcoin/runs/29789973072):
32-bit CentOS : `Hits: 1 / 715 (0.14 %)`
Win64 Cros
...
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2337609950)
> A quick glance on the ccache stats may indicate that ccache stopped working for some tasks.
Yea, seems like our ccache efficiency has tanked post-CMake. Is it possible this is somehow a stagnant cache or related issue (although given it's been 2 weeks since the switch, I'd think caches would have cycled)?.
ccache hits for some CI jobs of the last merge into master (https://github.com/bitcoin/bitcoin/runs/29789973072):
32-bit CentOS : `Hits: 1 / 715 (0.14 %)`
Win64 Cros
...
🚀 fanquake merged a pull request: "doc: fix minor typo"
(https://github.com/bitcoin/bitcoin/pull/30850)
(https://github.com/bitcoin/bitcoin/pull/30850)
💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1749932934)
Good idea, added `BytesToUInt8s` and `UInt8sToBytes`.
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1749932934)
Good idea, added `BytesToUInt8s` and `UInt8sToBytes`.
🚀 fanquake merged a pull request: "cmake: add `USE_SOURCE_PERMISSIONS` to all `configure_file()` usage"
(https://github.com/bitcoin/bitcoin/pull/30823)
(https://github.com/bitcoin/bitcoin/pull/30823)