Skip to content

perf(gamememory): Remove unnecessary overhead from GameMemory allocator function overloads - #3266

Open
xezon wants to merge 6 commits into
TheSuperHackers:mainfrom
xezon:xezon/reduce-gamememory-overhead
Open

perf(gamememory): Remove unnecessary overhead from GameMemory allocator function overloads#3266
xezon wants to merge 6 commits into
TheSuperHackers:mainfrom
xezon:xezon/reduce-gamememory-overhead

Conversation

@xezon

@xezon xezon commented Sep 7, 2026

Copy link
Copy Markdown

Merge with Rebase

This change provides a number of small optimizations to the GameMemory to reduce computation overhead in its various new, delete, malloc, free overrides.

  1. The memory link tester is now compiled out in Release
  2. Delete and free functions are now exited early when called with nullptr
  3. The preMainInitMemoryManager function is now inlined for the hot paths (verified in assembler)
  4. Adds delete overloads that carry the size_t argument so that it does not need to trampoline to the other delete overloads
  5. Remove superfluous extern keywords in GameMemoryNull.cpp
  6. Remove unnecessary preMainInitMemoryManager calls and make freeBytes noexcept to get rid of EH frame in delete functions which removes a number of unnecessary instructions (Claude says operator delete is down from 68 bytes to 24 bytes)

Performance impact was not measured, because it will likely be very small, but allocators are very hot functions so it will be good to make them cheaper.

AI use

All code changes were applied by hand, but assisted by Claude Opus 5 for ideas and verification.

TODO

  • Add pull id to commit titles

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Gen Relates to Generals ZH Relates to Zero Hour Memory Is memory related labels Sep 7, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Reduce GameMemory allocator override overhead

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Reduces allocator hot-path work through inline initialization checks and null exits.
• Adds sized delete overloads and noexcept freeing to avoid trampolines and exception metadata.
• Restricts link verification bookkeeping to debug builds.
Diagram

graph TD
  A["Call Site"] --> B{"Memory Operation"}
  B -->|Allocate| C["Allocation APIs"] --> D{"Allocator Ready"}
  D -->|No| E["Cold Path Init"] --> F["Dynamic Allocator"]
  D -->|Yes| F
  B -->|Release| G{"Pointer Null"} -->|Yes| H["Immediate Return"]
  G -->|No| F
Loading
High-Level Assessment

The focused hot-path optimization is appropriate for these global allocator overrides. Shared deallocation helpers or reliance on compiler folding could reduce source duplication, but may reintroduce calls or produce less predictable output across supported compilers; retaining explicit small overloads is preferable here.

Files changed (6) +125 / -64

Enhancement (4) +27 / -11
GameMemory.hDeclare noexcept freeing and sized delete overloads +3/-1

Declare noexcept freeing and sized delete overloads

• Marks 'DynamicMemoryAllocator::freeBytes' as 'noexcept'. Declares scalar and array sized-delete overloads so compiler-generated sized deletion resolves directly to GameMemory.

Core/GameEngine/Include/Common/GameMemory.h

GameMemoryNull.hAlign null allocator declarations with optimized APIs +3/-1

Align null allocator declarations with optimized APIs

• Applies the 'noexcept' free contract and adds sized scalar and array delete declarations for the null GameMemory implementation.

Core/GameEngine/Include/Common/GameMemoryNull.h

GameMemoryNull.cppImplement sized deletes for the null allocator +19/-9

Implement sized deletes for the null allocator

• Marks null-allocator freeing as 'noexcept' and implements sized scalar and array delete overloads using 'free'. Removes redundant 'extern' specifiers from operator definitions.

Core/GameEngine/Source/Common/System/GameMemoryNull.cpp

always.hExpose sized delete overloads to WWLib +2/-0

Expose sized delete overloads to WWLib

• Declares scalar and array sized-delete overloads alongside the existing global memory operators, keeping WWLib consumers aligned with GameMemory.

Core/Libraries/Source/WWVegas/WWLib/always.h

Other (2) +98 / -53
GameMemory.cppStreamline GameMemory allocation and release hot paths +90/-53

Streamline GameMemory allocation and release hot paths

• Introduces an inline allocator-readiness guard with a non-inlined cold initialization path. Adds direct sized-delete implementations, null release exits, noexcept freeing, and removes redundant initialization checks from deallocation. Link verification counters and tests are now compiled only when crash debugging is enabled.

Core/GameEngine/Source/Common/System/GameMemory.cpp

CppMacros.hAdd a portable NOINLINE compiler macro +8/-0

Add a portable NOINLINE compiler macro

• Defines 'NOINLINE' for MSVC, GCC, and Clang with an empty fallback. GameMemory uses it to isolate pre-main initialization from the inline hot-path guard.

