bodewig commented on issue #79: Pull/78 COMPRESS-485 + Substituting 'synchronized' with faster and fully thread-safe collections 'ConcurrentLinkedDeque' and iterators. URL: https://github.com/apache/commons-compress/pull/79#issuecomment-519438687 Thanks @Tibor17 Actually I've more or less been thinking out loud and not raising issues. I was trying to figure out whether the `synchronized` usage was giving the API users - or our code - any extra guarantees that the non-blocking collection code didn't. Keep in mind that I'm not the author of the original code either. I am the one who added the `synchronized` around the iteration over `streams` - but completely overlooked to the iteration over `futures` before that. Doesn't sound as if I was the most qualified person to comment ;-) What I meant with the first part was that if you added new threads once `writeTo` was underway then whatever your new threads contributed would not be part of the result - while it now is undefined. Looking at the current code in master I see I've been wrong as `writeTo` does quite a few things before entering the synchronized block and there is enough leeway anyway. No, I don't think we need to exclude the methods from each other. The class has a very clear usage pattern of two distinct phases: 1. add all the things you want to add 2. call `writeTo` and it should be clear that the result of calling `writeTo` before you are done with the first phase is a dubious idea. In particular as the javadocs of `writeTo` state it will shut down the executor. I am aware of the iteration guarantees of `ConcurrentLinkedDeque`. Back when Kristian added the parallel zip support Commons Compress' baseline has been Java 5 (we bumped that to 6 with Compress 1.12 and 7 in Compress 1.13, which is where we are today). If it had been Java 7 back then, then I'm sure Kristian would have used non-blocking collections instead.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
