Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpuminer: Rework to use bg template generator. #2277

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Jul 20, 2020

This requires #2276.

This significantly reworks the CPU miner to make use of the background block template generator that was introduced in the previous release as well as to convert its lifecycle to use the expected pattern for running subsystems based on contexts.

The switch to using the background block template generator provides all the benefits it offers to miners such as:

  • Intelligent vote propagation handling
  • Improved handling of chain reorganizations necessary when the current tip is unable to obtain enough votes
  • Current state synchronization
  • Near elimination of stale templates when new blocks and votes are received

This is particularly important in testing scenarios, such as when using the simulation network, since the difficulty is so low that the code this is replacing is often able to mine blocks so fast it ends up mining multiple alternative blocks while waiting for the votes to show up which makes testing with it more difficult.

This is no longer an issue when requesting template updates since the background block template generator has logic to provide the votes a chance to arrive before building and notifying the template.

Another modification this makes is to also make use of template notifications when mining blocks mined via the discrete mining mode and to only count blocks if they successfully extend the main chain. This also significantly helps in testing scenarios since it means the testing code and/or testers using the simulation test network can rely on the requested number of blocks actually being added to the main chain even if more need to be generated to make that happen.

Along similar lines, one area this change does not address, but does pave the way to support and would be useful in the future for the simulation test network is to add some additional logic to provide ticket purchases a chance to arrive when prior to stake validation height, since that is another area that often complicates testing.

In terms of the lifecycle changes, this replaces the Start, Stop, and Wait methods with a single method named Run and arranges for it to block until the provided context is cancelled. This is more flexible for the caller since it can easily turn blocking code into async code while the reverse is not true.

The new Run method waits for all goroutines that it starts to shutdown before returning to help ensure an orderly shutdown.

In addition, the semantics of Run are different from Start/Stop in that the Start method launched the default number of mining worker goroutines whereas Run only starts the CPU miner with the main infrastructure goroutines and zero workers, meaning it will be idle until explicitly requested to mine by the caller setting the number of workers to use.

Since there are exported methods that send messages to the CPU miner infrastructure goroutines via a channel and those goroutines are stopped during the shutdown process, all sends select across both the channel in question as well as a quit channel which is closed when the context is canceled. This ensures callers can't end up blocking on send to a goroutine that is no longer running without needing additional mutexes.

As part of the conversion, it improves a lot of the comments to better describe the expected behavior and further integrates the context to support cancellation where it previously was not supported.

Finally, all callers are updated to use the new API and the server is modified to take advantage of the new lifecycle semantics by taking ownership of the CPU miner code directly in its Run method where it more logically makes sense and only creating the template generate and CPU miner infrastructure if needed based on the configuration options.

@davecgh davecgh added this to the 1.6.0 milestone Jul 20, 2020
@davecgh davecgh force-pushed the cpuminer_rework_for_bgtplgen branch 4 times, most recently from 792e3b1 to fb4fd00 Compare July 20, 2020 10:38
@davecgh davecgh force-pushed the cpuminer_rework_for_bgtplgen branch from 69f26c0 to 2bd4ae7 Compare July 23, 2020 01:34
internal/mining/cpuminer/cpuminer.go Outdated Show resolved Hide resolved
internal/mining/cpuminer/cpuminer.go Outdated Show resolved Hide resolved
This significantly reworks the CPU miner to make use of the background
block template generator that was introduced in the previous release as
well as to convert its lifecycle to use the expected pattern for running
subsystems based on contexts.

The switch to using the background block template generator provides all
the benefits it offers to miners such as:

- Intelligent vote propagation handling
- Improved handling of chain reorganizations necessary when the current
  tip is unable to obtain enough votes
- Current state synchronization
- Near elimination of stale templates when new blocks and votes are received

This is particularly important in testing scenarios, such as when using
the simulation network, since the difficulty is so low that the code
this is replacing is often able to mine blocks so fast it ends up mining
multiple alternative blocks while waiting for the votes to show up which
makes testing with it more difficult.

This is no longer an issue when requesting template updates since the
background block template generator has logic to provide the votes a
chance to arrive before building and notifying the template.

Another modification this makes is to also make use of template
notifications when mining blocks mined via the discrete mining mode and
to only count blocks if they successfully extend the main chain.  This
also significantly helps in testing scenarios since it means the testing
code and/or testers using the simulation test network can rely on the
requested number of blocks actually being added to the main chain even
if more need to be generated to make that happen.

Along similar lines, one area this change does not address, but does
pave the way to support and would be useful in the future for the
simulation test network is to add some additional logic to provide
ticket purchases a chance to arrive when prior to stake validation
height, since that is another area that often complicates testing.

In terms of the lifecycle changes, this replaces the Start, Stop, and
Wait methods with a single method named Run and arranges for it to block
until the provided context is cancelled.  This is more flexible for the
caller since it can easily turn blocking code into async code while the
reverse is not true.

The new Run method waits for all goroutines that it starts to shutdown
before returning to help ensure an orderly shutdown.

In addition, the semantics of Run are different from Start/Stop in that
the Start method launched the default number of mining worker goroutines
whereas Run only starts the CPU miner with the main infrastructure
goroutines and zero workers, meaning it will be idle until explicitly
requested to mine by the caller setting the number of workers to use.

Since there are exported methods that send messages to the CPU miner
infrastructure goroutines via a channel and those goroutines are stopped
during the shutdown process, all sends select across both the channel in
question as well as a quit channel which is closed when the context is
canceled.  This ensures callers can't end up blocking on send to a
goroutine that is no longer running without needing additional mutexes.

As part of the conversion, it improves a lot of the comments to better
describe the expected behavior and further integrates the context to
support cancellation where it previously was not supported.

Finally, all callers are updated to use the new API and the server is
modified to take advantage of the new lifecycle semantics by taking
ownership of the CPU miner code directly in its Run method where it more
logically makes sense and only creating the template generate and CPU
miner infrastructure if needed based on the configuration options.
@davecgh davecgh force-pushed the cpuminer_rework_for_bgtplgen branch from 2bd4ae7 to a6feca2 Compare July 23, 2020 17:36
@davecgh davecgh merged commit a6feca2 into decred:master Jul 24, 2020
@davecgh davecgh deleted the cpuminer_rework_for_bgtplgen branch July 24, 2020 23:29
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.

None yet

4 participants