Used thread local buffers to tame GC pressure caused by piece validation - #184
Used thread local buffers to tame GC pressure caused by piece validation#184bwzhou wants to merge 98 commits into
Conversation
…sage.parse Order of the parameters to the HTTPAnnounceRequestMessage constructor was incorrect in the parse() method. Thanks to Dan Oxlade for reporting the problem. Closes mpetazzoni#48. Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Allow trackerless torrent files mpetazzoni#2 Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
…h InboundThread and OutboundThread to a common ExchangeThread superclass.
… download/upload rate limiting algorithm
Add ability to set upload/download rate limits. Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Fix read when message size wasn't read in one step Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
BDecode support for negative integers Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Fixes bug with selecting the announce client A previous change introduced a bug in the selection of the announce tier and client. Closes mpetazzoni#51. Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Use maven-shade-plugin to produce executable jar Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Update pom for compatibility with the maven-release-plugin Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Clean up javadoc problems Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Shell script updates for wider platform compatibility Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Move entry-point main methods to separate package Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Fixed bitfield payload in bitfield message
Resolves: mpetazzoni#144 See also: mpetazzoni#146
Remove dependency on Apache comons-codec
…eerexchange Allow immedidate shutdown of peerexchange by notifying out-going thread
This change prevents the modification of a storage file in case the file is already complete and all pieces are available.
…zoni#169 Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
…ng-file-only-if-required Change the length of an existing storage file only if required
Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
In preparation of incoming 1.5 release, with the cli/core split. Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
The old implementation of byteArrayToHexString drops the byte array’s leading zeros.
…tring Fix byteArrayToHexString and add tests
mpetazzoni
left a comment
mpetazzoni
left a comment
There was a problem hiding this comment.
Thanks! Interesting changes and nice profiling. I've never bothered to do any on this library so this is always helpful. I made a few comments.
| @@ -412,6 +412,14 @@ public static byte[] hash(byte[] data) throws NoSuchAlgorithmException { | |||
| return crypt.digest(); | |||
There was a problem hiding this comment.
Can you call the other version of hash() from here instead?
public static byte[] hash(byte[] data) throws NoSuchAlgorithmException {
return hash(data, 0, data.length);
}| * the piece boundary. | ||
| * @throws IOException If the read can't be completed (I/O error, or EOF | ||
| * reached, which can happen if the piece is not complete). | ||
| */ |
There was a problem hiding this comment.
Can't you call this method from the other _read() method with the newly allocated ByteBuffer?
| " overrun (" + offset + " + " + length + " > " + | ||
| this.length + ") !"); | ||
| } | ||
|
|
| private static final ThreadLocal<ByteBuffer> validateByteBuffer = new ThreadLocal<ByteBuffer>() { | ||
| @Override | ||
| protected ByteBuffer initialValue() { | ||
| return ByteBuffer.allocate(Torrent.DEFAULT_PIECE_LENGTH); |
There was a problem hiding this comment.
You could even be generous and use 4k here (default piece length is 512 bytes I think). A lot of torrents of large size will usually use larger pieces, so if you don't give a little more room here you'll rarely actually benefit from your optimization.
|
Thanks for the comments. I updated the PR to address them and also included an optimization for piece recording that follows the similar idea. |
mpetazzoni
left a comment
mpetazzoni
left a comment
There was a problem hiding this comment.
12MB per peer thread is probably too much when you can potentially have dozens, if not hundreds, of connected peers.
|
Torrent.DEFAULT_PIECE_LENGTH is 512k, so I thought your previous comment meant 4M instead of 4k. Would you suggest a number for the sizes of the 3 buffers? |
|
Correct me if I am wrong: previously, we allocate 1x the memory of a piece in recording, followed by 2x the memory of a piece in validation; but now, we pre-allocate 3x the memory of a piece in each thread. So on average, we should use 50% more memory than before if the buffer size equals to the piece size. |
|
"related" -> google/guava#2112 |
|
Also, I think that complete correction of this problem is by having an event loop instead of an unbounded number of threads. |
19 participants
Piece validation allocates 2x memory for each piece received, which could cause frequent young gen GC's and premature old gen promotions. This memory pressure can be alleviated by using pre-allocated thread local buffers as the scratch pad for the validation. A byte array and a ByteBuffer are allocated for each PeerExchange.IncomingThread and released when the thread exits.
Before:
After:
