Opened 2 years ago

Last modified 21 months ago

#4045 new enhancement

[PATCH] CmpRangeManager optimisation

Reported by: wraitii Owned by:
Priority: Nice to Have Milestone: Backlog
Component: Core engine Keywords: patch
Cc: Patch:

Description

Following some recent profiling, I have noticed that a big part of PerformQuery? is checking each entity for the existence of a given interface in TestEntity?.

The attached patch is rather barebones, but stores a pointer to the component cache in the entity data and thus makes this computation much much faster.

Would like some other people to confirm or infirm the results. I personally tested over an MP replay.

NB: fairly sure 64-bit systems will need the struct size to be 36 and not 32, should have used 28 + sizeof(void*).

Attachments (4)

CCmpRangeManager.cpp.patch (1.5 KB) - added by wraitii 2 years ago.
fewercopy.patch (6.2 KB) - added by wraitii 2 years ago.
fewercopy.2.patch (6.2 KB) - added by wraitii 2 years ago.
Updated to remove the empty check and the misplaced auto
fewercopy.3.patch (6.1 KB) - added by wraitii 2 years ago.
Saner version passing a vector thanks to sanderd

Download all attachments as: .zip

Change History (8)

Changed 2 years ago by wraitii

Attachment: CCmpRangeManager.cpp.patch added

comment:1 Changed 2 years ago by wraitii

In 18344:

Re-use the same vector for the range manager spatial subdivision queries to cut down on allocations and memory fragmentation. Refs #4045

Changed 2 years ago by wraitii

Attachment: fewercopy.patch added

comment:2 Changed 2 years ago by wraitii

fewercopy.patch changes the way we iterate the subdivision query results. Instead of copying the subdivision vectors in a larger vector, we copy a pointer to the first member of that vector and a size. Then we iterate in two steps, first over the returned vectors, then over the size (recasting everything properly). Data locality should be pretty good still.

This should be a bigger optimization if there are many entities in subdivisions.

Additionally hadn't noticed but we could probably skip the "empty" verification, since size would be 0.

Changed 2 years ago by wraitii

Attachment: fewercopy.2.patch added

Updated to remove the empty check and the misplaced auto

Changed 2 years ago by wraitii

Attachment: fewercopy.3.patch added

Saner version passing a vector thanks to sanderd

comment:3 Changed 2 years ago by elexis

Keywords: review removed

comment:4 Changed 21 months ago by elexis

Milestone: Alpha 21Backlog

Only one comment I think:

17:12 <@leper> #4045 not sure I like all that leaking of internal data structures all over the place
Note: See TracTickets for help on using tickets.