skip to main content

Tiny Code Fixes – Part 1

TinyCodeFixes-Part1

Sometimes, you come across some nice little code fixes/improvements that are worth making but that are so tiny, they don’t really deserve blogging about… but… as we’re not using Git at Coconut Lizard and would still like to pass these fixes back to Epic and the community, we decided that we could just pass these through the blog regardless… so, without further ado…

Ei8ht


This bug’s been in the codebase since sometime around May 2015.

  • fixed issue with not being able to tell if a content-based project needs to be built as code within the editor UE-11636 (CL 2533916, Peter Sauerbrei)

With this change came, amongst other things, the following:-

bool GameProjectUtils::HasDefaultBuildSettings(const FName InPlatformInfoName)
{
  // first check default build settings for all platforms
  TArray<FString> BoolKeys, IntKeys, StringKeys, BuildKeys;
  BuildKeys.Add(TEXT("bCompileApex")); BuildKeys.Add(TEXT("bCompileBox2D")); BuildKeys.Add(TEXT("bCompileICU"));
  BuildKeys.Add(TEXT("bCompileSimplygon")); BuildKeys.Add(TEXT("bCompi8leLeanAndMeanUE")); BuildKeys.Add(TEXT("bIncludeADO"));

Note the “8” in bCompi8leLeanAndMeanUE … this should of course be bCompileLeanAndMeanUE…

Mass Conversion


The next find is in FModuleManager::AddModule()… not a bug as such – but still quite a bad coding mistake. Thankfully, one that’s not in any runtime performance-critical code…

const FString ModuleName = *InModuleName.ToString();

If you disassembled this, you’d be horified with the amount of generated code here. What this line is doing is converting an FName to a string (through ToString()), then converting to a TCHAR array (through the dereference) and then back to an FString. This should of course be:-

const FString ModuleName = InModuleName.ToString();

This code is only used on non-shipping/test builds by the way.

Hidden Complexities


Our third tidbit today is found in UEnum::GenerateEnumPrefix():-

FString UEnum::GenerateEnumPrefix() const
{
  FString Prefix;
  if (Names.Num() > 0)
  {
    Prefix = Names[0].Key.ToString();

    // For each item in the enumeration, trim the prefix as much as necessary to keep it a prefix.
    // This ensures that once all items have been processed, a common prefix will have been constructed.
    // This will be the longest common prefix since as little as possible is trimmed at each step.
    for (int32 NameIdx = 1; NameIdx < Names.Num(); NameIdx++)
    {
      FString EnumItemName = *Names[NameIdx].Key.ToString();

There are actually 2 things to look at here:-

  • line 6: A few subtle things going on here… some suspicious memory deallocations appearing in the disassembly which likely are never needed;
  • line 13: We have another Name->String->TCHAR[]->String case here, as in the previous example.

Let’s look at the disassembly for each of these…

Line 6 compiles to:-

00007FF73D9164AB  mov         rcx,qword ptr [rcx+48h]  
00007FF73D9164AF  mov         qword ptr [rax+10h],rbp  
00007FF73D9164B3  lea         rdx,[rax-48h]  
00007FF73D9164B7  mov         qword ptr [rax-28h],r13  
00007FF73D9164BB  call        FName::ToString (07FF73D8AFB60h)  
00007FF73D9164C0  mov         rbx,rax  
00007FF73D9164C3  cmp         rsi,rax  
00007FF73D9164C6  je          UEnum::GenerateEnumPrefix+6Eh (07FF73D9164EEh)  
00007FF73D9164C8  mov         rcx,qword ptr [rsi]  
00007FF73D9164CB  test        rcx,rcx  
00007FF73D9164CE  je          UEnum::GenerateEnumPrefix+55h (07FF73D9164D5h)  
00007FF73D9164D0  call        FMemory::Free (07FF73D7A7F90h)  
00007FF73D9164D5  mov         rcx,qword ptr [rbx]  
00007FF73D9164D8  mov         qword ptr [rsi],rcx  
00007FF73D9164DB  mov         qword ptr [rbx],r14  
00007FF73D9164DE  mov         eax,dword ptr [rbx+8]  
00007FF73D9164E1  mov         dword ptr [rsi+8],eax  
00007FF73D9164E4  mov         eax,dword ptr [rbx+0Ch]  
00007FF73D9164E7  mov         dword ptr [rsi+0Ch],eax  
00007FF73D9164EA  mov         qword ptr [rbx+8],r14  
00007FF73D9164EE  mov         rcx,qword ptr [rsp+30h]  
00007FF73D9164F3  test        rcx,rcx  
00007FF73D9164F6  je          UEnum::GenerateEnumPrefix+7Dh (07FF73D9164FDh)  
00007FF73D9164F8  call        FMemory::Free (07FF73D7A7F90h)  

And line 13:-

00007FF73D916530  movsxd      rcx,r13d  
00007FF73D916533  lea         rdx,[rsp+30h]  
00007FF73D916538  shl         rcx,4  
00007FF73D91653C  add         rcx,qword ptr [r15+48h]  
00007FF73D916540  call        FName::ToString (07FF73D8AFB60h)  
00007FF73D916545  cmp         dword ptr [rax+8],0  
00007FF73D916549  je          UEnum::GenerateEnumPrefix+0D0h (07FF73D916550h)  
00007FF73D91654B  mov         rdi,qword ptr [rax]  
00007FF73D91654E  jmp         UEnum::GenerateEnumPrefix+0D3h (07FF73D916553h)  
00007FF73D916550  mov         rdi,rbp  
00007FF73D916553  mov         r12,r14  
00007FF73D916556  mov         ebx,r14d  
00007FF73D916559  test        rdi,rdi  
00007FF73D91655C  je          UEnum::GenerateEnumPrefix+135h (07FF73D9165B5h)  
00007FF73D91655E  cmp         word ptr [rdi],0  
00007FF73D916562  je          UEnum::GenerateEnumPrefix+135h (07FF73D9165B5h)  
00007FF73D916564  or          rbx,0FFFFFFFFFFFFFFFFh  
00007FF73D916568  nop         dword ptr [rax+rax]  
00007FF73D916570  inc         rbx  
00007FF73D916573  cmp         word ptr [rdi+rbx*2],0  
00007FF73D916578  jne         UEnum::GenerateEnumPrefix+0F0h (07FF73D916570h)  
00007FF73D91657A  inc         ebx  
00007FF73D91657C  test        ebx,ebx  
00007FF73D91657E  jle         UEnum::GenerateEnumPrefix+124h (07FF73D9165A4h)  
00007FF73D916580  xor         edx,edx  
00007FF73D916582  mov         ecx,ebx  
00007FF73D916584  lea         r8d,[rdx+2]  
00007FF73D916588  call        DefaultCalculateSlack (07FF73D76AF70h)  
00007FF73D91658D  test        eax,eax  
00007FF73D91658F  je          UEnum::GenerateEnumPrefix+124h (07FF73D9165A4h)  
00007FF73D916591  movsxd      rdx,eax  
00007FF73D916594  xor         r8d,r8d  
00007FF73D916597  xor         ecx,ecx  
00007FF73D916599  add         rdx,rdx  
00007FF73D91659C  call        FMemory::Realloc (07FF73D7BEBA0h)  
00007FF73D9165A1  mov         r12,rax  
00007FF73D9165A4  movsxd      r8,ebx  
00007FF73D9165A7  mov         rdx,rdi  
00007FF73D9165AA  mov         rcx,r12  
00007FF73D9165AD  add         r8,r8  
00007FF73D9165B0  call        FGenericPlatformString::Memcpy (07FF73D776F20h)  
00007FF73D9165B5  mov         rcx,qword ptr [rsp+30h]  
00007FF73D9165BA  test        rcx,rcx  
00007FF73D9165BD  je          UEnum::GenerateEnumPrefix+144h (07FF73D9165C4h)  
00007FF73D9165BF  call        FMemory::Free (07FF73D7A7F90h)  

Now let’s fixup the code and compare:-

A much better version of this code:-

FString UEnum::GenerateEnumPrefix() const
{
  FString Prefix;
  if (Names.Num() > 0)
  {
    Names[0].Key.ToString(Prefix);

    // For each item in the enumeration, trim the prefix as much as necessary to keep it a prefix.
    // This ensures that once all items have been processed, a common prefix will have been constructed.
    // This will be the longest common prefix since as little as possible is trimmed at each step.
    for (int32 NameIdx = 1; NameIdx < Names.Num(); NameIdx++)
    {
      FString EnumItemName = Names[NameIdx].Key.ToString();

And let’s see the disassembly again. Line 6 now gives:-

00007FF6AFA76944  mov         rcx,qword ptr [rcx+48h]  
00007FF6AFA76948  mov         qword ptr [rax+8],rsi  
00007FF6AFA7694C  mov         qword ptr [rax+10h],r12  
00007FF6AFA76950  mov         qword ptr [rax+18h],r14  
00007FF6AFA76954  mov         qword ptr [rax-28h],r15  
00007FF6AFA76958  call        FName::ToString (07FF6AFA0FB80h)

And line 13:-

00007FF6AFA76980  movsxd      rcx,r12d  
00007FF6AFA76983  lea         rdx,[EnumItemName]  
00007FF6AFA76988  shl         rcx,4  
00007FF6AFA7698C  add         rcx,qword ptr [r13+48h]  
00007FF6AFA76990  call        FName::ToString (07FF6AFA0FB60h)

In total, the entire GenerateEnumPrefix() was coming out 212 bytes shorter for me.

64k Test Data


The final finding I’m going to talk about is a 64k buffer called RawVoiceTestData[] … note the code below:-

#if WITH_DEV_AUTOMATION_TESTS
uint8 RawVoiceTestData[] = { 0x0, 0x0 };
#else
/** 16khz wav mono voice data "this is a test" 2 sec duration */
uint8 RawVoiceTestData[] = {
  0xcc, 0xff, 0xb8, 0xff, 0x97, 0xff, 0x87, 0xff, 0x94, 0xff, 0xa0, 0xff, 0xad, 0xff, 0xa0, 0xff, 0xb7, 0xff, 0x9f, 0xff, 0x84, 0xff, 0x80, 0xff, 0x80, 0xff, 0x95, 0xff, 0x85, 0xff, 0x8e, 0xff, 

... many more lines of raw data follow 

The above buffer is only ever accessed through code where WITH_DEV_AUTOMATION_TESTS is set, meaning that the shorter, 2-byte, version of the buffer will be used – but never the 64k version. So we have a bug on the test build and we have wasted memory on non-test builds. Doh!

The code should of course be:-

#if WITH_DEV_AUTOMATION_TESTS
/** 16khz wav mono voice data "this is a test" 2 sec duration */
uint8 RawVoiceTestData[] = {
  0xcc, 0xff, 0xb8, 0xff, 0x97, 0xff, 0x87, 0xff, 0x94, 0xff, 0xa0, 0xff, 0xad, 0xff, 0xa0, 0xff, 0xb7, 0xff, 0x9f, 0xff, 0x84, 0xff, 0x80, 0xff, 0x80, 0xff, 0x95, 0xff, 0x85, 0xff, 0x8e, 0xff, 

... etc

See you next time..!

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

Facebook Messenger Twitter Pinterest Whatsapp Email
Go to Top