From c601426248265e330d7ff76fff00c2231a4e4e74 Mon Sep 17 00:00:00 2001 From: Randy Heit Date: Thu, 9 Apr 2009 01:40:30 +0000 Subject: [PATCH] - 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 | 3 +- src/p_local.h | 31 ++++++++------ src/p_maputl.cpp | 105 +++++++++++++++++++++++++++-------------------- 3 files changed, 82 insertions(+), 57 deletions(-) diff --git a/docs/rh-log.txt b/docs/rh-log.txt index 74c60b1f4..68b3393dc 100644 --- a/docs/rh-log.txt +++ b/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. diff --git a/src/p_local.h b/src/p_local.h index 269a5142d..e1e248b6b 100644 --- a/src/p_local.h +++ b/src/p_local.h @@ -256,32 +256,39 @@ public: class FBlockThingsIterator { - static TArray 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 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(); - static void SetCheckIndex(int newvalue); - FBlockThingsIterator(int x, int y, int checkindex); + FBlockThingsIterator(); 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(); diff --git a/src/p_maputl.cpp b/src/p_maputl.cpp index 775a408b0..f1818a3d8 100644 --- a/src/p_maputl.cpp +++ b/src/p_maputl.cpp @@ -735,70 +735,54 @@ line_t *FBlockLinesIterator::Next() } } -//=========================================================================== -// -// FBlockThingsIterator :: CheckArray -// -//=========================================================================== - -TArray 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; - dontfreecheck = true; - minx = maxx = x; - miny = maxy = y; - Reset(); + minx = maxx = 0; + miny = maxy = 0; + ClearHash(); + block = NULL; } 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) - { - CheckArray.Push (me); + if (i < 0) + { // Add me to the hash table and return 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, mapy + mapystep, BTI_CheckIndex); + AddThingIntercepts(mapx + mapxstep, mapy, btit); + 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; }