Skip to content

Conversation

@codablock
Copy link

This was always quite spammy and so far never useful in debugging.

@codablock codablock added this to the 16 milestone Mar 21, 2020
This was always quite spammy and so far never useful in debugging.
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@codablock codablock merged commit f2ece10 into dashpay:develop Mar 24, 2020
@codablock codablock deleted the pr_wakeup_log branch March 24, 2020 09:55
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Feb 8, 2022
This was always quite spammy and so far never useful in debugging.
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 13, 2022
0bd882b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner)

Pull request description:

  The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member:

  https://github.com/bitcoin/bitcoin/blob/83daf47898f8a79cb20d20316c64becd564cf54c/src/validation.cpp#L2973-L2976

  ~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~

  ~~This is related to bitcoin#19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR dashpay#3370, commit 75f51f2) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7), probably also because the compilers didn't support them properly earlier.~~

  At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main.

ACKs for top commit:
  Zero-1729:
    ACK 0bd882b
  promag:
    Code review ACK 0bd882b.
  hebasto:
    ACK 0bd882b

Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 25, 2024
0bd882b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner)

Pull request description:

  The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member:

  https://github.com/bitcoin/bitcoin/blob/83daf47898f8a79cb20d20316c64becd564cf54c/src/validation.cpp#L2973-L2976

  ~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~

  ~~This is related to bitcoin#19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR dashpay#3370, commit 75f51f2) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7), probably also because the compilers didn't support them properly earlier.~~

  At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main.

ACKs for top commit:
  Zero-1729:
    ACK 0bd882b
  promag:
    Code review ACK 0bd882b.
  hebasto:
    ACK 0bd882b

Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants