The Case Of The ExportHash String Monster

The changes from this blog can be seen in our Github Pull Request: github.com/EpicGames/UnrealEngine/pull/3723

OK, forgive the title… just get over it. It’s there, I wrote it and I’m keeping it like that.

Moving right along… optimization isn’t just about getting better framerates. Sometimes, you want to improve load times, increase the speed of content cooking, or of commandlets… and sometimes you just want to make your own job easier by removing some of the bloat – clearing the trees so you can see the wood, as it were.

Today, I’m looking at something that could improve a few of those – though in this case, I was concentrating on load times. This is something that we’ll likely be looking at several times more on this blog as we’ve really been doing a lot of work in this field at Coconut Lizard, making sure that our client’s game (LawBreakers from Boss Key) loads as quick as possible, so you’ll see more blog posts from myself and others here over the coming months on this.

The function I want to highlight in this post is CreateExportHash(). With some very simple optimizations here we were able to drop the load time on our large test map (along with characters, weapons, UI and everything else we needed) from ~8.5 to ~7 seconds – ie. it was ~18% faster. CreateExportHash() itself, for us, was 40 times faster.

static inline int32 HashNames(FName Object, FName Class, FName Package)
{
  return Object.GetComparisonIndex() + 7 * Class.GetComparisonIndex() + 31 * FPackageName::GetShortFName(Package).GetComparisonIndex();
}

GetShortFName() here was doing several memory allocations just to convert such as ‘/script/engine’ to be ‘engine’. My immediate thought here: why? Unless it was originally intended as an optimization, that hashing a shorter name might be faster perhaps?, I couldn’t see why it was needed. Reasons for/against it that I could think of:-

For: Perhaps it needed to match up packages that could be in different paths? eg. ‘/Maps/MyMap/Hello’ and ‘/Maps/MyNewAndCoolMap/Hello’;
For: Perhaps, a long time ago, GetComparisonIndex() was more expensive? Right now, it just returns an index stored for each FName – so is incredibly fast;
Against: Perhaps those same 2 maps that I example above should actually hash differently..? If so, shortening the name could introduce a bug.

I analysed the code using the ExportHash to see if there was any good reason to use shortened names and couldn’t find one … maybe somebody from Epic, or elsewhere, can interject..? If my assumption is wrong then I apologise – but, either way, there are still optimizations that should be made here.

Anyway, I went ahead and just stripped the shortening out:-

static inline int32 HashNames(FName Object, FName Class, FName Package)
{
  return Object.GetComparisonIndex() + 7 * Class.GetComparisonIndex() + 31 * Package.GetComparisonIndex();
}

I recooked content, rebuilt everything, tested thoroughly (editor, client and server) – it all worked perfectly (caveat: it could be that others’ content layouts are different – somebody from Epic could probably validate my change?). Not only that but CreateExportHash() was now around 6 times faster.

Digging deeper into CreateExportHash(), I found some other “interesting” things happening. A lot of time was spent with “IsTimeLimitExceeded()”. This function is used within several load-time tasks to try to prevent the engine from stalling – as these functions are often run on the main thread, if something takes 1000ms, that’s the game “stuck” for a whole second… so it’s a useful thing to have – under the right circumstances – as it can be set to hint to various functions that they’ve been given too much time (written correctly, they can quickly return out and continue on the next game tick).

// Set up export hash, potentially spread across several frames.
while( ExportHashIndex < ExportMap.Num() && !IsTimeLimitExceeded(TEXT("creating export hash"),100) )

nb. the “100” above gives a granularity to the time limit check (so the calculation is only done once every hundred iterations).

With CreateExportHash(), before the optimization above, we were seeing the function take only a couple of milliseconds. With the optimization, each call was well below a single millisecond. So we just completely removed the time limit:-

// Set up export hash
while (ExportHashIndex < ExportMap.Num())

There’s a further time limit check at the end of the function:-

// Return whether we finished this step and it's safe to start with the next.
return ((ExportHashIndex == ExportMap.Num()) && !IsTimeLimitExceeded( TEXT("creating export hash") )) ? LINKER_Loaded : LINKER_TimedOut;

To me, this one looked problematic anyway… if the time limit had passed on the while loop above, but it hadn’t been picked up due to the granularity of checking, and then the ExportHash had completed (so ExportHashIndex would equal ExportMap.Num()), you’d end up returning “LINKER_TimedOut” when you should be returning “LINKER_Loaded”. Not exactly catastrophic – it would just mean that the control function would call CreateExportHash() again and it would return almost immediately with LINKER_Loaded.

Since we’ve removed the timeout completely from the while() loop, it means that CreateExportHash() will -always- complete the work given to it when it returns… so we can go ahead and change the return line to:-

return LINKER_Loaded;

Finally, we have another small optimization for this function to wrap this up.

As we’re now creating the entire ExportHash in a single pass, we don’t need to worry about external code reading incomplete hash entries. Note the following section:-

if( ExportHashIndex == 0 )
{
  for( int32 i=0; i<ARRAY_COUNT(ExportHash); i++ )
  {
    ExportHash[i] = INDEX_NONE;
  }
}

We’re filling in “almost” the entirety of ExportHash that will be accessed. So, in fact, we can replace this with the much more succint:-

if( ExportHashIndex == 0 )
{
  ExportHash[0] = INDEX_NONE;
}

And that wraps up another blog post!

Credit(s): Robert Troughton (Coconut Lizard)
Status: Currently unimplemented in 4.12