Limit mem usage on torrent creation

[zanella]

Following the footsteps of #184 and #35 , the torrent creation uses a lot of memory needlessly as well, below are the screenshots and test code, the mem usage peak dropped ~20x.

Not too proud of the code, I think the Guava PR mentioned in #184 is a better approach.

public class App {
    public static void main( String[] args ) {
    try {           
        for (int i = 0; i < 25; i++) {
            ////
            final Torrent bigFileTorrent = Torrent.create(new File("/tmp/BIG"), URI.create("http://some.where:1234"), "test");

            if (!"D28305C0608826F8189C82DCF23B99564D71A075".equals(bigFileTorrent.getHexInfoHash())) { throw new Exception("mismatch"); }
        }
    } catch (Exception e) {
        System.out.println(e.getMessage());
    }
}
}

=================================================================================

$ ls -lash /tmp/BIG 

1,4G -rw-rw-r-- 1 rzanella rzanella 1,4G Nov 14 19:28 /tmp/BIG

=================================================================================

BEFORE:

=================================================================================

AFTER:

[gzsombor]

I wont leake this GenericObjectPool to the caller, it would be simpler to have a "get" and "release" method.

[zanella]

Indeed, could just wrap/hide borrowObject() and returnObject()..., guess it's cleaner like this, I'll update the last commit

[gzsombor]

Why the declared Exception ? It looks very suspicious ...

[zanella]

It's due to the Commons pool borrowObject() signature

[mpetazzoni]

Keep constructor above methods

[mpetazzoni]

Javadoc

[zanella]

Will do, so I guess it means the inclusion of Common Pool is the way to go ? If it's I'll remove the 2 previous commits, no need to pollute the commit history with a dead alternative

[mpetazzoni]

Looks like this can be static

[mpetazzoni]

Since it doesn't look like you're doing much with that checked Exception is the upper layers anyway, it might be simpler to catch it here and propagate it as a RuntimeException instead (similar to what Throwables.propagate() would do.

[zanella]

Seems like a better fit indeed, since where it's used right not doesn't even care about the exception, no need to pollute the code with it. Didn't know this Guava class, thanks :-)

[mpetazzoni]

You have an asymmetry in the responsibility of dealing with the buffer pool that's concerning here: this class doesn't seem responsible for acquiring the buffer from the pool, but is responsible for returning it. That's usually a code smell.

[zanella]

Indeed, the ByteBufferPool is a good solution for the problem described at #35 as well, not thrashing the memory with short-lived objects, which makes it a good thing to not put it inside the hashing loop(and have it as a outer class), but unless I return a tuple as the Callable's type class CallableChunkHasher implements Callable<Pair<String, ByteBuffer> I don't see a way of fixing this assymetry, quite frankly I don't know what's better, of if there's a better way that I haven't seen...

I could put the buffer logic inside CallableChunkHasher, and change it a bit to have an init() and close(), but, then again, the borrowing of the buffers would still be done in the hashing loop...

Sorry for the long reply :-|

[mpetazzoni]

Fix the typo on pieceLength (everywhere)

[zanella]

haha, ok, didn't even notice, though it will generate noise around the patch itself, since the typos are unrelated

[zanella]

@mpetazzoni redid the patch moving the chunkHasher to it's own class, and gave up on using the Common Pool for simplicity and size. Maybe it's a better idea to merge #197 first.