powered by Jive Software

Removed unnecessary delays

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:

  1. PacketCollector.nextResult methods would ideally throw InterruptedException back to the caller. Setting the interrupted flag is a compromise solution that avoids changing the ABI.

  2. 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 .

And now, the patch:

Uhm… it looks wrong. I can mail it to whomever wants it.

Javier,

You now have permission to create attachments in this forum. Would you mind trying to post the patch again?

Thanks,

Matt

Thanks matt, here’'s the patch again.

Nope, here’'s the patch. There seems to be a problem with previewing posts with attach files.
smack-1.4.1-jkohen-interrupt.patch (10539 Bytes)

Also note that PacketReader.shutdown and PacketWriter.shutdown should be synchronized to avoid sending multiple disconnection notifications to the listeners.

In theory all accesses to ‘‘done’’ should be synchronized or the variable made volatile, as there is no guarantee that the variable will be updated among different threads otherwise. This could be easily triggered on SMP machines, maybe even on HyperThreaded CPUs.

That, or eliminating done in favor of thread interruption But I think the former would be easier to get right for now.