Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

Use xxHash instead of SHA-256 on incoming transaction hashes cache #1326

Conversation

viossat
Copy link
Contributor

@viossat viossat commented Feb 7, 2019

Description

Use xxHash instead of SHA-256 on incoming transaction hashes cache to reduce CPU load and allows higher TPS throughput.

Fixes #1171

Type of change

  • Enhancement (a non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@muXxer
Copy link
Contributor

muXxer commented Feb 7, 2019

Nice one! :)

@viossat viossat force-pushed the improvement/incoming-tx-hashes-cache-xxhash branch from 03b53c4 to f06ba33 Compare February 7, 2019 00:53
@GalRogozinski
Copy link
Contributor

We need to do some tests to see how this improves the cpu load

If @viossat or @muXxer did their own tests I will be happy if they upload the results here

@muXxer
Copy link
Contributor

muXxer commented Feb 7, 2019

I only made some benchmarks in golang, sorry. But here are my results for golang:

Running tool: /usr/local/go/bin/go test -benchmem -run=^$ -bench . -coverprofile=/tmp/vscode-gohKcwB0/go-code-cover

goos: linux
goarch: amd64
BenchmarkMD5-4               10000000           154 ns/op           0 B/op           0 allocs/op
BenchmarkSHA256-4            10000000           229 ns/op           0 B/op           0 allocs/op
BenchmarkHighwayHash-4       30000000          41.3 ns/op           0 B/op           0 allocs/op
BenchmarkXXHash-4           100000000          10.4 ns/op           0 B/op           0 allocs/op
PASS

229ns vs 10.4ns on my machine

This is called more often than Curl, depending on the amount of neighbors you have.
During a spam event, this should be much worse than at current network rate.
Also when a node is syncing and requesting a lot of tx from the neighbors, this should affect the CPU load.

@GalRogozinski
Copy link
Contributor

After 1.6.1 will be out we will check if this should be merged

@viossat
Copy link
Contributor Author

viossat commented Feb 7, 2019

Here a quick benchmark between SHA-256 and two xxHash libs (LZ4 and Zero-Allocation-Hashing) hashing the same million of 1653-byte arrays. The PR implements the last one. Please note the net.jpountz.xxhash lib seems the fastest below 1 million.

hash function time (ms)
java.security.MessageDigest ("SHA-256") 7204
net.jpountz.xxhash.XXHash32 260
net.jpountz.xxhash.XXHash64 180
net.openhft.hashing.LongHashFunction 163

That means a 5-neighbors node at 100 TPS would only save about 1 second every 5 minutes.

@muXxer
Copy link
Contributor

muXxer commented Feb 7, 2019

At higher rate this makes more impact

>>> (7.204/1000000.0)*5000*60*5
10.806000000000001
>>> (0.163/1000000.0)*5000*60*5
0.24450000000000002

Thats already a difference of 10.56s every 5 Minutes with 1000 TPS and 5 neighbors (28.4 % CPU load).
:wen: 1000 TPS eoy? :)

@GalRogozinski GalRogozinski added C-Network E-Performance Epic - Improving throughput of IRI labels Feb 7, 2019
Copy link
Contributor

@alon-e alon-e left a comment

Choose a reason for hiding this comment

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

looks good!

@GalRogozinski
Copy link
Contributor

I want to finish #1328 first so that we can track the overall improvement

Copy link
Contributor

@karimodm karimodm left a comment

Choose a reason for hiding this comment

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

gg

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C-Network E-Performance Epic - Improving throughput of IRI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use faster Hash function in "getBytesDigest"
5 participants