Tiny Code Fixes – Part 1
July 4, 2016
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