Cache m_cached_finished_ibd where SetTip is called. #32885

1. 
   pstratem commented at 9:29 am on July 6, 2025: contributor

   As IsInitialBlockDownload latches to false only once the Tip is sufficiently advanced there is no need to check the Tip everytime IsIBD is called.

   By caching this in advance we can avoid extra work and more importantly a lock.

2. 
   DrahtBot commented at 9:29 am on July 6, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32885.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29640 (Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties) by sr-gi)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label CI failed on Jul 6, 2025
4. pstratem force-pushed on Jul 6, 2025
5. pstratem force-pushed on Jul 6, 2025
6. pstratem force-pushed on Jul 6, 2025
7. pstratem force-pushed on Jul 6, 2025
8. DrahtBot removed the label CI failed on Jul 7, 2025
9. pstratem force-pushed on Jul 7, 2025
10. pstratem force-pushed on Jul 7, 2025
11. DrahtBot added the label CI failed on Jul 7, 2025
12. 
    DrahtBot commented at 1:58 am on July 7, 2025: contributor

    🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/runs/45440913265 LLM reason (✨ experimental): The CI failure is caused by compilation errors due to missing mutex lock assertions in validation.cpp.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

13. Cache m_cached_finished_ibd where SetTip is called.

    As IsInitialBlockDownload latches to false only once the Tip is sufficiently
    advanced there is no need to check the Tip everytime IsIBD is called.
    
    By caching this in advance we can avoid extra work and more importantly a lock.

    2e3fefbaaa
14. pstratem force-pushed on Jul 7, 2025
15. pstratem renamed this:
    Cache m_cached_finished_ibd in ActivateBestChain.
    Cache m_cached_finished_ibd where SetTip is called.
    on Jul 7, 2025
16. DrahtBot renamed this:
    Cache m_cached_finished_ibd where SetTip is called.
    Cache m_cached_finished_ibd where SetTip is called.
    on Jul 7, 2025
17. DrahtBot removed the label CI failed on Jul 7, 2025
18. 
    stickies-v commented at 5:17 pm on July 7, 2025: contributor

    Conceptually not a bad idea to cache and lock less, but imo this makes the code more brittle (and harder to understand), e.g. if any tip updates happen without the cache being updated separately.

    Do you have any data as to the actual performance improvements from this PR?

19. 
    furszy commented at 6:10 pm on July 7, 2025: member

    I think something like this, if properly implemented (I haven’t thought much about the code yet), would reduce the GUI freezes during IBD in a noticeable manner.

Contributors
pstratem DrahtBot stickies-v furszy