Opened 18 months ago

Last modified 14 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 18 months ago.
fewercopy.patch (6.2 KB) - added by wraitii 18 months ago.
fewercopy.2.patch (6.2 KB) - added by wraitii 18 months ago.
Updated to remove the empty check and the misplaced auto
fewercopy.3.patch (6.1 KB) - added by wraitii 18 months ago.
Saner version passing a vector thanks to sanderd

Download all attachments as: .zip

Change History (8)

Changed 18 months ago by wraitii

Attachment: CCmpRangeManager.cpp.patch added

comment:1 Changed 18 months 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 18 months ago by wraitii

Attachment: fewercopy.patch added

comment:2 Changed 18 months 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 18 months ago by wraitii

Attachment: fewercopy.2.patch added

Updated to remove the empty check and the misplaced auto

Changed 18 months ago by wraitii

Attachment: fewercopy.3.patch added

Saner version passing a vector thanks to sanderd

comment:3 Changed 18 months ago by elexis

Keywords: review removed

comment:4 Changed 14 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.