This adds Filterer and FiltererV2 interfaces to rpcserver, which
decouples rpcserver from the implementation to retrieve committed
filters and allows mocked versions to be provided for testing.
This modifies the dial timeout test to make use of a channel for the
cancellation signal synchronization instead of a sleep to prevent false
positive test failures in slower environments such as CI servers.
Part of adding nodes to the block index involves updating the current
chain tip in order to be able to efficiently keep track of all known
chain tips.
Since there can be multiple tips at a given height, the current code
uses a slice for each height to track them. However, this is wasteful
since it is extremely rare for there to be more than one chain tip at
the same height.
It is particularly notable at startup since the block index has to load
up hundreds of thousands of nodes.
Given that, this modifies the chain tip tracking logic to avoid creating
a slice unless it is actually needed by making use of an entry with a
static field for the normal case and a dynamic slice for the case when
more than 1 tip for the height actually exists.
It also keeps a running sum of the total number of known chips for use
as a hint to further reduce allocations when serving up related data and
updates the chain tip tests to ensure 100% branch coverage is maintained.
The end result is that this reduces the total number of allocs at
startup by around 1%, which results in faster startup, and also reduces
the overall memory usage by a few MB for nodes that have a long running
history of side chains.
Similar to the recent changes made to txscript logging, this updates the
peer code to take advantage of the ability to determine the logging
level when constructing expensive log strings in order to avoid the
allocations caused by closures and removes the no longer necessary code
related to creating them.
After recent optimizations, the current next biggest offender of more
allocations than would be expected revealed by profiling is due to the
trace logging closures for the scripting engine execution.
Once upon a time, there was no way to check the current logging level
with the logging infrastructure at the time and thus a logging closure
was used to defer the fairly expensive construction of the trace logging
information until it was actually invoked (meaning tracing is enabled).
However, those closures come at the cost of allocations, and since
script execution is something that happens non-stop during normal
operation, those allocations really add up, as the profiling shows.
As some point, the logging infrastructure was changed out, and it is now
possible to determine the logging level, so this updates the code to
take advantage of that and avoid the closures while still only
performing the fairly expensive construction of trace logging
information when tracing is enabled. In other words, with this change
there is zero cost (other than the conditional check, of course) when
tracing is not enabled.
Finally, this also removes the no longer necessary code related to
creating the logging closures.
Profiling shows that approximately 0.67% of the total allocations are
coming from DropN even though it should realistically be zero.
The cause is that DropN returns an error if it is called with 0 and the
code that drops the alternate data stack in between scripts calls it
with the depth of the alternate data stack, which is almost always zero.
The call has no observable effect on the result of the script execution
in that case since the error is intentionally ignored, there is nothing
to be dropped, and is therefore correct semantically, however, it is
wasteful to generate and throw away the error.
There really is no good reason for the internal stack implementation to
return an error from DropN when called with 0 since it equivalent to a
NOP.
I have done a careful analysis of all call sites to ensure the change
will break any observable semantics.
Thus, this modifies DropN to remove the unnecessary error condition and
updates the associated internal stack implementation test accordingly.
This removes the internal blockchain/dbnamespace package in favor of
defining the bucket names and keys locally in the file that works with
them in the blockchain package.
An internal package for this gives the false impression that there is
some expectation that other packages in the module would be digging
around in the internals of what is intended to be an implementation
detail of the package.
It would also significantly complicates the ability to change the
database implementation.
The IDs are assigned in increasing order, so this has the net effect
of peer results returned in the order they were connected. It also
keeps the order the same across multiple calls.
This matches the behavior of the current pull request implementing
getpeerinfo in dcrwallet.
This modifies the signature cache Exists method to make use of the new
efficient equality testing methods provided by the secp256k1 types
versus indirectly checking their equality via their serializations.
The following benchmark shows a before and after comparison of a typical
signature cache exists check:
name old time/op new time/op delta
-------------------------------------------------------------------------
SigCacheExists 400ns ± 0% 62ns ± 0% -84.54% (p=0.000 n=5+5)
name old alloc/op new alloc/op delta
--------------------------------------------------------------------------
SigCacheExists 256B ± 0% 0B -100.00% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
--------------------------------------------------------------------------
SigCacheExists 4.00 ± 0% 0.00 -100.00% (p=0.008 n=5+5)
This modifies the websocket notification manager to make use of the
newer pattern that synchronizes the Run context with an internal quit
channel to ensure that any exported functions that communicate with the
goroutines can't possibly hang during shutdown.
This was previously done by storing the context in the struct (which is
generally discouraged) and protecting it by a mutex which means every
single notification had to take the mutex.
As can be seen by the diff of this commit, this approach is not only
more performant, it's also less code.
Profiling revealed that the number one cause of allocations in
blockchain is currently due to stake node pruning, which definitely
should not be the case.
Upon closer examination, the culprit is that every single time the stake
nodes are pruned the entire ancestry of nodes prior to the prune height,
including those that have already been pruned, are appended to the list
of nodes to prune. In practice, this amounts to hundreds of thousands
of nodes and thus the slice is resized multiple times (likely around
18 times per run given the current runtime semantics).
This remedies that by ensuring that only the nodes that actually need to
be pruned are added to the relevant slice and also preallocating space
per a hint that will handle the number of nodes expected to be pruned
with a 99% confidence level to further reduce allocations when more than
a single node needs to be pruned.
It also renames the slice to reflect what it actually does.
This modifies the pruning logic to set the pruning interval to the
target block time for the given chain parameters instead of hard coding
a constant with the mainnet value.
This modifies the main startup logic to make use of the fact that the
server now has its lifecycle tied to the same context that is cancelled
when a shutdown signal is received.
The end result is that the separate goroutine it was running in before
and associated synchronization logic is no longer necessary.
This makes the fees package an internal package such that it will no
longer be an exported module. The only place its functionality is
really needed is for the internal implementation of dcrd itself and thus
having an unnecessary exported module significantly increases the
maintenance burden.
This is part of the overall effort to reduce the total number of modules
and eventually get to the point that it will be possible to follow
semver for the root module.
It is worth noting that there are a few constants, which will be listed
below, that were exported from this module that callers might have
previously relied upon. However, due to previous discussions about the
goal of removing the module, a code search has revealed that there are
no known callers that rely on them.
The aforementioned constants are:
- DefaultMaxBucketFeeMultiplier
- DefaultMaxConfirmations
- DefaultFeeRateStep
Overview of the major changes:
- Move the following files from fees -> internal/fees:
- README.md
- cmd/dumpfeedb/dumpfeedb.go
- doc.go
- estimator.go
- log.go
- Remove fees/go.mod and fees/go.sum
- Make the README.md and doc.go files match the new reality
- Update all import paths in the repository accordingly
- Remove the dependency from the root go.mod
- Run go mod tidy on all modules
This constructs the RPC API semantic version string from the individual
major, minor, and path constants versus requiring updates in both
places. This is something contributors have previously forgotten to
update, so it makes sense to avoid the need to do it altogether.
This moves rpcserver from the main module to a standalone internal
package.
Overview of the major changes:
- Move the following files from the main module -> internal/rpcserver:
- rpcserver.go
- rpcserver_test.go
- rpcserverhandlers_test.go
- rpcserverhelp.go
- rpcserverhelp_test.go
- rpcwebsocket.go
- Update the README.md and doc.go files
- Update all import paths in the repository accordingly
- Update `testDataPath` in rpcserverhandlers_test.go
- Update all `rpcsLog` references to `log`
- Define the following convenience functions and vars in rpcserver.go:
- `normalizeAddress`
- `directionString`
- `zeroHash`
This moves `genCertPair` from rpcserver.go to server.go. `genCertPair`
is only used by server.go and is being moved there in anticipation of
rpcserver being moved to a separate internal package.
This is part of the effort to move rpcserver into a separate internal
package.
This adds the following exported RPCServer functions, which delegate to
the corresponding underlying rpcwebsocket notification functions:
- NotifyWork
- NotifyStakeDifficulty
- NotifyNewTickets
- NotifyBlockConnected
- NotifySpentAndMissedTickets
- NotifyBlockDisconnected
- NotifyReorganization
- NotifyWinningTickets
This is done so that blockmanager.go and server.go can still access
these functions after rpcserver is moved to a separate internal package.
This is part of the effort to move rpcserver into a separate internal
package.
This exports `rpcServer`, `rpcserverConfig`, and `newRPCServer` from
rpcserver.go and updates all references accordingly.
This is part of the effort to move rpcserver into a separate internal
package.
This removes the dependency that rpcserver has on the PeerNotifier
interface defined in blockmanager.go. It accomplishes this by removing
the call to AnnounceNewTransactions and instead calling the already
defined ConnMgr.RelayTransactions interface method, followed by directly
calling NotifyNewTransactions to notify websocket clients of the newly
accepted transactions.
This is part of the effort to move rpcserver into a separate internal
package.
This removes the dependency that rpcserver has on the
supportedSubsystems and parseAndSetDebugLevels functions defined in
config.go. It accomplishes this by creating and implementing a
rpcserver.LogManager interface.
This is part of the effort to move rpcserver into a separate internal
package.
This removes the dependency that rpcserver has on the const
maxProtocolVersion and var userAgentVersion defined in server.go. It
accomplishes this by passing these values through the rpcserverConfig
struct.
This is part of the effort to move rpcserver into a separate internal
package.
This removes the dependency that rpcserver has on the global config in
the main module. It accomplishes this by passing the necessary config
information through the rpcserverConfig struct. Additionally, this adds
a Lookup method to the rpcserver.ConnManager interface.
This is part of the effort to move rpcserver into a separate internal
package.
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.
This does the minimum work necessary to refactor the CPU miner code into
its own internal package. The idea is that separating this code into
its own package will improve its testability and ultimately be useful to
other parts of the codebase such as the various tests which currently
effectively have their own stripped-down versions of this code.
The API will certainly need some additional cleanup and changes to make
it more usable outside of the specific circumstances it was originally
designed to support (namely the generate RPC), however it is better to
do that in future commits in order to keep the changeset as small as
possible during this refactor.
Overview of the major changes:
- Create the new package
- Move internal/mining/cpuminer.go -> internal/mining/cpuminer/cpuminer.go
- Update mining logging to use the new cpuminer package logger
- Rename CPUMinerConfig to Config (so it's now cpuminer.Config)
- Rename NewCPUMiner to New (so it's now cpuminer.New)
- Add exported BestSnapshot method to mining.BlkTmplGenerator
- Add exported TxSource method to mining.BlkTmplGenerator
- Update all references to the cpuminer to use the package
- Add a skeleton README.md
- Add a skeleton doc.go
This makes the mining package an internal package such that it will no
longer be an exported module. The only place its functionality is
really needed is for the internal implementation of dcrd itself and thus
having an unnecessary exported module significantly increases the
maintenance burden.
This is part of the overall effort to reduce the total number of modules
and eventually get to the point it will be possible to follow semver for
the root module.
It is worth noting that there are a few constants, which will be listed
below, that were exported from this module that callers might have
previously relied upon. However, due to previous discussions about the
goal of removing the module, a code search has revealed that there are
no known callers that still rely on them.
The aforementioned constants are:
- UnminedHeight
- MinHighPriority
Overview of the major changes:
- Move the following files from mining -> internal/mining:
- README.md
- cpuminer.go
- doc.go
- miningerror.go -> error.go
- log.go
- mining.go
- mining_test.go
- policy.go
- policy_test.go
- Remove mining/go.mod and mining/go.sum
- Make the README.md and doc.go files match the new reality
- Update all import paths in the repository accordingly
- Remove the dependency from the root go.mod
- Run go mod tidy on all modules
This makes the mempool an internal package such that it will no longer
be an exported module. The only place its functionality is really
needed is for the internal implementation of dcrd itself and thus having
an unnecessary exported module significantly increases the maintenance
burden.
This is part of the overall effort to reduce the total number of modules
and eventually get to the point it will be possible to follow semver for
the root module.
It is worth noting that there are a few constants, which will be listed
below, that were exported from this module that callers might have
previously relied upon. However, due to previous discussions about the
goal of removing the module, a code search has revealed that there are
no known callers that still rely on them.
The aforementioned constants are:
- MaxStandardTxSize
- DefaultMinRelayTxFee
- BaseStandardVerifyFlags
Overview of the major changes:
- Move the following files from mempool -> internal/mempool:
- README.md
- doc.go
- error.go
- log.go
- mempool.go
- mempool_test.go
- policy.go
- policy_test.go
- Remove mempool/go.mod and mempool/go.sum
- Make the README.md and doc.go files match the new reality
- Update all import paths in the repository accordingly
- Remove the dependency from the root go.mod
- Run go mod tidy on all modules
