Scrap dlight lighthead link lists

[TwelveEyes]

Replaces the lighthead double linked lists with unordered_maps. Vastly improves performance of large moving dlights over complex map geometry.

Test recordings on Bastion of Chaos (a very performance demanding map for GZDoom).

Renderer	Before	After
SW Pal	before_pal.webm	after_pal.webm
SW RGBA	before_rgba.webm	after_rgba.webm
HW	before_hw.webm	after_hw.webm

Before:
One large moving light utterly annihilates performance.

After:
Many large moving lights with acceptable performance.

[coelckers]

One more thing:

Please avoid using iterators in 'for' loops like this:

for (auto iter = touchlists->wall_tlist.begin(); iter != touchlists->wall_tlist.end(); iter++)

Use range-based 'for' loops instead.

[coelckers]

As a general comment, have you tried using TMap instead of unordered_map? While this is faster than the old linked list, at least in this particular case, I know the heap allocation behavior of these standard containers is utterly atrocious and this is low level rendering code and extremely dynamic data after all.

Also: Have you made performance tests with different compilers and standard libraries? One very unpleasant thing about these is that what works well on one compiler might not on the next due to different implementation of the container classes.
There's a reason, after all, why we never migrated off our custom container classes. Unlike the STL variants, these have fixed properties we control ourselves.

[TwelveEyes]

As a general comment, have you tried using TMap instead of unordered_map?

I have not tried TMap, no. I'll do some testing with TMap vs unordered_map.

Also: Have you made performance tests with different compilers and standard libraries? One very unpleasant thing about these is that what works well on one compiler might not on the next due to different implementation of the container classes. There's a reason, after all, why we never migrated off our custom container classes. Unlike the STL variants, these have fixed properties we control ourselves.

I've tested with clang 17 and VS2022 with comparable performance improvement. Don't know how GCC or earlier clang/VS compilations fare.

[Boondorl]

At a glance it looks like this removes the old light node allocator. Any particular reason? From what I could tell that was perfectly fine and simply prevented allocating/deallocating nodes constantly.

[TwelveEyes]

At a glance it looks like this removes the old light node allocator. Any particular reason? From what I could tell that was perfectly fine and simply prevented allocating/deallocating nodes constantly.

What part are you referring to, exactly?

[TwelveEyes]

Testing with TMap on this branch. The performance increase with moving dlights seems comparable to unordered_map. Though just looking directly down without moving from spawn in Bastion of Chaos, viewing stat fps with unordered_map I get ~31ms, while with TMap I get ~38ms.

[coelckers]

At a glance it looks like this removes the old light node allocator. Any particular reason? From what I could tell that was perfectly fine and simply prevented allocating/deallocating nodes constantly.

You cannot use that if you let the map allocate the nodes.

[coelckers]

I tried to understand where the speedup here comes from but failed so far.

@TwelveEyes
Can you please explain why using a map here makes such a difference? It cannot really be cache locality because with an unordered_map that isn't perfect either.

[MrRaveYard]

I tried to understand where the speedup here comes from but failed so far.

If we are talking old code vs this PR:

In order to avoid duplicate entries in the light lists, the list is linearly searched before anything is inserted. That's what VS profiler revealed (two years ago). Thus the old code is painfully slow in complex scenes if the light has to change position.

[TwelveEyes]

@TwelveEyes Can you please explain why using a map here makes such a difference? It cannot really be cache locality because with an unordered_map that isn't perfect either.

As @MrRaveYard said, the biggest drag on performance comes from traversing the lighthead link lists for every sidedef and section the light touches to check if the lists already contain a lightnode.

https://github.com/ZDoom/gzdoom/blob/master/src/playsim/a_dynlight.cpp#L435-L443

[madame-rachelle]

I've gone with the branch merge instead of this PR, because ultimately while performance is indeed worse with TMap it is still an improvement over the previous status quo.

Also, it lights a goalpost for improving TMap at some point, hopefully. :)

[Talon1024]

Does this change not apply to spot lights? When I was playing Dragon Sector last week, I noticed some severe FPS drops in the outdoor areas when I turned on the flashlight. And as far as I know, the flashlight in Dragon Sector/Project Brutality is a spot light.

[RicardoLuis0]

Does this change not apply to spot lights? When I was playing Dragon Sector last week, I noticed some severe FPS drops in the outdoor areas when I turned on the flashlight. And as far as I know, the flashlight in Dragon Sector/Project Brutality is a spot light.

it will still have fps drops with extreme light ranges and sector heavy mods, since it still fundamentally iterates over sectors, this just improves the performance of the iteration, if you can on the flashlight mod, simply reduce the light range/radius