β
SecondPort closed a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
(https://github.com/bitcoin/bitcoin/pull/31574)
π SecondPort reopened a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
Fixes #31573
(https://github.com/bitcoin/bitcoin/pull/31574)
Fixes #31573
β
SecondPort closed a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
(https://github.com/bitcoin/bitcoin/pull/31574)
π SecondPort opened a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31575)
Fixes #31573
Reduce memory usage during chainstate initialization on low-memory systems.
* **src/txdb.h**
- Reduce the default `dbcache` value to 100 MiB.
* **src/validation.h**
- Add a function to adjust `dbcache` to 100 MiB if the system has less than 4GB of RAM.
* **doc/reduce-memory.md**
- Update the documentation to reflect the new default `dbcache` value.
- Add information about the memory check for systems with less than 4GB of RAM.
(https://github.com/bitcoin/bitcoin/pull/31575)
Fixes #31573
Reduce memory usage during chainstate initialization on low-memory systems.
* **src/txdb.h**
- Reduce the default `dbcache` value to 100 MiB.
* **src/validation.h**
- Add a function to adjust `dbcache` to 100 MiB if the system has less than 4GB of RAM.
* **doc/reduce-memory.md**
- Update the documentation to reflect the new default `dbcache` value.
- Add information about the memory check for systems with less than 4GB of RAM.
π¬ l0rinc commented on pull request "Fix memory issue during chainstate initialization on 2GB box":
(https://github.com/bitcoin/bitcoin/pull/31574#discussion_r1898673122)
NACK
(https://github.com/bitcoin/bitcoin/pull/31574#discussion_r1898673122)
NACK
π¬ l0rinc commented on pull request "Fix memory issue during chainstate initialization on 2GB box":
(https://github.com/bitcoin/bitcoin/pull/31575#issuecomment-2563957123)
> GetTotalSystemMemoryβ was not declared in this scope
Please compile it locally before pushing.
(https://github.com/bitcoin/bitcoin/pull/31575#issuecomment-2563957123)
> GetTotalSystemMemoryβ was not declared in this scope
Please compile it locally before pushing.
π hebasto opened a pull request: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
This PR ensures that the `script_assets_tests` test case is explicitly reported as "Skipped" when it is not run, making it clearer when running the test suite with `ctest`:
- on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:
```
$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 87: script_tests
Start 83: script_p2sh_tests
...
(https://github.com/bitcoin/bitcoin/pull/31576)
This PR ensures that the `script_assets_tests` test case is explicitly reported as "Skipped" when it is not run, making it clearer when running the test suite with `ctest`:
- on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:
```
$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 87: script_tests
Start 83: script_p2sh_tests
...
π hebasto converted_to_draft a pull request: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
This PR ensures that the `script_assets_tests` test case is explicitly reported as "Skipped" when it is not run, making it clearer when running the test suite with `ctest`:
- on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:
```
$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 87: script_tests
Start 83: script_p2sh_tests
...
(https://github.com/bitcoin/bitcoin/pull/31576)
This PR ensures that the `script_assets_tests` test case is explicitly reported as "Skipped" when it is not run, making it clearer when running the test suite with `ctest`:
- on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:
```
$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 87: script_tests
Start 83: script_p2sh_tests
...
π¬ mzumsande commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1898689788)
This comment could be adjusted - `fPeriodicWrite` is no longer just about the block index.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1898689788)
This comment could be adjusted - `fPeriodicWrite` is no longer just about the block index.
π hebasto's pull request is ready for review: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
(https://github.com/bitcoin/bitcoin/pull/31576)
π¬ deepgrewal commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2563996883)
I do not agree with this change and it should not be merged.
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2563996883)
I do not agree with this change and it should not be merged.
π¬ davidgumberg commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2564008796)
> The node in question finished sync after I ran it inside heaptrack and is humming along just fine now. I printed out the flamegraph although I have trouble interpreting it:
>
> [ flamegraph.svg]
At first glance it seems pretty unusual that so much time (~40% of samples) is spent in the call stack above `CScriptCheck::operator()` seemingly doing allocations.
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2564008796)
> The node in question finished sync after I ran it inside heaptrack and is humming along just fine now. I printed out the flamegraph although I have trouble interpreting it:
>
> [ flamegraph.svg]
At first glance it seems pretty unusual that so much time (~40% of samples) is spent in the call stack above `CScriptCheck::operator()` seemingly doing allocations.
π¬ mzumsande commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2564016601)
> I do not agree with this change and it should not be merged.
"[A NACK needs to include a rationale why the change is not worthwhile. NACKs without accompanying reasoning may be disregarded.](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md)"
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2564016601)
> I do not agree with this change and it should not be merged.
"[A NACK needs to include a rationale why the change is not worthwhile. NACKs without accompanying reasoning may be disregarded.](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md)"
π¬ l0rinc commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2564016916)
@ismaelsadeeq, I ran your benchmarks from https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2446067820, but I only see a 7% speedup in the `TxVerbosity::SHOW_TXID` bench - while the rest were exactly the same as before.
But if it's *that* important here, we could easily get a 30% speedup as well there by deduplicating the `GetSerializeSize` calls:
```C++
static int64_t GetBlockWeight(const size_t size_no_witness, const size_t size_with_witness)
{
return size_no_witness * (WITNE
...
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2564016916)
@ismaelsadeeq, I ran your benchmarks from https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2446067820, but I only see a 7% speedup in the `TxVerbosity::SHOW_TXID` bench - while the rest were exactly the same as before.
But if it's *that* important here, we could easily get a 30% speedup as well there by deduplicating the `GetSerializeSize` calls:
```C++
static int64_t GetBlockWeight(const size_t size_no_witness, const size_t size_with_witness)
{
return size_no_witness * (WITNE
...
π€ furszy reviewed a pull request: "validation: Send correct notification during snapshot completion"
(https://github.com/bitcoin/bitcoin/pull/31556#pullrequestreview-2524378694)
Code review ACK bc43ecaf6dc
(https://github.com/bitcoin/bitcoin/pull/31556#pullrequestreview-2524378694)
Code review ACK bc43ecaf6dc
π¬ brunoerg commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898710992)
Done.
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898710992)
Done.
π¬ brunoerg commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#issuecomment-2564026547)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898539802
(https://github.com/bitcoin/bitcoin/pull/31570#issuecomment-2564026547)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898539802
π brunoerg opened a pull request: "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`"
(https://github.com/bitcoin/bitcoin/pull/31577)
`DecodeBase58` and `DecodeBase58Check` are fuzzed with a fixed value for `max_ret_len` (100). However, different values are used for this parameter depending on the usage (see key_io and unit tests).
(https://github.com/bitcoin/bitcoin/pull/31577)
`DecodeBase58` and `DecodeBase58Check` are fuzzed with a fixed value for `max_ret_len` (100). However, different values are used for this parameter depending on the usage (see key_io and unit tests).
π¬ l0rinc commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564034989)
Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746/files instead?
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564034989)
Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746/files instead?
π¬ brunoerg commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564037774)
> Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746 instead?
Feel free to cherry-pick this change there, then I close this one.
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564037774)
> Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746 instead?
Feel free to cherry-pick this change there, then I close this one.