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

mempool: Optimize orphan map limiting. #1117

Merged

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Mar 4, 2018

This PR contains the following upstream commits:


This optimizes the way in which the mempool oprhan map is limited in the
same way the server block manager maps were previously optimized.

Previously the code would read a cryptographically random value large
enough to construct a hash, find the first entry larger than that value,
and evict it.

That approach is quite inefficient and could easily become a
bottleneck when processing transactions due to the need to read from a
source such as /dev/urandom and all of the subsequent hash comparisons.

Luckily, strong cryptographic randomness is not needed here. The primary
intent of limiting the maps is to control memory usage with a secondary
concern of making it difficult for adversaries to force eviction of
specific entries.

Consequently, this changes the code to make use of the pseudorandom
iteration order of Go's maps along with the preimage resistance of the
hashing function to provide the desired functionality. It has
previously been discussed that the specific pseudorandom iteration order
is not guaranteed by the Go spec even though in practice that is how it
is implemented. This is not a concern however because even if the
specific compiler doesn't implement that, the preimage resistance of the
hashing function alone is enough.

The following is a before and after comparison of the function for both
speed and memory allocations:

benchmark                    old ns/op     new ns/op     delta
----------------------------------------------------------------
BenchmarkLimitNumOrphans     3727          243           -93.48%

benchmark                    old allocs    new allocs    delta
-----------------------------------------------------------------
BenchmarkLimitNumOrphans     4             0             -100.00%

davecgh and others added 2 commits October 23, 2016 21:14
This optimizes the way in which the mempool oprhan map is limited in the
same way the server block manager maps were previously optimized.

Previously the code would read a cryptographically random value large
enough to construct a hash, find the first entry larger than that value,
and evict it.

That approach is quite inefficient and could easily become a
bottleneck when processing transactions due to the need to read from a
source such as /dev/urandom and all of the subsequent hash comparisons.

Luckily, strong cryptographic randomness is not needed here. The primary
intent of limiting the maps is to control memory usage with a secondary
concern of making it difficult for adversaries to force eviction of
specific entries.

Consequently, this changes the code to make use of the pseudorandom
iteration order of Go's maps along with the preimage resistance of the
hashing function to provide the desired functionality.  It has
previously been discussed that the specific pseudorandom iteration order
is not guaranteed by the Go spec even though in practice that is how it
is implemented.  This is not a concern however because even if the
specific compiler doesn't implement that, the preimage resistance of the
hashing function alone is enough.

The following is a before and after comparison of the function for both
speed and memory allocations:

benchmark                    old ns/op     new ns/op     delta
----------------------------------------------------------------
BenchmarkLimitNumOrphans     3727          243           -93.48%

benchmark                    old allocs    new allocs    delta
-----------------------------------------------------------------
BenchmarkLimitNumOrphans     4             0             -100.00%
This optimizes the way in which the mempool oprhan map is limited in the
same way the server block manager maps were previously optimized.

Previously the code would read a cryptographically random value large
enough to construct a hash, find the first entry larger than that value,
and evict it.

That approach is quite inefficient and could easily become a
bottleneck when processing transactions due to the need to read from a
source such as /dev/urandom and all of the subsequent hash comparisons.

Luckily, strong cryptographic randomness is not needed here. The primary
intent of limiting the maps is to control memory usage with a secondary
concern of making it difficult for adversaries to force eviction of
specific entries.

Consequently, this changes the code to make use of the pseudorandom
iteration order of Go's maps along with the preimage resistance of the
hashing function to provide the desired functionality.  It has
previously been discussed that the specific pseudorandom iteration order
is not guaranteed by the Go spec even though in practice that is how it
is implemented.  This is not a concern however because even if the
specific compiler doesn't implement that, the preimage resistance of the
hashing function alone is enough.

The following is a before and after comparison of the function for both
speed and memory allocations:

benchmark                    old ns/op     new ns/op     delta
----------------------------------------------------------------
BenchmarkLimitNumOrphans     3727          243           -93.48%

benchmark                    old allocs    new allocs    delta
-----------------------------------------------------------------
BenchmarkLimitNumOrphans     4             0             -100.00%
@davecgh davecgh merged commit 2c14694 into decred:master Mar 4, 2018
@dnldd dnldd deleted the merge_mempool_more_efficient_orphan_limiting branch March 4, 2018 00:56
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.

2 participants