[jboss-cvs] JBoss Messaging SVN: r5032 - in trunk: src/main/org/jboss/messaging/util and 1 other directories.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Fri Sep 26 15:41:39 EDT 2008
Author: clebert.suconic at jboss.com
Date: 2008-09-26 15:41:39 -0400 (Fri, 26 Sep 2008)
New Revision: 5032
Modified:
trunk/src/main/org/jboss/messaging/core/journal/impl/JournalImpl.java
trunk/src/main/org/jboss/messaging/util/TimeAndCounterIDGenerator.java
trunk/tests/src/org/jboss/messaging/tests/unit/util/TimeAndCounterIDGeneratorTest.java
Log:
Changing TimeAndCounterIDGenerator to not need a synchronized block
Modified: trunk/src/main/org/jboss/messaging/core/journal/impl/JournalImpl.java
===================================================================
--- trunk/src/main/org/jboss/messaging/core/journal/impl/JournalImpl.java 2008-09-26 17:45:57 UTC (rev 5031)
+++ trunk/src/main/org/jboss/messaging/core/journal/impl/JournalImpl.java 2008-09-26 19:41:39 UTC (rev 5032)
@@ -46,7 +46,6 @@
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
Modified: trunk/src/main/org/jboss/messaging/util/TimeAndCounterIDGenerator.java
===================================================================
--- trunk/src/main/org/jboss/messaging/util/TimeAndCounterIDGenerator.java 2008-09-26 17:45:57 UTC (rev 5031)
+++ trunk/src/main/org/jboss/messaging/util/TimeAndCounterIDGenerator.java 2008-09-26 19:41:39 UTC (rev 5032)
@@ -36,15 +36,22 @@
*/
public class TimeAndCounterIDGenerator implements IDGenerator
{
- // (0x7fffffff) We take one bit out, as we don't want negative numbers
- // (take out the signal bit before merging the numbers)
- private static final long MASK_TIME = Integer.MAX_VALUE;
-
- // Attributes ----------------------------------------------------
+ // Constants ----------------------------------------------------
/**
- * Using a long just to avoid making cast conversions on every ID generated
+ * Bits to move the date accordingly to MASK_TIME
*/
+ private static final int BITS_TO_MOVE = 28;
+
+ // We take one bit out, as we don't want negative numbers
+ // With 4 bytes + 4 bits, we would minimize the possibility of duplicate IDs.
+ // The date portion would be repeated on every 397 days
+ //
+ // 4 bytes + 4 bits without the signal bit
+ public static final long MASK_TIME = 0x7ffffffffl;
+
+ // Attributes ----------------------------------------------------
+
private final AtomicLong counter = new AtomicLong(0);
private volatile long tmMark;
@@ -60,50 +67,59 @@
// Public --------------------------------------------------------
- //TODO - I have temporarily sychronized this since there is a race condition otherwise.
- //Since tmMark could get reset by another thread between entering this method and the tmMark|value being evaluated
- //at the end
- //The fix is simple - don't evaulate the bitwise or every time, but no time to do it right now
- public synchronized long generateID()
+ // Public --------------------------------------------------------
+
+ public long generateID()
{
- long value = counter.incrementAndGet();
- if (value >= Integer.MAX_VALUE)
+ long retValue = counter.incrementAndGet();
+
+ // The probability of a negative is very low.
+ // The server has to be started at the exact millisecond (or very close) to when
+ // System.currentTimeMillis() == **7ffffffffl or **fffffffffl(what would
+ // happen every 397 days and few hours), for instance:
+ // (117FFFFFFFF = Sat Feb 09 15:00:42 GMT-06:00 2008).
+ // But I still wanted to verify this for correctness.
+ while (retValue < 0)
{
- synchronized (this)
- {
- if (counter.get() >= Integer.MAX_VALUE)
- {
- refresh();
- }
- value = counter.incrementAndGet();
- }
+ refresh();
+ retValue = counter.incrementAndGet();
}
- return tmMark | value;
+ return retValue;
}
-
+
public long getCurrentID()
{
- return tmMark | counter.get();
+ return counter.get();
}
-
+ // for use in testcases
public void setInternalID(final long id)
{
- counter.set(id);
+ counter.set(tmMark | id);
}
+ // for use in testcases
+ public void setInternalDate(final long date)
+ {
+ tmMark = (date & MASK_TIME) << BITS_TO_MOVE;
+ counter.set(tmMark);
+ }
+
public synchronized void refresh()
{
+ long oldTm = counter.get() >> BITS_TO_MOVE;
long newTm = newTM();
- // To avoid quick restarts. We need to ensure that not more than Integer.MAX_VALUE aren't produced
- // for some value of time
- while (newTm <= tmMark)
+ // To avoid quick restarts on testcases.
+ // In a real scenario this will never happen, as refresh is called only on constructor or when the first bit on the counter explodes
+ // And that would happen only at one specific millisecond every 368 days, and that would never hit a situation where
+ // newTM == oldTM
+ while (newTm == oldTm)
{
try
{
- Thread.sleep(1);
+ Thread.sleep(20);
}
catch (InterruptedException e)
{
@@ -111,13 +127,19 @@
newTm = newTM();
}
tmMark = newTm;
- counter.set(0);
+ counter.set(tmMark);
}
@Override
public String toString()
{
- return "TimeAndCounterIDGenerator(tmMark=" + String.format("%1$X", tmMark) + ", counter = " + counter.get() + ")";
+ long currentCounter = counter.get();
+ return "SequenceGenerator(tmMark=" + hex(tmMark) +
+ ", CurrentCounter = " +
+ currentCounter +
+ ", HexCurrentCounter = " +
+ hex(currentCounter) +
+ ")";
}
// Package protected ---------------------------------------------
@@ -128,9 +150,12 @@
private long newTM()
{
- return (System.currentTimeMillis() & MASK_TIME) << 32;
+ return (System.currentTimeMillis() & MASK_TIME) << BITS_TO_MOVE;
}
- // Inner classes -------------------------------------------------
+ private String hex(final long x)
+ {
+ return String.format("%1$X", x);
+ }
}
Modified: trunk/tests/src/org/jboss/messaging/tests/unit/util/TimeAndCounterIDGeneratorTest.java
===================================================================
--- trunk/tests/src/org/jboss/messaging/tests/unit/util/TimeAndCounterIDGeneratorTest.java 2008-09-26 17:45:57 UTC (rev 5031)
+++ trunk/tests/src/org/jboss/messaging/tests/unit/util/TimeAndCounterIDGeneratorTest.java 2008-09-26 19:41:39 UTC (rev 5032)
@@ -88,7 +88,7 @@
final TimeAndCounterIDGenerator seq = new TimeAndCounterIDGenerator();
- seq.setInternalID(Integer.MAX_VALUE - 50);
+ seq.setInternalID(0xfffffffl - 1);
final int NUMBER_OF_THREADS = 100;
@@ -155,10 +155,31 @@
hashSet.clear();
}
+
+ public void testEdgeCaseOnDate()
+ {
+ long date = 0x117FFFFFFFFl;
+ final TimeAndCounterIDGenerator seq = new TimeAndCounterIDGenerator();
+
+ seq.setInternalDate(date);
+
+ seq.setInternalID(0xfffffffl);
+
+ long newID = seq.generateID();
+
+ assertTrue("should be a positive number", newID > 0);
+
+ assertEquals("Counter part on ID should be 0x0000001, but it was " + (hex(newID & 0xfffffffl)),
+ 1,
+ newID & 0xfffffffl);
+
+ }
+
private static String hex(final long value)
{
return String.format("%1$X", value);
}
+
}
More information about the jboss-cvs-commits
mailing list