💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399027783)
Prefiltering on the main thread is too slow. It is several milliseconds to check every input in large blocks whether they exist in the cache.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399027783)
Prefiltering on the main thread is too slow. It is several milliseconds to check every input in large blocks whether they exist in the cache.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399028082)
Done.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399028082)
Done.
💬 alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399028947)
Feel like we can remove this added line
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399028947)
Feel like we can remove this added line
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399032242)
I believe this is resolved.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399032242)
I believe this is resolved.
💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#issuecomment-3361478681)
For future me, wondering how to make fuzzing work on a Mac with latest Clang + AppleClang - this is the version that mostly works for me (though it fails in a lot of places where Linux doesn't):
```
rm -rfd build_fuzz && cmake --preset=libfuzzer \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_OSX_SYSROOT="$(xcrun --show-sdk-path)" \
-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld -L$(brew --prefix llvm)/lib/c++
...
(https://github.com/bitcoin/bitcoin/pull/32084#issuecomment-3361478681)
For future me, wondering how to make fuzzing work on a Mac with latest Clang + AppleClang - this is the version that mostly works for me (though it fails in a lot of places where Linux doesn't):
```
rm -rfd build_fuzz && cmake --preset=libfuzzer \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_OSX_SYSROOT="$(xcrun --show-sdk-path)" \
-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld -L$(brew --prefix llvm)/lib/c++
...
👍 l0rinc approved a pull request: "build: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3294934753)
Concept ACK for the version bump
Looking around for more versions and checking https://github.com/bitcoin/bitcoin/commit/446e73cc0bb7cd628039eaf9b1bcc93db23b924f it seem to me we could update a few more places:
* https://github.com/bitcoin/bitcoin/blob/3fa1185dda3b000b9c3956422fd2351e40969dec/.github/ISSUE_TEMPLATE/bug.yml?plain=1#L81
* https://github.com/bitcoin/bitcoin/blob/3b824169c7766460794bb445028ac55a7111ee3e/contrib/guix/README.md?plain=1#L252
Note that latest `AppleClang` doesn't play
...
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3294934753)
Concept ACK for the version bump
Looking around for more versions and checking https://github.com/bitcoin/bitcoin/commit/446e73cc0bb7cd628039eaf9b1bcc93db23b924f it seem to me we could update a few more places:
* https://github.com/bitcoin/bitcoin/blob/3fa1185dda3b000b9c3956422fd2351e40969dec/.github/ISSUE_TEMPLATE/bug.yml?plain=1#L81
* https://github.com/bitcoin/bitcoin/blob/3b824169c7766460794bb445028ac55a7111ee3e/contrib/guix/README.md?plain=1#L252
Note that latest `AppleClang` doesn't play
...
💬 amishhaa commented on pull request "contrib: fix macOS deployment with no translations":
(https://github.com/bitcoin/bitcoin/pull/33482#issuecomment-3361487099)
hey @ismaelsadeeq ,Thank you for further highlighting that it is optional by the config comment, ill add this to the PR description as well and more details.
(https://github.com/bitcoin/bitcoin/pull/33482#issuecomment-3361487099)
hey @ismaelsadeeq ,Thank you for further highlighting that it is optional by the config comment, ill add this to the PR description as well and more details.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399049434)
This is dangerous because it doesn't check for freshness or if already inserted. It is meant to bulk load new utxos from the assume utxo set. Since assume utxo assumes the utxo set is currently empty, the coins would always be inserted.
This is repurposed here to bulk load utxos from the db directly into the cache. However, an invalid block could be mined which spends an already spent utxo that is in the cache but has not been synced to the db yet. In that case,
the insertion will fail here.
...
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399049434)
This is dangerous because it doesn't check for freshness or if already inserted. It is meant to bulk load new utxos from the assume utxo set. Since assume utxo assumes the utxo set is currently empty, the coins would always be inserted.
This is repurposed here to bulk load utxos from the db directly into the cache. However, an invalid block could be mined which spends an already spent utxo that is in the cache but has not been synced to the db yet. In that case,
the insertion will fail here.
...
💬 sipa commented on pull request "Improve LastCommonAncestor performance + add tests":
(https://github.com/bitcoin/bitcoin/pull/33515#discussion_r2399052098)
Done.
(https://github.com/bitcoin/bitcoin/pull/33515#discussion_r2399052098)
Done.
💬 sipa commented on pull request "Improve LastCommonAncestor performance + add tests":
(https://github.com/bitcoin/bitcoin/pull/33515#discussion_r2399052318)
Done.
(https://github.com/bitcoin/bitcoin/pull/33515#discussion_r2399052318)
Done.
💬 sipa commented on pull request "Improve LastCommonAncestor performance + add tests":
(https://github.com/bitcoin/bitcoin/pull/33515#discussion_r2399052574)
Done.
(https://github.com/bitcoin/bitcoin/pull/33515#discussion_r2399052574)
Done.
💬 alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399052921)
I suggest, we move the `index` -> `tree entry` change to the previous commit.
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399052921)
I suggest, we move the `index` -> `tree entry` change to the previous commit.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399056272)
We iterate the prevouts directly from the block now. However, we store the tx index and vin index in a global vector now. This way we can flatten the inputs instead of having to scan the txs to see how many inputs they have.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399056272)
We iterate the prevouts directly from the block now. However, we store the tx index and vin index in a global vector now. This way we can flatten the inputs instead of having to scan the txs to see how many inputs they have.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399059149)
Not sure, would have to benchmark this.
I have updated the functions though to make the main thread's entrance clearer.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399059149)
Not sure, would have to benchmark this.
I have updated the functions though to make the main thread's entrance clearer.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399062889)
It is definitely safe to do, since all access would be on main thread. It is also safe to do from parallel threads if we don't write until all threads are done reading, which is what this PR does.
Prefiltering is slow though (several milliseconds) so is better to do in parallel.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399062889)
It is definitely safe to do, since all access would be on main thread. It is also safe to do from parallel threads if we don't write until all threads are done reading, which is what this PR does.
Prefiltering is slow though (several milliseconds) so is better to do in parallel.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399067712)
I'm not sure I understand this.
The m_txids set is computed on the main thread, and is only read from multiple threads. If we didn't do this we would try to fetch non-existent outputs from the db, which would be much slower.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399067712)
I'm not sure I understand this.
The m_txids set is computed on the main thread, and is only read from multiple threads. If we didn't do this we would try to fetch non-existent outputs from the db, which would be much slower.
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399069027)
Not sure why that's problematic, we don't *have* to have perfect parallelism, it seems to me we can assume uniform distribution - it's fine if there are outliers if that makes the code simpler (which I think it should, it could even eliminate most locks, since the jobs are basically completely independent)
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399069027)
Not sure why that's problematic, we don't *have* to have perfect parallelism, it seems to me we can assume uniform distribution - it's fine if there are outliers if that makes the code simpler (which I think it should, it could even eliminate most locks, since the jobs are basically completely independent)
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399069282)
> I don't understand what this has to do with being lock free.
We may have fewer file system locks if the threads are accessing different regions
> I've updated to use semaphores instead of mutex
I will review that in more detail soon, probably next week.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399069282)
> I don't understand what this has to do with being lock free.
We may have fewer file system locks if the threads are accessing different regions
> I've updated to use semaphores instead of mutex
I will review that in more detail soon, probably next week.
💬 alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399070131)
Think this can be removed.
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399070131)
Think this can be removed.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399071701)
We don't really care, but it would be good to not continue doing work here if we know it's pointless. This just exits early. No validation is happening.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399071701)
We don't really care, but it would be good to not continue doing work here if we know it's pointless. This just exits early. No validation is happening.