Dependencies/Utility/Utility/CppMacros.h

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR reduces overhead in the GameMemory allocation and deallocation paths.

  • Inlines the allocator-initialization fast-path while preserving a no-inline slow-path across supported compilers.
  • Adds sized scalar and array delete overloads.
  • Avoids allocator initialization and other work for null deallocations.
  • Compiles link validation only when its crash-based diagnostics are available.
  • Marks freeBytes as non-throwing and aligns the null allocator implementation with the optimized API.

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness, security, or repository-rule issues identified.

The latest compatibility adjustment scopes legacy MSVC auto-inlining pragmas precisely around the intended slow-path function, while supported newer compilers retain explicit no-inline attributes. The prior link-validation concern was correctly disputed and conceded because the tester has no useful feedback without crash diagnostics.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/System/GameMemory.cpp Optimizes global allocation and deallocation paths, adds sized deletes, and preserves pre-main initialization through an inline fast-path and no-inline implementation.
Dependencies/Utility/Utility/CppMacros.h Defines NOINLINE using compiler-supported attributes while leaving legacy MSVC to the locally scoped pragma fallback.
Core/GameEngine/Source/Common/System/GameMemoryNull.cpp Adds sized delete implementations, removes redundant extern specifiers from definitions, and makes freeBytes non-throwing.
Core/GameEngine/Include/Common/GameMemory.h Updates the allocator contract to noexcept and declares sized scalar and array delete overloads.
Core/GameEngine/Include/Common/GameMemoryNull.h Keeps the null allocator declarations consistent with the optimized GameMemory interface.

Reviews (2): Last reviewed commit: "perf(gamememory): Remove unnecessary cal..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/Common/System/GameMemory.cpp
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Exiting after a replay save can crash 🐞 Bug ≡ Correctness
Description
operator delete no longer calls preMainInitMemoryManager() and dereferences
TheDynamicMemoryAllocator after shutdownMemoryManager() has cleared it. When saveReplay()
assigns the static replayPath string, its heap storage is released during static destruction after
WinMain performs the explicit shutdown, reaching either changed delete overload with a null
allocator.
Code

Core/GameEngine/Source/Common/System/GameMemory.cpp[3290]

-	preMainInitMemoryManager();
+	LINK_TESTER_INCREMENT();
+	if (p == nullptr)
+		return;
+	DEBUG_ASSERTCRASH(TheDynamicMemoryAllocator != nullptr, ("must init memory manager before calling global operator delete"));
+	TheDynamicMemoryAllocator->freeBytes(p);
+}
+
+void operator delete(void *p, size_t)
+{
+	LINK_TESTER_INCREMENT();
+	if (p == nullptr)
+		return;
Evidence
The normal main path explicitly shuts the allocator down before returning, and shutdown sets
TheDynamicMemoryAllocator to null. replayPath has static storage duration and is assigned a full
replay path, so its destructor runs later and releases owned storage through the global delete
implementation, which now asserts and then dereferences the null allocator without initialization;
both sized and unsized variants have this behavior.

Core/GameEngine/Source/Common/System/GameMemory.cpp[3330-3346]
Core/GameEngine/Source/Common/System/GameMemory.cpp[3352-3368]
Core/GameEngine/Source/Common/System/GameMemory.cpp[3545-3571]
Generals/Code/Main/WinMain.cpp[900-931]
Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupReplay.cpp[235-258]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Global delete overloads can run during static destruction after `shutdownMemoryManager()` has destroyed and cleared the allocator. Ensure deallocation remains valid throughout process teardown rather than dereferencing a null allocator or reinitializing an allocator to free storage owned by the destroyed instance.

## Issue Context
The replay UI stores an allocated `std::string` in static storage, while `WinMain` explicitly shuts down GameMemory before static destructors execute. Apply the lifecycle-safe solution consistently to scalar, array, sized, placement-cleanup, STL, and optional C deallocation paths.

## Fix Focus Areas
- Core/GameEngine/Source/Common/System/GameMemory.cpp[3295-3368]
- Core/GameEngine/Source/Common/System/GameMemory.cpp[3389-3448]
- Core/GameEngine/Source/Common/System/GameMemory.cpp[3541-3571]
- Generals/Code/Main/WinMain.cpp[900-931]
- GeneralsMD/Code/Main/WinMain.cpp[900-959]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 30/18, lines 189/200; both must reach the floor). Router rationale: This modifies global allocation/deallocation behavior across multiple platforms and overloads, with initialization, null handling, ABI, and debug/release paths creating many independent, easy-to-miss correctness risks.

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Core/GameEngine/Source/Common/System/GameMemory.cpp
@xezon
xezon force-pushed the xezon/reduce-gamememory-overhead branch from 79a1e9e to 3a3b027 Compare September 7, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals Memory Is memory related Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant