From 90969e252a9f4effa9591fc859908228a823d4ad Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 22 Feb 2015 10:44:22 +0100 Subject: [PATCH] Improve the bundle and defer mechanism Instead of always bundling when the queue is empty, only bundle once the queue has become empty and then only if it has been become empty again. The previous algorithm would also bundle and defer the last element found in the queue. This is not the case now. --- .../smack/tcp/BundleAndDeferCallback.java | 2 +- .../smack/tcp/XMPPTCPConnection.java | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/BundleAndDeferCallback.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/BundleAndDeferCallback.java index 5102b0019..050a16485 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/BundleAndDeferCallback.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/BundleAndDeferCallback.java @@ -20,7 +20,7 @@ package org.jivesoftware.smack.tcp; * This callback is used to get the current value of the period in which Smack does bundle and defer * outgoing stanzas. *

- * Smack will bundle and defer stanzas if the connection is authenticated, the send queue is empty + * Smack will bundle and defer stanzas if the connection is authenticated * and if a bundle and defer callback is set, either via * {@link XMPPTCPConnection#setDefaultBundleAndDeferCallback(BundleAndDeferCallback)} or * {@link XMPPTCPConnection#setBundleandDeferCallback(BundleAndDeferCallback)}, and diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 774a5e24c..a6dc0a516 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -1177,6 +1177,16 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { private volatile boolean instantShutdown; + /** + * True if some preconditions are given to start the bundle and defer mechanism. + *

+ * This will likely get set to true right after the start of the writer thread, because + * {@link #nextStreamElement()} will check if {@link queue} is empty, which is probably the case, and then set + * this field to true. + *

+ */ + private boolean shouldBundleAndDefer; + /** * Initializes the writer in order to be used. It is called at the first connection and also * is invoked if the connection is disconnected by an error. @@ -1262,6 +1272,10 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { * @return the next element for writing or null. */ private Element nextStreamElement() { + // It is important the we check if the queue is empty before removing an element from it + if (queue.isEmpty()) { + shouldBundleAndDefer = true; + } Element packet = null; try { packet = queue.take(); @@ -1292,7 +1306,10 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { // If the preconditions are given (e.g. bundleAndDefer callback is set, queue is // empty), then we could wait a bit for further stanzas attempting to decrease // our energy consumption - if (localBundleAndDeferCallback != null && isAuthenticated() && queue.isEmpty()) { + if (localBundleAndDeferCallback != null && isAuthenticated() && shouldBundleAndDefer) { + // Reset shouldBundleAndDefer to false, nextStreamElement() will set it to true once the + // queue is empty again. + shouldBundleAndDefer = false; final AtomicBoolean bundlingAndDeferringStopped = new AtomicBoolean(); final int bundleAndDeferMillis = localBundleAndDeferCallback.getBundleAndDeferMillis(new BundleAndDefer( bundlingAndDeferringStopped));