Hi,
I created another patch that replaces all timed waits but one with infinite waits and thread interruption. The advantages are that less context switches are made (mostly important for systems that use many XMPPConnections in parallel) and also reduces the delays inherent to poll. For instance, instead of waiting an average of 1 second to shut down the Smack Listeners Thread, the shutdown is near to immediate. And instead of unconditionally waiting 150 ms. before closing the socket for the threads to finalize, now the threads are joined until they finished (for at most 100 ms. each and at least nothing).
Does this help performance for single users? Yes, I made a simple test that logs in and out to a local server 50 times in a row, and the time went down from 10 seconds to 3. The credibility of this benchmark is arguable, but honestly it’'s actually what I needed to do
More technically, I now handle many InterruptedExceptionS and ask the threads to interrupt instead of waiting them to poll the done variable. The done variable is still there because I didn’‘t want to risk breaking the logic and my time is limited. To be honest, I didn’'t check all the library, but I think I addressed the most important cases.
However, this time I had to make two compromises:
-
PacketCollector.nextResult methods would ideally throw InterruptedException back to the caller. Setting the interrupted flag is a compromise solution that avoids changing the ABI.
-
There is one sleep that I couldn’‘t remove in PacketReader.processListener because it’‘s supposed to poll frequently the packet collector. Polls are evil because they force regular context switches and consume a lot of CPU in multi-threaded systems, they are not scalable and the polling interval cannot be increased too much lest it becomes unresponsive. If possible, listeners should be used instead. However, I’'d like to know your opinion before undertaking such a change myself, as it requires a change in the design of that part of the PacketReader and friends.
Oh, and this patch applies on top of my previous patch: http://www.jivesoftware.org/forums/thread.jspa?threadID=13730&tstart=0 .