- More performance optimization for FBlockThingsIterator::Next(): Changed

the array used to keep track of visited actors into a hash table.

SVN r1533 (trunk)

docs/rh-log.txt

@@ -1,7 +1,8 @@
 April 8, 2009
 - Performance optimization for FBlockThingsIterator::Next(): Actors that
   exist in only one block don't need to be added to the CheckArray or
-  scanned for in it.
+  scanned for in it. Also changed the array used to keep track of visited
+  actors into a hash table.
 
 April 7, 2009  (Changes by Graf Zahl)
 - added some default definitions for constants that may miss in some headers.

src/p_local.h

@@ -256,32 +256,39 @@ public:
 
 class FBlockThingsIterator
 {
-	static TArray<AActor *> CheckArray;
-
 	int minx, maxx;
 	int miny, maxy;
 
 	int curx, cury;
 
-	bool dontfreecheck;
-	int checkindex;
-
 	FBlockNode *block;
 
-	void StartBlock(int x, int y);
+	int Buckets[32];
 
-	// The following 3 functions are only for use in the path traverser 
+	struct HashEntry
+	{
+		AActor *Actor;
+		int Next;
+	};
+	HashEntry FixedHash[10];
+	int NumFixedHash;
+	TArray<HashEntry> DynHash;
+
+	HashEntry *GetHashEntry(int i) { return i < countof(FixedHash) ? &FixedHash[i] : &DynHash[i - countof(FixedHash)]; }
+
+	void StartBlock(int x, int y);
+	void SwitchBlock(int x, int y);
+	void ClearHash();
+
+	// The following is only for use in the path traverser 
 	// and therefore declared private.
-	static int GetCheckIndex();
+	FBlockThingsIterator();
-	static void SetCheckIndex(int newvalue);
-	FBlockThingsIterator(int x, int y, int checkindex);
 
 	friend class FPathTraverse;
 
 public:
 	FBlockThingsIterator(int minx, int miny, int maxx, int maxy);
 	FBlockThingsIterator(const FBoundingBox &box);
-	~FBlockThingsIterator();
 	AActor *Next();
 	void Reset() { StartBlock(minx, miny); }
 };
@@ -297,7 +304,7 @@ class FPathTraverse
 	unsigned int count;
 
 	void AddLineIntercepts(int bx, int by);
-	void AddThingIntercepts(int bx, int by, int checkindex);
+	void AddThingIntercepts(int bx, int by, FBlockThingsIterator &it);
 public:
 
 	intercept_t *Next();

src/p_maputl.cpp

@@ -735,70 +735,54 @@ line_t *FBlockLinesIterator::Next()
 	}
 }
 
-//===========================================================================
-//
-// FBlockThingsIterator :: CheckArray
-//
-//===========================================================================
-
-TArray<AActor *> FBlockThingsIterator::CheckArray(32);
-
-int FBlockThingsIterator::GetCheckIndex()
-{
-	return CheckArray.Size();
-}
-
-void FBlockThingsIterator::SetCheckIndex(int newvalue)
-{
-	CheckArray.Resize(newvalue);
-}
-
 //===========================================================================
 //
 // FBlockThingsIterator :: FBlockThingsIterator
 //
 //===========================================================================
 
-FBlockThingsIterator::FBlockThingsIterator(int x, int y, int check)
+FBlockThingsIterator::FBlockThingsIterator()
+: DynHash(0)
 {
-	checkindex = check;
+	minx = maxx = 0;
-	dontfreecheck = true;
+	miny = maxy = 0;
-	minx = maxx = x;
+	ClearHash();
-	miny = maxy = y;
+	block = NULL;
-	Reset();
 }
 
 FBlockThingsIterator::FBlockThingsIterator(int _minx, int _miny, int _maxx, int _maxy)
+: DynHash(0)
 {
-	checkindex = CheckArray.Size();
-	dontfreecheck = false;
 	minx = _minx;
 	maxx = _maxx;
 	miny = _miny;
 	maxy = _maxy;
+	ClearHash();
 	Reset();
 }
 
 FBlockThingsIterator::FBlockThingsIterator(const FBoundingBox &box)
+: DynHash(0)
 {
-	checkindex = CheckArray.Size();
-	dontfreecheck = false;
 	maxy = (box.Top() - bmaporgy) >> MAPBLOCKSHIFT;
 	miny = (box.Bottom() - bmaporgy) >> MAPBLOCKSHIFT;
 	maxx = (box.Right() - bmaporgx) >> MAPBLOCKSHIFT;
 	minx = (box.Left() - bmaporgx) >> MAPBLOCKSHIFT;
+	ClearHash();
 	Reset();
 }
 
 //===========================================================================
 //
-// FBlockThingsIterator :: FreeCheckArray
+// FBlockThingsIterator :: ClearHash
 //
 //===========================================================================
 
