parallelgzip
parallelgzip copied to clipboard
shevek
Do you want to use put instead of add?
This was a great find, nice work.
Anyway, I wanted to point this out, not sure if you care. Pretty sure you want to block here.
https://github.com/shevek/parallelgzip/blob/master/src/main/java/org/anarres/parallelgzip/ParallelGZIPOutputStream.java#L233
@rdifalco This is master thread only and emitQueue is an ArrayBlockingQueue. Can you clarify a bit?
@simlu maybe I didn't read closely enough, but I figured since you are using an ArrayBlockingQueue that you'd want to block if the queue became full. Is it unbounded? I would have expected it to use put instead of add.
Ah, now I see. I was pretty sure we deal with that in logic, but I don't see it right now.
Anyways, still a valuable change. Waiting rather than crashing is definitely preferred.
This seems legit and should be changed @shevek. I'll create a pr here shortly.
The immediately preceding call to emitUntil() should ensure that an add() won't crash. It only returns when there's at least one space in the queue. Also, the testing has been sufficiently exhaustive on high-core-count machines that I'd be surprised if it had never shown up.
That said, this is still a good change, as it makes the software more robust, and still results in exactly one raising of the lock in ABQ, so I'll definitely take it. Good find, thank you.