-FBlockThingsIterator::~FBlockThingsIterator()
+void FBlockThingsIterator::ClearHash()
 {
-	if (!dontfreecheck) CheckArray.Resize(checkindex);
+	clearbuf(Buckets, countof(Buckets), -1);
+	NumFixedHash = 0;
+	DynHash.Clear();
 }
 
 //===========================================================================
@@ -822,6 +806,19 @@ void FBlockThingsIterator::StartBlock(int x, int y)
 	}
 }
 
+//===========================================================================
+//
+// FBlockThingsIterator :: SwitchBlock
+//
+//===========================================================================
+
+void FBlockThingsIterator::SwitchBlock(int x, int y)
+{
+	minx = maxx = x;
+	miny = maxy = y;
+	StartBlock(x, y);
+}
+
 //===========================================================================
 //
 // FBlockThingsIterator :: Next
@@ -836,6 +833,7 @@ AActor *FBlockThingsIterator::Next()
 		{
 			AActor *me = block->Me;
 			FBlockNode *mynode = block;
+			HashEntry *entry;
 			int i;
 
 			block = block->NextActor;
@@ -844,16 +842,36 @@ AActor *FBlockThingsIterator::Next()
 			{ // This actor doesn't span blocks, so we know it can only ever be checked once.
 				return me;
 			}
-			for (i = (int)CheckArray.Size() - 1; i >= checkindex; --i)
+			size_t hash = ((size_t)me >> 3) % countof(Buckets);
+			for (i = Buckets[hash]; i >= 0; )
 			{
-				if (CheckArray[i] == me)
+				entry = GetHashEntry(i);
-				{
+				if (entry->Actor == me)
+				{ // I've already been checked. Skip to the next actor.
 					break;
 				}
+				i = entry->Next;
 			}
-			if (i < checkindex)
+			if (i < 0)
-			{
+			{ // Add me to the hash table and return me.
-				CheckArray.Push (me);
+				if (NumFixedHash < countof(FixedHash))
+				{
+					entry = &FixedHash[NumFixedHash];
+					entry->Next = Buckets[hash];
+					Buckets[hash] = NumFixedHash++;
+				}
+				else
+				{
+					if (DynHash.Size() == 0)
+					{
+						DynHash.Grow(50);
+					}
+					i = DynHash.Reserve(1);
+					entry = &DynHash[i];
+					entry->Next = Buckets[hash];
+					Buckets[hash] = i + countof(FixedHash);
+				}
+				entry->Actor = me;
 				return me;
 			}
 		}
@@ -941,11 +959,11 @@ void FPathTraverse::AddLineIntercepts(int bx, int by)
 //
 //===========================================================================
 
-void FPathTraverse::AddThingIntercepts (int bx, int by, int checkindex)
+void FPathTraverse::AddThingIntercepts (int bx, int by, FBlockThingsIterator &it)
 {
-	FBlockThingsIterator it(bx, by, checkindex);
 	AActor *thing;
 
+	it.SwitchBlock(bx, by);
 	while ((thing = it.Next()))
 	{
 		int numfronts = 0;
@@ -1180,7 +1198,7 @@ FPathTraverse::FPathTraverse (fixed_t x1, fixed_t y1, fixed_t x2, fixed_t y2, in
 	mapy = yt1;
 		
 	// we want to use one list of checked actors for the entire operation
-	int BTI_CheckIndex = FBlockThingsIterator::GetCheckIndex();
+	FBlockThingsIterator btit;
 	for (count = 0 ; count < 100 ; count++)
 	{
 		if (flags & PT_ADDLINES)
@@ -1190,7 +1208,7 @@ FPathTraverse::FPathTraverse (fixed_t x1, fixed_t y1, fixed_t x2, fixed_t y2, in
 		
 		if (flags & PT_ADDTHINGS)
 		{
-			AddThingIntercepts(mapx, mapy, BTI_CheckIndex);
+			AddThingIntercepts(mapx, mapy, btit);
 		}
 				
 		if (mapx == xt2 && mapy == yt2)
@@ -1228,8 +1246,8 @@ FPathTraverse::FPathTraverse (fixed_t x1, fixed_t y1, fixed_t x2, fixed_t y2, in
 			
 			if (flags & PT_ADDTHINGS)
 			{
-				AddThingIntercepts(mapx + mapxstep, mapy, BTI_CheckIndex);
+				AddThingIntercepts(mapx + mapxstep, mapy, btit);
-				AddThingIntercepts(mapx, mapy + mapystep, BTI_CheckIndex);
+				AddThingIntercepts(mapx, mapy + mapystep, btit);
 			}
 			xintercept += xstep;
 			yintercept += ystep;
@@ -1238,7 +1256,6 @@ FPathTraverse::FPathTraverse (fixed_t x1, fixed_t y1, fixed_t x2, fixed_t y2, in
 			break;
 		}
 	}
-	FBlockThingsIterator::SetCheckIndex(BTI_CheckIndex);
 	maxfrac = FRACUNIT;
 }